Patchwork [4,of,4,RFC] smtp: verify the certificate of the SMTP server for STARTTLS/SMTPS

login
register
mail settings
Submitter Katsunori FUJIWARA
Date March 25, 2013, 5:32 p.m.
Message ID <9e6f7b025beec226db65.1364232754@juju>
Download mbox | patch
Permalink /patch/1189/
State Accepted
Commit 19d489404d79058d518473798423185bd1cd7558
Delegated to: Bryan O'Sullivan
Headers show

Comments

Katsunori FUJIWARA - March 25, 2013, 5:32 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1364232490 -32400
# Node ID 9e6f7b025beec226db65af27bf9ed7dcbfdfa190
# Parent  6148e84c076b4b660408d76a519daedf06a5255a
smtp: verify the certificate of the SMTP server for STARTTLS/SMTPS

Before this patch, the certificate of the SMTP server for STARTTLS or
SMTPS isn't verified.

This may cause man-in-the-middle security problem (stealing
authentication information), even though SMTP channel itself is
encrypted by SSL.

When "[smtp] tls" is configured as "smtps" or "starttls", this patch:

  - uses classes introduced by preceding patches instead of "SMTP" or
    "SMTP_SSL" of smtplib, and

  - verifies the certificate of the SMTP server, if "[smtp]
    verifycert" is configured as other than False

"[smtp] verifycert" can be configured in 3 levels:

  - "strict":

    This verifies peer certificate, and aborts if:

      - peer certification is not valid, or
      - no configuration in "[hostfingerprints]" and "[web] cacerts"

    This is default value of "[smtp] verifycert" for security.

  - "loose":

    This verifies peer certificate, and aborts if peer certification is
    not valid.

    This just shows warning message ("certificate not verified"), if
    there is no configuration in "[hostfingerprints]" and "[web]
    cacerts".

    This is as same as verification for HTTPS connection.

  - False(no verification):

    Peer certificate is not verified.

    This is as same as the behavior before this patch series.

"hg email --insecure" uses "loose" level, and ignores "[web] cacerts"
as same as push/pull/etc... with --insecure.

Ignoring "[web] cacerts" configuration for "hg email --insecure" is
already done in "dispatch._dispatch()" by looking "insecure" up in the
table of command options.
Augie Fackler - April 7, 2013, 7:58 p.m.
Series LGTM, but since it's security code I'd like someone else to take a quick look. Putting a couple of crew folks on To: in hopes that one of them will be willing to take a look.

Sorry about the delay in looking at this.

On Mar 25, 2013, at 1:32 PM, FUJIWARA Katsunori <foozy@lares.dti.ne.jp> wrote:

> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1364232490 -32400
> # Node ID 9e6f7b025beec226db65af27bf9ed7dcbfdfa190
> # Parent  6148e84c076b4b660408d76a519daedf06a5255a
> smtp: verify the certificate of the SMTP server for STARTTLS/SMTPS
> 
> Before this patch, the certificate of the SMTP server for STARTTLS or
> SMTPS isn't verified.
> 
> This may cause man-in-the-middle security problem (stealing
> authentication information), even though SMTP channel itself is
> encrypted by SSL.
> 
> When "[smtp] tls" is configured as "smtps" or "starttls", this patch:
> 
>  - uses classes introduced by preceding patches instead of "SMTP" or
>    "SMTP_SSL" of smtplib, and
> 
>  - verifies the certificate of the SMTP server, if "[smtp]
>    verifycert" is configured as other than False
> 
> "[smtp] verifycert" can be configured in 3 levels:
> 
>  - "strict":
> 
>    This verifies peer certificate, and aborts if:
> 
>      - peer certification is not valid, or
>      - no configuration in "[hostfingerprints]" and "[web] cacerts"
> 
>    This is default value of "[smtp] verifycert" for security.
> 
>  - "loose":
> 
>    This verifies peer certificate, and aborts if peer certification is
>    not valid.
> 
>    This just shows warning message ("certificate not verified"), if
>    there is no configuration in "[hostfingerprints]" and "[web]
>    cacerts".
> 
>    This is as same as verification for HTTPS connection.
> 
>  - False(no verification):
> 
>    Peer certificate is not verified.
> 
>    This is as same as the behavior before this patch series.
> 
> "hg email --insecure" uses "loose" level, and ignores "[web] cacerts"
> as same as push/pull/etc... with --insecure.
> 
> Ignoring "[web] cacerts" configuration for "hg email --insecure" is
> already done in "dispatch._dispatch()" by looking "insecure" up in the
> table of command options.
> 
> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
> --- a/hgext/patchbomb.py
> +++ b/hgext/patchbomb.py
> @@ -540,7 +540,13 @@
>                 fp.close()
>         else:
>             if not sendmail:
> -                sendmail = mail.connect(ui, mbox=mbox)
> +                verifycert = ui.config('smtp', 'verifycert')
> +                if opts.get('insecure'):
> +                    ui.setconfig('smtp', 'verifycert', 'loose')
> +                try:
> +                    sendmail = mail.connect(ui, mbox=mbox)
> +                finally:
> +                    ui.setconfig('smtp', 'verifycert', verifycert)
>             ui.status(_('sending '), subj, ' ...\n')
>             ui.progress(_('sending'), i, item=subj, total=len(msgs))
>             if not mbox:
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1052,6 +1052,16 @@
>     Optional. Method to enable TLS when connecting to mail server: starttls,
>     smtps or none. Default: none.
> 
> +``verifycert``
> +    Optional. Verification for the certificate of mail server, when
> +    ``tls`` is starttls or smtps. "strict", "loose" or False. For
> +    "strict" or "loose", the certificate is verified as same as the
> +    verification for HTTPS connections (see ``[hostfingerprints]`` and
> +    ``[web] cacerts`` also). For "strict", sending email is also
> +    aborted, if there is no configuration for mail server in
> +    ``[hostfingerprints]`` and ``[web] cacerts``.  --insecure for
> +    :hg:`email` overwrites this as "loose". Default: "strict".
> +
> ``username``
>     Optional. User name for authenticating with the SMTP server.
>     Default: none.
> diff --git a/mercurial/mail.py b/mercurial/mail.py
> --- a/mercurial/mail.py
> +++ b/mercurial/mail.py
> @@ -91,14 +91,25 @@
>     smtps = tls == 'smtps'
>     if (starttls or smtps) and not util.safehasattr(socket, 'ssl'):
>         raise util.Abort(_("can't use TLS: Python SSL support not installed"))
> -    if smtps:
> -        ui.note(_('(using smtps)\n'))
> -        s = smtplib.SMTP_SSL(local_hostname=local_hostname)
> -    else:
> -        s = smtplib.SMTP(local_hostname=local_hostname)
>     mailhost = ui.config('smtp', 'host')
>     if not mailhost:
>         raise util.Abort(_('smtp.host not configured - cannot send mail'))
> +    verifycert = ui.config('smtp', 'verifycert', 'strict')
> +    if verifycert not in ['strict', 'loose']:
> +        if util.parsebool(verifycert) is not False:
> +            raise util.Abort(_('invalid smtp.verifycert configuration: %s')
> +                             % (verifycert))
> +    if (starttls or smtps) and verifycert:
> +        sslkwargs = sslutil.sslkwargs(ui, mailhost)
> +    else:
> +        sslkwargs = {}
> +    if smtps:
> +        ui.note(_('(using smtps)\n'))
> +        s = SMTPS(sslkwargs, local_hostname=local_hostname)
> +    elif starttls:
> +        s = STARTTLS(sslkwargs, local_hostname=local_hostname)
> +    else:
> +        s = smtplib.SMTP(local_hostname=local_hostname)
>     mailport = util.getport(ui.config('smtp', 'port', 25))
>     ui.note(_('sending mail: smtp host %s, port %s\n') %
>             (mailhost, mailport))
> @@ -108,6 +119,9 @@
>         s.ehlo()
>         s.starttls()
>         s.ehlo()
> +    if (starttls or smtps) and verifycert:
> +        ui.note(_('(verifying remote certificate)\n'))
> +        sslutil.validator(ui, mailhost)(s.sock, verifycert == 'strict')
>     username = ui.config('smtp', 'username')
>     password = ui.config('smtp', 'password')
>     if username and not password:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -540,7 +540,13 @@ 
                 fp.close()
         else:
             if not sendmail:
