[PATCH 4 of 4 RFC] smtp: verify the certificate of the SMTP server for STARTTLS/SMTPS
Augie Fackler
raf at durin42.com
Sun Apr 7 14:58:19 CDT 2013
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 at lares.dti.ne.jp> wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at 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 at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list