Patchwork [1,of,9,V4] mail: unsupport smtp.verifycert (BC)

login
register
mail settings
Submitter Gregory Szorc
Date June 4, 2016, 6:16 p.m.
Message ID <0910e8d232a8d28708f3.1465064207@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15401/
State Accepted
Headers show

Comments

Gregory Szorc - June 4, 2016, 6:16 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1465064008 25200
#      Sat Jun 04 11:13:28 2016 -0700
# Node ID 0910e8d232a8d28708f3a9427d60b544465ddb1c
# Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
mail: unsupport smtp.verifycert (BC)

smtp.verifycert was accidentally broken by cca59ef27e60. And,
I believe the "loose" value has been broken for longer than that.
The current code refuses to talk to a remote server unless the
CA is trusted or the fingerprint is validated. In other words,
we lost the ability for smtp.verifycert to lower/disable security.

There are special considerations for smtp.verifycert in
sslutil.validatesocket() (the "strict" argument). This violates
the direction sslutil is evolving towards, which has all security
options determined at wrapsocket() time and a unified code path and
configs for determining security options.

Since smtp.verifycert is broken and since we'll soon have new
security defaults and new mechanisms for controlling host security,
this patch formally deprecates smtp.verifycert. With this patch,
the socket security code in mail.py now effectively mirrors code
in url.py and other places we're doing socket security.

For the record, removing smtp.verifycert because it was accidentally
broken is a poor excuse to remove it. However, I would have done this
anyway because smtp.verifycert is a one-off likely used by few people
(users of the patchbomb extension) and I don't think the existence
of this seldom-used one-off in security code can be justified,
especially when you consider that better mechanisms are right around
the corner.
Yuya Nishihara - June 5, 2016, 12:34 p.m.
On Sat, 04 Jun 2016 11:16:47 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1465064008 25200
> #      Sat Jun 04 11:13:28 2016 -0700
> # Node ID 0910e8d232a8d28708f3a9427d60b544465ddb1c
> # Parent  48b38b16a8f83ea98ebdf0b370f59fd90dc17935
> mail: unsupport smtp.verifycert (BC)

Pushed this series to the committed repo, thanks.

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -703,23 +703,17 @@  def email(ui, repo, *revs, **opts):
                 fp.write('\n')
             except IOError as inst:
                 if inst.errno != errno.EPIPE:
                     raise
             if fp is not ui:
                 fp.close()
         else:
             if not sendmail:
-                verifycert = ui.config('smtp', 'verifycert', 'strict')
-                if opts.get('insecure'):
-                    ui.setconfig('smtp', 'verifycert', 'loose', 'patchbomb')
-                try:
-                    sendmail = mail.connect(ui, mbox=mbox)
-                finally:
-                    ui.setconfig('smtp', 'verifycert', verifycert, 'patchbomb')
+                sendmail = mail.connect(ui, mbox=mbox)
             ui.status(_('sending '), subj, ' ...\n')
             ui.progress(_('sending'), i, item=subj, total=len(msgs),
                         unit=_('emails'))
             if not mbox:
                 # Exim does not remove the Bcc field
                 del m['Bcc']
             fp = stringio()
             generator = emailmod.Generator.Generator(fp, mangle_from_=False)
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1481,26 +1481,16 @@  Configuration for extensions that need t
 ``port``
     Optional. Port to connect to on mail server. (default: 465 if
     ``tls`` is smtps; 25 otherwise)
 
 ``tls``
     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)
 
 ``password``
     Optional. Password for authenticating with the SMTP server. If not
     specified, interactive sessions will prompt the user for a
     password; non-interactive sessions will fail. (default: None)
diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -101,23 +101,16 @@  def _smtp(ui):
     # backward compatible: when tls = true, we use starttls.
     starttls = tls == 'starttls' or util.parsebool(tls)
     smtps = tls == 'smtps'
     if (starttls or smtps) and not util.safehasattr(socket, 'ssl'):
         raise error.Abort(_("can't use TLS: Python SSL support not installed"))
     mailhost = ui.config('smtp', 'host')
     if not mailhost:
         raise error.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 error.Abort(_('invalid smtp.verifycert configuration: %s')
-                             % (verifycert))
-        verifycert = False
-
     if smtps:
         ui.note(_('(using smtps)\n'))
         s = SMTPS(ui, local_hostname=local_hostname, host=mailhost)
     elif starttls:
         s = STARTTLS(ui, local_hostname=local_hostname, host=mailhost)
     else:
         s = smtplib.SMTP(local_hostname=local_hostname)
     if smtps:
@@ -128,19 +121,19 @@  def _smtp(ui):
     ui.note(_('sending mail: smtp host %s, port %d\n') %
             (mailhost, mailport))
     s.connect(host=mailhost, port=mailport)
     if starttls:
         ui.note(_('(using starttls)\n'))
         s.ehlo()
         s.starttls()
         s.ehlo()
-    if (starttls or smtps) and verifycert:
+    if starttls or smtps:
         ui.note(_('(verifying remote certificate)\n'))
-        sslutil.validatesocket(s.sock, verifycert == 'strict')
+        sslutil.validatesocket(s.sock)
     username = ui.config('smtp', 'username')
     password = ui.config('smtp', 'password')
     if username and not password:
         password = ui.getpass()
     if username and password:
         ui.note(_('(authenticating to mail server as %s)\n') %
                   (username))
         try: