Patchwork sslutil: per-host config option to define certificates

login
register
mail settings
Submitter Gregory Szorc
Date June 8, 2016, 4:03 a.m.
Message ID <323f0c9c91e02be86bde.1465358624@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15430/
State Accepted
Headers show

Comments

Gregory Szorc - June 8, 2016, 4:03 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1465356594 25200
#      Tue Jun 07 20:29:54 2016 -0700
# Node ID 323f0c9c91e02be86bde60620cec5f38020f4c86
# Parent  1b3a0b0c414faa3d6d4dbcf4c5abbbe18aa9efd4
sslutil: per-host config option to define certificates

Recent work has introduced the [hostsecurity] config section for
defining per-host security settings. This patch builds on top
of this foundation and implements the ability to define a per-host
path to a file containing certificates used for verifying the server
certificate. It is logically a per-host web.cacerts setting.

This patch also introduces a warning when both per-host
certificates and fingerprints are defined. These are mutually
exclusive for host verification and I think the user should be
alerted when security settings are ambiguous because, well,
security is important.

Tests validating the new behavior have been added.

I decided against putting "ca" in the option name because a
non-CA certificate can be specified and used to validate the server
certificate (commonly this will be the exact public certificate
used by the server). It's worth noting that the underlying
Python API used is load_verify_locations(cafile=X) and it calls
into OpenSSL's SSL_CTX_load_verify_locations(). Even OpenSSL's
documentation seems to omit that the file can contain a non-CA
certificate if it matches the server's certificate exactly. I
thought a CA certificate was a special kind of x509 certificate.
Perhaps I'm wrong and any x509 certificate can be used as a
CA certificate [as far as OpenSSL is concerned]. In any case,
I thought it best to drop "ca" from the name because this reflects
reality.
Augie Fackler - June 10, 2016, 3:32 a.m.
On Tue, Jun 07, 2016 at 09:03:44PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1465356594 25200
> #      Tue Jun 07 20:29:54 2016 -0700
> # Node ID 323f0c9c91e02be86bde60620cec5f38020f4c86
> # Parent  1b3a0b0c414faa3d6d4dbcf4c5abbbe18aa9efd4
> sslutil: per-host config option to define certificates

sure, queued

>
> Recent work has introduced the [hostsecurity] config section for
> defining per-host security settings. This patch builds on top
> of this foundation and implements the ability to define a per-host
> path to a file containing certificates used for verifying the server
> certificate. It is logically a per-host web.cacerts setting.
>
> This patch also introduces a warning when both per-host
> certificates and fingerprints are defined. These are mutually
> exclusive for host verification and I think the user should be
> alerted when security settings are ambiguous because, well,
> security is important.
>
> Tests validating the new behavior have been added.
>
> I decided against putting "ca" in the option name because a
> non-CA certificate can be specified and used to validate the server
> certificate (commonly this will be the exact public certificate
> used by the server). It's worth noting that the underlying
> Python API used is load_verify_locations(cafile=X) and it calls
> into OpenSSL's SSL_CTX_load_verify_locations(). Even OpenSSL's
> documentation seems to omit that the file can contain a non-CA
> certificate if it matches the server's certificate exactly. I
> thought a CA certificate was a special kind of x509 certificate.
> Perhaps I'm wrong and any x509 certificate can be used as a
> CA certificate [as far as OpenSSL is concerned]. In any case,
> I thought it best to drop "ca" from the name because this reflects
> reality.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1019,21 +1019,49 @@ The following per-host settings can be d
>
>      If a fingerprint is specified, the CA chain is not validated for this
>      host and Mercurial will require the remote certificate to match one
>      of the fingerprints specified. This means if the server updates its
>      certificate, Mercurial will abort until a new fingerprint is defined.
>      This can provide stronger security than traditional CA-based validation
>      at the expense of convenience.
>
> +    This option takes precedence over ``verifycertsfile``.
> +
> +``verifycertsfile``
> +    Path to file a containing a list of PEM encoded certificates used to
> +    verify the server certificate. Environment variables and ``~user``
> +    constructs are expanded in the filename.
> +
> +    The server certificate or the certificate's certificate authority (CA)
> +    must match a certificate from this file or certificate verification
> +    will fail and connections to the server will be refused.
> +
> +    If defined, only certificates provided by this file will be used:
> +    ``web.cacerts`` and any system/default certificates will not be
> +    used.
> +
> +    This option has no effect if the per-host ``fingerprints`` option
> +    is set.
> +
> +    The format of the file is as follows:
> +
> +        -----BEGIN CERTIFICATE-----
> +        ... (certificate in base64 PEM encoding) ...
> +        -----END CERTIFICATE-----
> +        -----BEGIN CERTIFICATE-----
> +        ... (certificate in base64 PEM encoding) ...
> +        -----END CERTIFICATE-----
> +
>  For example::
>
>      [hostsecurity]
>      hg.example.com:fingerprints = sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2
>      hg2.example.com:fingerprints = sha1:914f1aff87249c09b6859b88b1906d30756491ca, sha1:fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
> +    foo.example.com:verifycertsfile = /etc/ssl/trusted-ca-certs.pem
>
>  ``http_proxy``
>  --------------
>
>  Used to access web-based Mercurial repositories through a HTTP
>  proxy.
>
>  ``host``
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> --- a/mercurial/sslutil.py
> +++ b/mercurial/sslutil.py
> @@ -157,33 +157,52 @@ def _hostsettings(ui, hostname):
>      # If --insecure is used, don't take CAs into consideration.
>      elif ui.insecureconnections:
>          s['disablecertverification'] = True
>          s['verifymode'] = ssl.CERT_NONE
>
>      if ui.configbool('devel', 'disableloaddefaultcerts'):
>          s['allowloaddefaultcerts'] = False
>
> +    # If both fingerprints and a per-host ca file are specified, issue a warning
> +    # because users should not be surprised about what security is or isn't
> +    # being performed.
> +    cafile = ui.config('hostsecurity', '%s:verifycertsfile' % hostname)
> +    if s['certfingerprints'] and cafile:
> +        ui.warn(_('(hostsecurity.%s:verifycertsfile ignored when host '
> +                  'fingerprints defined; using host fingerprints for '
> +                  'verification)\n') % hostname)
> +
>      # Try to hook up CA certificate validation unless something above
>      # makes it not necessary.
>      if s['verifymode'] is None:
> -        # Find global certificates file in config.
> -        cafile = ui.config('web', 'cacerts')
> -
> +        # Look at per-host ca file first.
>          if cafile:
>              cafile = util.expandpath(cafile)
>              if not os.path.exists(cafile):
> -                raise error.Abort(_('could not find web.cacerts: %s') % cafile)
> +                raise error.Abort(_('path specified by %s does not exist: %s') %
> +                                  ('hostsecurity.%s:verifycertsfile' % hostname,
> +                                   cafile))
> +            s['cafile'] = cafile
>          else:
> -            # No global CA certs. See if we can load defaults.
> -            cafile = _defaultcacerts()
> +            # Find global certificates file in config.
> +            cafile = ui.config('web', 'cacerts')
> +
>              if cafile:
> -                ui.debug('using %s to enable OS X system CA\n' % cafile)
> +                cafile = util.expandpath(cafile)
> +                if not os.path.exists(cafile):
> +                    raise error.Abort(_('could not find web.cacerts: %s') %
> +                                      cafile)
> +            else:
> +                # No global CA certs. See if we can load defaults.
> +                cafile = _defaultcacerts()
> +                if cafile:
> +                    ui.debug('using %s to enable OS X system CA\n' % cafile)
>
> -        s['cafile'] = cafile
> +            s['cafile'] = cafile
>
>          # Require certificate validation if CA certs are being loaded and
>          # verification hasn't been disabled above.
>          if cafile or (_canloaddefaultcerts and s['allowloaddefaultcerts']):
>              s['verifymode'] = ssl.CERT_REQUIRED
>          else:
>              # At this point we don't have a fingerprint, aren't being
>              # explicitly insecure, and can't load CA certs. Connecting
> diff --git a/tests/test-https.t b/tests/test-https.t
> --- a/tests/test-https.t
> +++ b/tests/test-https.t
> @@ -166,16 +166,64 @@ Our test cert is not signed by a trusted
>  we are able to load CA certs.
>
>  #if defaultcacerts
>    $ hg clone https://localhost:$HGPORT/ copy-pull
>    abort: error: *certificate verify failed* (glob)
>    [255]
>  #endif
>
> +Specifying a per-host certificate file that doesn't exist will abort
> +
> +  $ hg --config hostsecurity.localhost:verifycertsfile=/does/not/exist clone https://localhost:$HGPORT/
> +  abort: path specified by hostsecurity.localhost:verifycertsfile does not exist: /does/not/exist
> +  [255]
> +
> +A malformed per-host certificate file will raise an error
> +
> +  $ echo baddata > badca.pem
> +  $ hg --config hostsecurity.localhost:verifycertsfile=badca.pem clone https://localhost:$HGPORT/
> +  abort: error: unknown error* (glob)
> +  [255]
> +
> +A per-host certificate mismatching the server will fail verification
> +
> +  $ hg --config hostsecurity.localhost:verifycertsfile=client-cert.pem clone https://localhost:$HGPORT/
> +  abort: error: *certificate verify failed* (glob)
> +  [255]
> +
> +A per-host certificate matching the server's cert will be accepted
> +
> +  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem clone -U https://localhost:$HGPORT/ perhostgood1
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 4 changes to 4 files
> +
> +A per-host certificate with multiple certs and one matching will be accepted
> +
> +  $ cat client-cert.pem pub.pem > perhost.pem
> +  $ hg --config hostsecurity.localhost:verifycertsfile=perhost.pem clone -U https://localhost:$HGPORT/ perhostgood2
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 4 changes to 4 files
> +
> +Defining both per-host certificate and a fingerprint will print a warning
> +
> +  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem --config hostsecurity.localhost:fingerprints=sha1:914f1aff87249c09b6859b88b1906d30756491ca clone -U https://localhost:$HGPORT/ caandfingerwarning
> +  (hostsecurity.localhost:verifycertsfile ignored when host fingerprints defined; using host fingerprints for verification)
> +  requesting all changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 4 changes to 4 files
> +
>    $ DISABLECACERTS="--config devel.disableloaddefaultcerts=true"
>
>  clone via pull
>
>    $ hg clone https://localhost:$HGPORT/ copy-pull $DISABLECACERTS
>    warning: certificate for localhost not verified (set hostsecurity.localhost:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30 or web.cacerts config settings)
>    requesting all changes
>    adding changesets
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - June 10, 2016, 2:14 p.m.
On Thu, 9 Jun 2016 23:32:38 -0400, Augie Fackler wrote:
> On Tue, Jun 07, 2016 at 09:03:44PM -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1465356594 25200
> > #      Tue Jun 07 20:29:54 2016 -0700
> > # Node ID 323f0c9c91e02be86bde60620cec5f38020f4c86
> > # Parent  1b3a0b0c414faa3d6d4dbcf4c5abbbe18aa9efd4
> > sslutil: per-host config option to define certificates

