Perform best effort ssl revocation check on Windows

When performing a request via curl on Windows using schannel, ssl
certificate revocation checks does not handle verification failures
gracefully when an error is encountered that is unrelated to the actual
revocation of a certificate.

A new option is available to perform best effort revocation checks on
curl, so this is enabled by default on the Windows platform. A new
config option (`box_download_disable_ssl_revoke_best_effort`) has also
been added which can be optionally enabled to restore previous behavior
which results in a hard error if any error is encountered.

lib/vagrant/action/builtin/box_add.rb

@@ -443,6 +443,7 @@ module Vagrant
           downloader_options[:headers] = ["Accept: application/json"] if opts[:json]
           downloader_options[:ui] = env[:ui] if opts[:ui]
           downloader_options[:location_trusted] = env[:box_download_location_trusted]
+          downloader_options[:disable_ssl_revoke_best_effort] = env[:box_download_disable_ssl_revoke_best_effort]
           downloader_options[:box_extra_download_options] = env[:box_extra_download_options]
 
           d = Util::Downloader.new(url, temp_path, downloader_options)

lib/vagrant/action/builtin/box_check_outdated.rb

@@ -47,6 +47,8 @@ module Vagrant
                            machine.config.vm.box_download_client_cert,
             insecure: !env[:insecure].nil? ?
                         env[:insecure] : machine.config.vm.box_download_insecure,
+            disable_ssl_revoke_best_effort: env.fetch(:box_download_disable_ssl_revoke_best_effort,
+                                                      machine.config.vm.box_download_disable_ssl_revoke_best_effort),
             box_extra_download_options: env[:box_extra_download_options] || machine.config.vm.box_extra_download_options,
           }
 

lib/vagrant/action/builtin/handle_box.rb

@@ -67,6 +67,7 @@ module Vagrant
           box_download_checksum_type = machine.config.vm.box_download_checksum_type
           box_download_checksum = machine.config.vm.box_download_checksum
           box_download_location_trusted = machine.config.vm.box_download_location_trusted
+          box_download_disable_ssl_revoke_best_effort = machine.config.vm.box_download_disable_ssl_revoke_best_effort
           box_extra_download_options = machine.config.vm.box_extra_download_options
           box_formats = machine.provider_options[:box_format] ||
             machine.provider_name
@@ -93,6 +94,7 @@ module Vagrant
               box_checksum_type: box_download_checksum_type,
               box_checksum: box_download_checksum,
               box_download_location_trusted: box_download_location_trusted,
+              box_download_disable_ssl_revoke_best_effort: box_download_disable_ssl_revoke_best_effort,
               box_extra_download_options: box_extra_download_options,
             }))
           rescue Errors::BoxAlreadyExists

lib/vagrant/util/downloader.rb

@@ -72,6 +72,14 @@ module Vagrant
           :sha512 => options[:sha512]
         }.compact
         @extra_download_options = options[:box_extra_download_options] || []
+        # If on Windows SSL revocation checks should be best effort. More context
+        # for this usage can be found in the following links:
+        #
+        # https://github.com/curl/curl/issues/3727
+        # https://github.com/curl/curl/pull/4981
+        if Platform.windows?
+          @ssl_revoke_best_effort = !options[:disable_ssl_revoke_best_effort]
+        end
       end
 
       # This executes the actual download, downloading the source file
@@ -247,6 +255,7 @@ module Vagrant
         options << "--cert" << @client_cert if @client_cert
         options << "-u" << @auth if @auth
         options << "--location-trusted" if @location_trusted
+        options << "--ssl-revoke-best-effort" if @ssl_revoke_best_effort
 
         options.concat(@extra_download_options)
 

plugins/kernel_v2/config/vm.rb

@@ -39,6 +39,7 @@ module VagrantPlugins
       attr_accessor :box_download_checksum
       attr_accessor :box_download_checksum_type
       attr_accessor :box_download_client_cert
+      attr_accessor :box_download_disable_ssl_revoke_best_effort
       attr_accessor :box_download_insecure
       attr_accessor :box_download_location_trusted
       attr_accessor :box_download_options
@@ -72,6 +73,7 @@ module VagrantPlugins
         @box_download_checksum         = UNSET_VALUE
         @box_download_checksum_type    = UNSET_VALUE
         @box_download_client_cert      = UNSET_VALUE
+        @box_download_disable_ssl_revoke_best_effort = UNSET_VALUE
         @box_download_insecure         = UNSET_VALUE
         @box_download_location_trusted = UNSET_VALUE
         @box_download_options          = UNSET_VALUE
@@ -529,6 +531,7 @@ module VagrantPlugins
         @box_download_checksum = nil if @box_download_checksum == UNSET_VALUE
         @box_download_checksum_type = nil if @box_download_checksum_type == UNSET_VALUE
         @box_download_client_cert = nil if @box_download_client_cert == UNSET_VALUE
+        @box_download_disable_ssl_revoke_best_effort = false if @box_download_disable_ssl_revoke_best_effort == UNSET_VALUE
         @box_download_insecure = false if @box_download_insecure == UNSET_VALUE
         @box_download_location_trusted = false if @box_download_location_trusted == UNSET_VALUE
         @box_url = nil if @box_url == UNSET_VALUE

test/unit/vagrant/action/builtin/box_add_test.rb

@@ -1353,4 +1353,97 @@ describe Vagrant::Action::Builtin::BoxAdd, :skip_windows, :bsdtar do
       expect(env[:box_added]).to equal(box)
     end
   end
+
+  describe "#downloader" do
+    let(:options) { {} }
+
+    after { subject.send(:downloader, url, env, **options) }
+
+    context "when box is local" do
+      let(:url) { "/dev/null/vagrant.box" }
+
+      it "should prepend file protocol to path" do
+        expect(Vagrant::Util::Downloader).to receive(:new) do |durl, *_|
+          expect(durl).to eq("file://#{url}")
+        end
+      end
+    end
+
+    context "when box is remote" do
+      let(:url) { "http://localhost/vagrant.box" }
+
+      it "should not modify the url" do
+        expect(Vagrant::Util::Downloader).to receive(:new) do |durl, *_|
+          expect(durl).to eq(url)
+        end
+      end
+    end
+
+    context "when options are set in the environment" do
+      let(:url) { "http://localhost/vagrant.box" }
+
+      context "when ca cert value is set" do
+        let(:ca_cert) { "/dev/null/ca.cert" }
+        before { env[:box_download_ca_cert] = ca_cert }
+
+        it "should include ca cert value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:ca_cert]).to eq(ca_cert)
+          end
+        end
+      end
+
+      context "when ca path value is set" do
+        let(:ca_path) { "/dev/null/ca.path" }
+        before { env[:box_download_ca_path] = ca_path }
+
+        it "should include ca path value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:ca_path]).to eq(ca_path)
+          end
+        end
+      end
+
+      context "when insecure value is set" do
+        before { env[:box_download_insecure] = true }
+
+        it "should include insecure value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:insecure]).to be_truthy
+          end
+        end
+      end
+
+      context "when client cert value is set" do
+        let(:client_cert_path) { "/dev/null/client.cert" }
+        before { env[:box_download_client_cert] = client_cert_path }
+
+        it "should include client cert value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:client_cert]).to eq(client_cert_path)
+          end
+        end
+      end
+
+      context "when location trusted is set" do
+        before { env[:box_download_location_trusted] = true }
+
+        it "should include client cert value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:location_trusted]).to be_truthy
+          end
+        end
+      end
+
+      context "when disable ssl revoke best effort is set" do
+        before { env[:box_download_disable_ssl_revoke_best_effort] = true }
+
+        it "should include disable revoke best effort value value" do
+          expect(Vagrant::Util::Downloader).to receive(:new) do |_, _, opts|
+            expect(opts[:disable_ssl_revoke_best_effort]).to be_truthy
+          end
+        end
+      end
+    end
+  end
 end

test/unit/vagrant/action/builtin/box_check_outdated_test.rb

@@ -129,7 +129,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
 
       expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
           {download_options:
-            {automatic_check: true, ca_cert: nil, ca_path: nil, client_cert: nil, insecure: false, box_extra_download_options: []}}).
+            {automatic_check: true, ca_cert: nil, ca_path: nil, client_cert: nil, insecure: false, disable_ssl_revoke_best_effort: false, box_extra_download_options: []}}).
         and_return([md, md.version("1.1"), md.version("1.1").provider("virtualbox")])
 
       expect(app).to receive(:call).with(env).once
@@ -175,7 +175,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
         expect(box).to receive(:has_update?).with(
           machine.config.vm.box_version,
           { download_options: { automatic_check: true, ca_cert: nil, ca_path: nil,
-            client_cert: nil, insecure: false, box_extra_download_options: []
+            client_cert: nil, disable_ssl_revoke_best_effort: false, insecure: false, box_extra_download_options: []
           }}).and_return(
             [md, md.version("1.2"), md.version("1.2").provider("virtualbox")]
           )
@@ -249,6 +249,7 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
         machine.config.vm.box_download_client_cert = "baz"
         machine.config.vm.box_download_insecure = true
         machine.config.vm.box_download_options = {"opt": "val"}
+        machine.config.vm.box_download_disable_ssl_revoke_best_effort = true
         machine.config.vm.finalize!
       end
 
@@ -256,7 +257,8 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
         expect(box).to receive(:has_update?).with(machine.config.vm.box_version,
           { download_options: {
             automatic_check: true, ca_cert: "foo", ca_path: "bar", client_cert: "baz", 
-            insecure: true, box_extra_download_options: ["--opt", "val"]}})
+            insecure: true, disable_ssl_revoke_best_effort: true,
+            box_extra_download_options: ["--opt", "val"]}})
 
         expect(app).to receive(:call).with(env).once
 
@@ -267,7 +269,8 @@ describe Vagrant::Action::Builtin::BoxCheckOutdated do
         expect(box).to receive(:has_update?).with(
           machine.config.vm.box_version,
           { download_options: {automatic_check: true, ca_cert: "oof", ca_path: "rab", client_cert: "zab", 
-            insecure: false, box_extra_download_options: ["--tpo"],
+            insecure: false, disable_ssl_revoke_best_effort: true,
+            box_extra_download_options: ["--tpo"],
           }})
 
         env[:ca_cert] = "oof"

test/unit/vagrant/util/downloader_test.rb

@@ -35,6 +35,31 @@ describe Vagrant::Util::Downloader do
        "--output", destination, source, {}]
     }
 
+    context "on Windows" do
+      before do
+        allow(Vagrant::Util::Platform).to receive(:windows?).and_return(true)
+      end
+
+      it "should use best effort for ssl revocation check by default" do
+        expect(subject).to receive(:execute_curl) do |opts, *_|
+          expect(opts).to include("--ssl-revoke-best-effort")
+        end
+        subject.download!
+      end
+
+      context "when ssl revoke best effort is disabled" do
+        let(:options) { {disable_ssl_revoke_best_effort: true} }
+
+        it "should not use best effort for ssl revocation check" do
+          expect(subject).to receive(:execute_curl) do |opts, _|
+            expect(opts).not_to include("--ssl-revoke-best-effort")
+          end
+
+          subject.download!
+        end
+      end
+    end
+
     context "with UI" do
       let(:ui) { Vagrant::UI::Silent.new }
       let(:options) { {ui: ui} }

website/content/docs/vagrantfile/machine_settings.mdx

@@ -72,6 +72,11 @@ the name of the synced folder plugin.
   CA certificates for downloading a box directly. By default, Vagrant will
   use the Mozilla CA cert bundle.
 
+- `config.vm.box_download_disable_ssl_revoke_best_effort` (boolean) - Disable SSL
+  revocation checking from being best effort. If an error is encountered when attempting
+  to check certificate revocation, enabling this option will halt the request. This
+  option is only applied on the Windows platform and defaults to `false`.
+
 - `config.vm.box_download_options` (map) - A map of extra download options
   to pass to the downloader. For example, a path to a key that the downloader
   should use could be specified as `{key: "<path/to/key>"}`. The keys should