| Submitter | Gregory Szorc |
|---|---|
| Date | April 10, 2016, 6:04 p.m. |
| Message ID | <3d9c3f8bfed01b5ab163.1460311476@ubuntu-vm-main> |
| Download | mbox | patch |
| Permalink | /patch/14490/ |
| State | Accepted |
| Headers | show |
Comments
On Sun, 10 Apr 2016 11:04:36 -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1460311241 25200 > # Sun Apr 10 11:00:41 2016 -0700 > # Node ID 3d9c3f8bfed01b5ab163bc95180576a79e6016ef > # Parent a0c629f58c3ed8acfa008a12415c172f315c70d3 > sslutil: require a server hostname when wrapping sockets (API) > > All callers appear to be passing the hostname. So this shouldn't > break anything. By specifying the hostname, more validation options > from the ssl module are available to us. Although this patch stops > short of using them. > > diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py > --- a/mercurial/sslutil.py > +++ b/mercurial/sslutil.py > @@ -115,16 +115,19 @@ def wrapsocket(sock, keyfile, certfile, > > In addition to the arguments supported by ``ssl.wrap_socket``, we allow > the following additional arguments: > > * serverhostname - The expected hostname of the remote server. If the > server (and client) support SNI, this tells the server which certificate > to use. > """ > + if not serverhostname: > + raise error.Abort('serverhostname argument required') % hg --config smtp.verifycert=false email tip --trace Traceback (most recent call last): File "mercurial/dispatch.py", line 204, in _runcatch return _dispatch(req) File "mercurial/dispatch.py", line 887, in _dispatch cmdpats, cmdoptions) File "mercurial/dispatch.py", line 632, in runcommand ret = _runcommand(ui, options, cmd, d) File "mercurial/extensions.py", line 204, in closure return func(*(args + a), **kw) File "hgext/pager.py", line 160, in pagecmd return orig(ui, options, cmd, cmdfunc) File "mercurial/extensions.py", line 204, in closure return func(*(args + a), **kw) File "hgext/color.py", line 494, in colorcmd return orig(ui_, opts, cmd, cmdfunc) File "mercurial/dispatch.py", line 1018, in _runcommand return checkargs() File "mercurial/dispatch.py", line 978, in checkargs return cmdfunc() File "mercurial/dispatch.py", line 884, in <lambda> d = lambda: util.checksignature(func)(ui, *args, **cmdoptions) File "mercurial/util.py", line 1000, in check return func(*args, **kwargs) File "mercurial/extensions.py", line 204, in closure return func(*(args + a), **kw) File "mercurial/util.py", line 1000, in check return func(*args, **kwargs) File "hgext/mq.py", line 3519, in mqcommand return orig(ui, repo, *args, **kwargs) File "mercurial/util.py", line 1000, in check return func(*args, **kwargs) File "hgext/patchbomb.py", line 715, in email sendmail = mail.connect(ui, mbox=mbox) File "mercurial/mail.py", line 193, in connect return _smtp(ui) File "mercurial/mail.py", line 133, in _smtp s.starttls() File "mercurial/mail.py", line 62, in starttls **self._sslkwargs) File "mercurial/sslutil.py", line 124, in wrapsocket raise error.Abort('serverhostname argument required') Abort: serverhostname argument required abort: serverhostname argument required Cheers,
On Mon, Apr 11, 2016 at 4:22 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Sun, 10 Apr 2016 11:04:36 -0700, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc <gregory.szorc@gmail.com> > > # Date 1460311241 25200 > > # Sun Apr 10 11:00:41 2016 -0700 > > # Node ID 3d9c3f8bfed01b5ab163bc95180576a79e6016ef > > # Parent a0c629f58c3ed8acfa008a12415c172f315c70d3 > > sslutil: require a server hostname when wrapping sockets (API) > > > > All callers appear to be passing the hostname. So this shouldn't > > break anything. By specifying the hostname, more validation options > > from the ssl module are available to us. Although this patch stops > > short of using them. > > > > diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py > > --- a/mercurial/sslutil.py > > +++ b/mercurial/sslutil.py > > @@ -115,16 +115,19 @@ def wrapsocket(sock, keyfile, certfile, > > > > In addition to the arguments supported by ``ssl.wrap_socket``, we > allow > > the following additional arguments: > > > > * serverhostname - The expected hostname of the remote server. If > the > > server (and client) support SNI, this tells the server which > certificate > > to use. > > """ > > + if not serverhostname: > > + raise error.Abort('serverhostname argument required') > > % hg --config smtp.verifycert=false email tip --trace > Traceback (most recent call last): > File "mercurial/dispatch.py", line 204, in _runcatch > return _dispatch(req) > File "mercurial/dispatch.py", line 887, in _dispatch > cmdpats, cmdoptions) > File "mercurial/dispatch.py", line 632, in runcommand > ret = _runcommand(ui, options, cmd, d) > File "mercurial/extensions.py", line 204, in closure > return func(*(args + a), **kw) > File "hgext/pager.py", line 160, in pagecmd > return orig(ui, options, cmd, cmdfunc) > File "mercurial/extensions.py", line 204, in closure > return func(*(args + a), **kw) > File "hgext/color.py", line 494, in colorcmd > return orig(ui_, opts, cmd, cmdfunc) > File "mercurial/dispatch.py", line 1018, in _runcommand > return checkargs() > File "mercurial/dispatch.py", line 978, in checkargs > return cmdfunc() > File "mercurial/dispatch.py", line 884, in <lambda> > d = lambda: util.checksignature(func)(ui, *args, **cmdoptions) > File "mercurial/util.py", line 1000, in check > return func(*args, **kwargs) > File "mercurial/extensions.py", line 204, in closure > return func(*(args + a), **kw) > File "mercurial/util.py", line 1000, in check > return func(*args, **kwargs) > File "hgext/mq.py", line 3519, in mqcommand > return orig(ui, repo, *args, **kwargs) > File "mercurial/util.py", line 1000, in check > return func(*args, **kwargs) > File "hgext/patchbomb.py", line 715, in email > sendmail = mail.connect(ui, mbox=mbox) > File "mercurial/mail.py", line 193, in connect > return _smtp(ui) > File "mercurial/mail.py", line 133, in _smtp > s.starttls() > File "mercurial/mail.py", line 62, in starttls > **self._sslkwargs) > File "mercurial/sslutil.py", line 124, in wrapsocket > raise error.Abort('serverhostname argument required') > Abort: serverhostname argument required > abort: serverhostname argument required > This got filed as 5203 and timeless submitted a patch the other day before I had a chance to fix it. Thank you, timeless!
Patch
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py --- a/mercurial/sslutil.py +++ b/mercurial/sslutil.py @@ -115,16 +115,19 @@ def wrapsocket(sock, keyfile, certfile, In addition to the arguments supported by ``ssl.wrap_socket``, we allow the following additional arguments: * serverhostname - The expected hostname of the remote server. If the server (and client) support SNI, this tells the server which certificate to use. """ + if not serverhostname: + raise error.Abort('serverhostname argument required') + # Despite its name, PROTOCOL_SSLv23 selects the highest protocol # that both ends support, including TLS protocols. On legacy stacks, # the highest it likely goes in TLS 1.0. On modern stacks, it can # support TLS 1.2. # # The PROTOCOL_TLSv* constants select a specific TLS version # only (as opposed to multiple versions). So the method for # supporting multiple TLS versions is to use PROTOCOL_SSLv23 and