> > +A per-host certificate mismatching the server will fail verification
> > +
> > +  $ hg --config hostsecurity.localhost:verifycertsfile=client-cert.pem clone https://localhost:$HGPORT/
> > +  abort: error: *certificate verify failed* (glob)
> > +  [255]
> > +
> > +A per-host certificate matching the server's cert will be accepted
> > +
> > +  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem clone -U https://localhost:$HGPORT/ perhostgood1
> > +  requesting all changes
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 1 changesets with 4 changes to 4 files
> > +
> > +A per-host certificate with multiple certs and one matching will be accepted
> > +
> > +  $ cat client-cert.pem pub.pem > perhost.pem
> > +  $ hg --config hostsecurity.localhost:verifycertsfile=perhost.pem clone -U https://localhost:$HGPORT/ perhostgood2
> > +  requesting all changes
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 1 changesets with 4 changes to 4 files
> > +
> > +Defining both per-host certificate and a fingerprint will print a warning
> > +
> > +  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem --config hostsecurity.localhost:fingerprints=sha1:914f1aff87249c09b6859b88b1906d30756491ca clone -U https://localhost:$HGPORT/ caandfingerwarning
> > +  (hostsecurity.localhost:verifycertsfile ignored when host fingerprints defined; using host fingerprints for verification)
> > +  requesting all changes
> > +  adding changesets
> > +  adding manifests
> > +  adding file changes
> > +  added 1 changesets with 4 changes to 4 files

Updated $CERTSDIR per my change.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1019,21 +1019,49 @@  The following per-host settings can be d
 
     If a fingerprint is specified, the CA chain is not validated for this
     host and Mercurial will require the remote certificate to match one
     of the fingerprints specified. This means if the server updates its
     certificate, Mercurial will abort until a new fingerprint is defined.
     This can provide stronger security than traditional CA-based validation
     at the expense of convenience.
 
+    This option takes precedence over ``verifycertsfile``.
+
+``verifycertsfile``
+    Path to file a containing a list of PEM encoded certificates used to
+    verify the server certificate. Environment variables and ``~user``
+    constructs are expanded in the filename.
+
+    The server certificate or the certificate's certificate authority (CA)
+    must match a certificate from this file or certificate verification
+    will fail and connections to the server will be refused.
+
+    If defined, only certificates provided by this file will be used:
+    ``web.cacerts`` and any system/default certificates will not be
+    used.
+
+    This option has no effect if the per-host ``fingerprints`` option
+    is set.
+
+    The format of the file is as follows:
+
+        -----BEGIN CERTIFICATE-----
+        ... (certificate in base64 PEM encoding) ...
+        -----END CERTIFICATE-----
+        -----BEGIN CERTIFICATE-----
+        ... (certificate in base64 PEM encoding) ...
+        -----END CERTIFICATE-----
+
 For example::
 
     [hostsecurity]
     hg.example.com:fingerprints = sha256:c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2
     hg2.example.com:fingerprints = sha1:914f1aff87249c09b6859b88b1906d30756491ca, sha1:fc:e2:8d:d9:51:cd:cb:c1:4d:18:6b:b7:44:8d:49:72:57:e6:cd:33
+    foo.example.com:verifycertsfile = /etc/ssl/trusted-ca-certs.pem
 
 ``http_proxy``
 --------------
 
 Used to access web-based Mercurial repositories through a HTTP
 proxy.
 
 ``host``
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -157,33 +157,52 @@  def _hostsettings(ui, hostname):
     # If --insecure is used, don't take CAs into consideration.
     elif ui.insecureconnections:
         s['disablecertverification'] = True
         s['verifymode'] = ssl.CERT_NONE
 
     if ui.configbool('devel', 'disableloaddefaultcerts'):
         s['allowloaddefaultcerts'] = False
 
+    # If both fingerprints and a per-host ca file are specified, issue a warning
+    # because users should not be surprised about what security is or isn't
+    # being performed.
+    cafile = ui.config('hostsecurity', '%s:verifycertsfile' % hostname)
+    if s['certfingerprints'] and cafile:
+        ui.warn(_('(hostsecurity.%s:verifycertsfile ignored when host '
+                  'fingerprints defined; using host fingerprints for '
+                  'verification)\n') % hostname)
+
     # Try to hook up CA certificate validation unless something above
     # makes it not necessary.
     if s['verifymode'] is None:
-        # Find global certificates file in config.
-        cafile = ui.config('web', 'cacerts')
-
+        # Look at per-host ca file first.
         if cafile:
             cafile = util.expandpath(cafile)
             if not os.path.exists(cafile):