-                sendmail = mail.connect(ui, mbox=mbox)
+                verifycert = ui.config('smtp', 'verifycert')
+                if opts.get('insecure'):
+                    ui.setconfig('smtp', 'verifycert', 'loose')
+                try:
+                    sendmail = mail.connect(ui, mbox=mbox)
+                finally:
+                    ui.setconfig('smtp', 'verifycert', verifycert)
             ui.status(_('sending '), subj, ' ...\n')
             ui.progress(_('sending'), i, item=subj, total=len(msgs))
             if not mbox:
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1052,6 +1052,16 @@ 
     Optional. Method to enable TLS when connecting to mail server: starttls,
     smtps or none. Default: none.
 
+``verifycert``
+    Optional. Verification for the certificate of mail server, when
+    ``tls`` is starttls or smtps. "strict", "loose" or False. For
+    "strict" or "loose", the certificate is verified as same as the
+    verification for HTTPS connections (see ``[hostfingerprints]`` and
+    ``[web] cacerts`` also). For "strict", sending email is also
+    aborted, if there is no configuration for mail server in
+    ``[hostfingerprints]`` and ``[web] cacerts``.  --insecure for
+    :hg:`email` overwrites this as "loose". Default: "strict".
+
 ``username``
     Optional. User name for authenticating with the SMTP server.
     Default: none.
diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -91,14 +91,25 @@ 
     smtps = tls == 'smtps'
     if (starttls or smtps) and not util.safehasattr(socket, 'ssl'):
         raise util.Abort(_("can't use TLS: Python SSL support not installed"))
-    if smtps:
-        ui.note(_('(using smtps)\n'))
-        s = smtplib.SMTP_SSL(local_hostname=local_hostname)
-    else:
-        s = smtplib.SMTP(local_hostname=local_hostname)
     mailhost = ui.config('smtp', 'host')
     if not mailhost:
         raise util.Abort(_('smtp.host not configured - cannot send mail'))
+    verifycert = ui.config('smtp', 'verifycert', 'strict')
+    if verifycert not in ['strict', 'loose']:
+        if util.parsebool(verifycert) is not False:
+            raise util.Abort(_('invalid smtp.verifycert configuration: %s')
+                             % (verifycert))
+    if (starttls or smtps) and verifycert:
+        sslkwargs = sslutil.sslkwargs(ui, mailhost)
+    else:
+        sslkwargs = {}
+    if smtps:
+        ui.note(_('(using smtps)\n'))
+        s = SMTPS(sslkwargs, local_hostname=local_hostname)
+    elif starttls:
+        s = STARTTLS(sslkwargs, local_hostname=local_hostname)
+    else:
+        s = smtplib.SMTP(local_hostname=local_hostname)
     mailport = util.getport(ui.config('smtp', 'port', 25))
     ui.note(_('sending mail: smtp host %s, port %s\n') %
             (mailhost, mailport))
@@ -108,6 +119,9 @@ 
         s.ehlo()
         s.starttls()
         s.ehlo()
+    if (starttls or smtps) and verifycert:
+        ui.note(_('(verifying remote certificate)\n'))
+        sslutil.validator(ui, mailhost)(s.sock, verifycert == 'strict')
     username = ui.config('smtp', 'username')
     password = ui.config('smtp', 'password')
     if username and not password: