Patchwork [08,of,11] sslutil: handle ui.insecureconnections in validator

login
register
mail settings
Submitter Gregory Szorc
Date May 5, 2016, 7:53 a.m.
Message ID <bd7ebeaf995eaab11afe.1462434805@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/14900/
State Accepted
Headers show

Comments

Gregory Szorc - May 5, 2016, 7:53 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1462433848 25200
#      Thu May 05 00:37:28 2016 -0700
# Node ID bd7ebeaf995eaab11afe722a6fba61d9c7c0f3c4
# Parent  a32c736a9c48137accd777b343cbed85191409ef
sslutil: handle ui.insecureconnections in validator

Right now, web.cacerts=! means one of two things:

1) Use of --insecure
2) No CAs could be found and were loaded (see sslkwargs)

This isn't very obvious and makes changing behavior of these
different scenarios independent of the other impossible.

This patch changes the validator code to explicit handle the
case of --insecure being used.

As the inline comment indicates, there is room to possibly change
messaging and logic here. For now, we are backwards compatible.
Yuya Nishihara - May 6, 2016, 5:29 a.m.
On Thu, 05 May 2016 00:53:25 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1462433848 25200
> #      Thu May 05 00:37:28 2016 -0700
> # Node ID bd7ebeaf995eaab11afe722a6fba61d9c7c0f3c4
> # Parent  a32c736a9c48137accd777b343cbed85191409ef
> sslutil: handle ui.insecureconnections in validator
> 
> Right now, web.cacerts=! means one of two things:
> 
> 1) Use of --insecure
> 2) No CAs could be found and were loaded (see sslkwargs)
> 
> This isn't very obvious and makes changing behavior of these
> different scenarios independent of the other impossible.
> 
> This patch changes the validator code to explicit handle the
> case of --insecure being used.
> 
> As the inline comment indicates, there is room to possibly change
> messaging and logic here. For now, we are backwards compatible.
> 
> diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
> --- a/mercurial/sslutil.py
> +++ b/mercurial/sslutil.py
> @@ -324,16 +324,29 @@ class validator(object):
>              if not fingerprintmatch:
>                  raise error.Abort(_('certificate for %s has unexpected '
>                                     'fingerprint %s') % (host, nicefingerprint),
>                                   hint=_('check hostfingerprint configuration'))
>              self.ui.debug('%s certificate matched fingerprint %s\n' %
>                            (host, nicefingerprint))
>              return
>  
> +        # If insecure connections were explicitly requested via --insecure,
> +        # print a warning and do no verification.
> +        #
> +        # It may seem odd that this is checked *after* host fingerprint pinning.
> +        # This is for backwards compatibility (for now). The message is also
> +        # the same as below for BC.
> +        if self.ui.insecureconnections:
> +            self.ui.warn(_('warning: %s certificate with fingerprint %s not '
> +                           'verified (check hostfingerprints or web.cacerts '
> +                           'config setting)\n') %
> +                         (host, nicefingerprint))
> +            return
> +
>          # No pinned fingerprint. Establish trust by looking at the CAs.
>          cacerts = self.ui.config('web', 'cacerts')
>          if cacerts != '!':

Confirmed that strict=True isn't set if --insecure is specified, so there
should be no behavior change.

Looks good to me.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -324,16 +324,29 @@  class validator(object):
             if not fingerprintmatch:
                 raise error.Abort(_('certificate for %s has unexpected '
                                    'fingerprint %s') % (host, nicefingerprint),
                                  hint=_('check hostfingerprint configuration'))
             self.ui.debug('%s certificate matched fingerprint %s\n' %
                           (host, nicefingerprint))
             return
 
+        # If insecure connections were explicitly requested via --insecure,
+        # print a warning and do no verification.
+        #
+        # It may seem odd that this is checked *after* host fingerprint pinning.
+        # This is for backwards compatibility (for now). The message is also
+        # the same as below for BC.
+        if self.ui.insecureconnections:
+            self.ui.warn(_('warning: %s certificate with fingerprint %s not '
+                           'verified (check hostfingerprints or web.cacerts '
+                           'config setting)\n') %
+                         (host, nicefingerprint))
+            return
+
         # No pinned fingerprint. Establish trust by looking at the CAs.
         cacerts = self.ui.config('web', 'cacerts')
         if cacerts != '!':
             msg = _verifycert(peercert2, host)
             if msg:
                 raise error.Abort(_('%s certificate error: %s') % (host, msg),
                                  hint=_('configure hostfingerprint %s or use '
                                         '--insecure to connect insecurely') %