-                raise error.Abort(_('could not find web.cacerts: %s') % cafile)
+                raise error.Abort(_('path specified by %s does not exist: %s') %
+                                  ('hostsecurity.%s:verifycertsfile' % hostname,
+                                   cafile))
+            s['cafile'] = cafile
         else:
-            # No global CA certs. See if we can load defaults.
-            cafile = _defaultcacerts()
+            # Find global certificates file in config.
+            cafile = ui.config('web', 'cacerts')
+
             if cafile:
-                ui.debug('using %s to enable OS X system CA\n' % cafile)
+                cafile = util.expandpath(cafile)
+                if not os.path.exists(cafile):
+                    raise error.Abort(_('could not find web.cacerts: %s') %
+                                      cafile)
+            else:
+                # No global CA certs. See if we can load defaults.
+                cafile = _defaultcacerts()
+                if cafile:
+                    ui.debug('using %s to enable OS X system CA\n' % cafile)
 
-        s['cafile'] = cafile
+            s['cafile'] = cafile
 
         # Require certificate validation if CA certs are being loaded and
         # verification hasn't been disabled above.
         if cafile or (_canloaddefaultcerts and s['allowloaddefaultcerts']):
             s['verifymode'] = ssl.CERT_REQUIRED
         else:
             # At this point we don't have a fingerprint, aren't being
             # explicitly insecure, and can't load CA certs. Connecting
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -166,16 +166,64 @@  Our test cert is not signed by a trusted
 we are able to load CA certs.
 
 #if defaultcacerts
   $ hg clone https://localhost:$HGPORT/ copy-pull
   abort: error: *certificate verify failed* (glob)
   [255]
 #endif
 
+Specifying a per-host certificate file that doesn't exist will abort
+
+  $ hg --config hostsecurity.localhost:verifycertsfile=/does/not/exist clone https://localhost:$HGPORT/
+  abort: path specified by hostsecurity.localhost:verifycertsfile does not exist: /does/not/exist
+  [255]
+
+A malformed per-host certificate file will raise an error
+
+  $ echo baddata > badca.pem
+  $ hg --config hostsecurity.localhost:verifycertsfile=badca.pem clone https://localhost:$HGPORT/
+  abort: error: unknown error* (glob)
+  [255]
+
+A per-host certificate mismatching the server will fail verification
+
+  $ hg --config hostsecurity.localhost:verifycertsfile=client-cert.pem clone https://localhost:$HGPORT/
+  abort: error: *certificate verify failed* (glob)
+  [255]
+
+A per-host certificate matching the server's cert will be accepted
+
+  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem clone -U https://localhost:$HGPORT/ perhostgood1
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 4 changes to 4 files
+
+A per-host certificate with multiple certs and one matching will be accepted
+
+  $ cat client-cert.pem pub.pem > perhost.pem
+  $ hg --config hostsecurity.localhost:verifycertsfile=perhost.pem clone -U https://localhost:$HGPORT/ perhostgood2
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 4 changes to 4 files
+
+Defining both per-host certificate and a fingerprint will print a warning
+
+  $ hg --config hostsecurity.localhost:verifycertsfile=pub.pem --config hostsecurity.localhost:fingerprints=sha1:914f1aff87249c09b6859b88b1906d30756491ca clone -U https://localhost:$HGPORT/ caandfingerwarning
+  (hostsecurity.localhost:verifycertsfile ignored when host fingerprints defined; using host fingerprints for verification)
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 4 changes to 4 files
+
   $ DISABLECACERTS="--config devel.disableloaddefaultcerts=true"
 
 clone via pull
 
   $ hg clone https://localhost:$HGPORT/ copy-pull $DISABLECACERTS
   warning: certificate for localhost not verified (set hostsecurity.localhost:certfingerprints=sha256:62:09:97:2f:97:60:e3:65:8f:12:5d:78:9e:35:a1:36:7a:65:4b:0e:9f:ac:db:c3:bc:6e:b6:a3:c0:16:e0:30 or web.cacerts config settings)
   requesting all changes
   adding changesets