Patchwork [01,of,11,V4] hgweb: pass ui into preparehttpserver

login
register
mail settings
Submitter Gregory Szorc
Date July 15, 2016, 4:09 a.m.
Message ID <9e91be071422676679cd.1468555742@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15865/
State Superseded
Headers show

Comments

Gregory Szorc - July 15, 2016, 4:09 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1468394090 25200
#      Wed Jul 13 00:14:50 2016 -0700
# Node ID 9e91be071422676679cdef44e74f4ea34dd81be1
# Parent  9d02bed8477bec7f679d6aeb5b1dd8bcdb80f64d
hgweb: pass ui into preparehttpserver

Upcoming patches will need the built-in HTTPS server to be more
configurable.
Gregory Szorc - July 15, 2016, 4:16 a.m.
This series includes patches that have already landed on the committed
repo. The first two patches shouldn't be controversial. They are included
in case it is decided to wipe out all my draft changesets currently on the
committed repo.

Patches 3 to 7 are a refactor what has already landed on the committed repo
to avoid create_default_context. They should be an easy review.

Patches 8 and 9 seem to have already had eyeballs on them and should
hopefully sail through pretty easy. I think I fixed Pierre-Yves's reported
test failure by reordering tests to avoid killing tinyproxy.py.

Patches 10 and 11 are a bit more controversial. They are purposefully at
the end of the series because they are the least reviewed and they may get
dropped if we're not willing to make the commitment to dropping TLS 1.0.

After this series, I'm considering my work on overhauling the security code
complete. Just in time for the 3.9 freeze too...

On Thu, Jul 14, 2016 at 9:09 PM, Gregory Szorc <gregory.szorc@gmail.com>
wrote:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1468394090 25200
> #      Wed Jul 13 00:14:50 2016 -0700
> # Node ID 9e91be071422676679cdef44e74f4ea34dd81be1
> # Parent  9d02bed8477bec7f679d6aeb5b1dd8bcdb80f64d
> hgweb: pass ui into preparehttpserver
>
> Upcoming patches will need the built-in HTTPS server to be more
> configurable.
>
> diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
> --- a/mercurial/hgweb/server.py
> +++ b/mercurial/hgweb/server.py
> @@ -53,17 +53,17 @@ class _error_logger(object):
>          for msg in seq:
>              self.handler.log_error("HG error:  %s", msg)
>
>  class _httprequesthandler(BaseHTTPServer.BaseHTTPRequestHandler):
>
>      url_scheme = 'http'
>
>      @staticmethod
> -    def preparehttpserver(httpserver, ssl_cert):
> +    def preparehttpserver(httpserver, ui):
>          """Prepare .socket of new HTTPServer instance"""
>          pass
>
>      def __init__(self, *args, **kargs):
>          self.protocol_version = 'HTTP/1.1'
>          BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args,
> **kargs)
>
>      def _log_any(self, fp, format, *args):
> @@ -217,25 +217,27 @@ class _httprequesthandler(BaseHTTPServer
>              self.wfile.flush()
>
>  class _httprequesthandlerssl(_httprequesthandler):
>      """HTTPS handler based on Python's ssl module"""
>
>      url_scheme = 'https'
>
>      @staticmethod
> -    def preparehttpserver(httpserver, ssl_cert):
> +    def preparehttpserver(httpserver, ui):
>          try:
>              import ssl
>              ssl.wrap_socket
>          except ImportError:
>              raise error.Abort(_("SSL support is unavailable"))
> +
> +        certfile = ui.config('web', 'certificate')
>          httpserver.socket = ssl.wrap_socket(
>              httpserver.socket, server_side=True,
> -            certfile=ssl_cert, ssl_version=ssl.PROTOCOL_TLSv1)
> +            certfile=certfile, ssl_version=ssl.PROTOCOL_TLSv1)
>
>      def setup(self):
>          self.connection = self.request
>          self.rfile = socket._fileobject(self.request, "rb", self.rbufsize)
>          self.wfile = socket._fileobject(self.request, "wb", self.wbufsize)
>
>  try:
>      import threading
> @@ -259,17 +261,17 @@ class MercurialHTTPServer(object, _mixin
>      if os.name == 'nt':
>          allow_reuse_address = 0
>
>      def __init__(self, ui, app, addr, handler, **kwargs):
>          BaseHTTPServer.HTTPServer.__init__(self, addr, handler, **kwargs)
>          self.daemon_threads = True
>          self.application = app
>
> -        handler.preparehttpserver(self, ui.config('web', 'certificate'))
> +        handler.preparehttpserver(self, ui)
>
>          prefix = ui.config('web', 'prefix', '')
>          if prefix:
>              prefix = '/' + prefix.strip('/')
>          self.prefix = prefix
>
>          alog = openlog(ui.config('web', 'accesslog', '-'), sys.stdout)
>          elog = openlog(ui.config('web', 'errorlog', '-'), sys.stderr)
> diff --git a/tests/test-https.t b/tests/test-https.t
> --- a/tests/test-https.t
> +++ b/tests/test-https.t
> @@ -399,22 +399,23 @@ Test https with cert problems through pr
>
>  Start patched hgweb that requires client certificates:
>
>    $ cat << EOT > reqclientcert.py
>    > import ssl
>    > from mercurial.hgweb import server
>    > class _httprequesthandlersslclientcert(server._httprequesthandlerssl):
>    >     @staticmethod
> -  >     def preparehttpserver(httpserver, ssl_cert):
> +  >     def preparehttpserver(httpserver, ui):
> +  >         certfile = ui.config('web', 'certificate')
>    >         sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
>    >         sslcontext.verify_mode = ssl.CERT_REQUIRED
> -  >         sslcontext.load_cert_chain(ssl_cert)
> +  >         sslcontext.load_cert_chain(certfile)
>    >         # verify clients by server certificate
> -  >         sslcontext.load_verify_locations(ssl_cert)
> +  >         sslcontext.load_verify_locations(certfile)
>    >         httpserver.socket = sslcontext.wrap_socket(httpserver.socket,
>    >                                                    server_side=True)
>    > server._httprequesthandlerssl = _httprequesthandlersslclientcert
>    > EOT
>    $ cd test
>    $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
>    > --config extensions.reqclientcert=../reqclientcert.py
>    $ cat ../hg0.pid >> $DAEMON_PIDS
>
Yuya Nishihara - July 15, 2016, 1:40 p.m.
On Thu, 14 Jul 2016 21:16:51 -0700, Gregory Szorc wrote:
> This series includes patches that have already landed on the committed
> repo. The first two patches shouldn't be controversial. They are included
> in case it is decided to wipe out all my draft changesets currently on the
> committed repo.
> 
> Patches 3 to 7 are a refactor what has already landed on the committed repo
> to avoid create_default_context. They should be an easy review.

The patch 1-4 look good to me. The patch 5 looks okay, but I slightly prefer
the previous version if we're sure we can use create_default_context() for
CLIENT_AUTH.

I've marked these patches as "2nd review requested." I can't do bulk rebasing
right now since I'm unsure about the patch 5, and I except it will be settled
while I'm in bed.

I haven't read the other patches yet.

Patch

diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
--- a/mercurial/hgweb/server.py
+++ b/mercurial/hgweb/server.py
@@ -53,17 +53,17 @@  class _error_logger(object):
         for msg in seq:
             self.handler.log_error("HG error:  %s", msg)
 
 class _httprequesthandler(BaseHTTPServer.BaseHTTPRequestHandler):
 
     url_scheme = 'http'
 
     @staticmethod
-    def preparehttpserver(httpserver, ssl_cert):
+    def preparehttpserver(httpserver, ui):
         """Prepare .socket of new HTTPServer instance"""
         pass
 
     def __init__(self, *args, **kargs):
         self.protocol_version = 'HTTP/1.1'
         BaseHTTPServer.BaseHTTPRequestHandler.__init__(self, *args, **kargs)
 
     def _log_any(self, fp, format, *args):
@@ -217,25 +217,27 @@  class _httprequesthandler(BaseHTTPServer
             self.wfile.flush()
 
 class _httprequesthandlerssl(_httprequesthandler):
     """HTTPS handler based on Python's ssl module"""
 
     url_scheme = 'https'
 
     @staticmethod
-    def preparehttpserver(httpserver, ssl_cert):
+    def preparehttpserver(httpserver, ui):
         try:
             import ssl
             ssl.wrap_socket
         except ImportError:
             raise error.Abort(_("SSL support is unavailable"))
+
+        certfile = ui.config('web', 'certificate')
         httpserver.socket = ssl.wrap_socket(
             httpserver.socket, server_side=True,
-            certfile=ssl_cert, ssl_version=ssl.PROTOCOL_TLSv1)
+            certfile=certfile, ssl_version=ssl.PROTOCOL_TLSv1)
 
     def setup(self):
         self.connection = self.request
         self.rfile = socket._fileobject(self.request, "rb", self.rbufsize)
         self.wfile = socket._fileobject(self.request, "wb", self.wbufsize)
 
 try:
     import threading
@@ -259,17 +261,17 @@  class MercurialHTTPServer(object, _mixin
     if os.name == 'nt':
         allow_reuse_address = 0
 
     def __init__(self, ui, app, addr, handler, **kwargs):
         BaseHTTPServer.HTTPServer.__init__(self, addr, handler, **kwargs)
         self.daemon_threads = True
         self.application = app
 
-        handler.preparehttpserver(self, ui.config('web', 'certificate'))
+        handler.preparehttpserver(self, ui)
 
         prefix = ui.config('web', 'prefix', '')
         if prefix:
             prefix = '/' + prefix.strip('/')
         self.prefix = prefix
 
         alog = openlog(ui.config('web', 'accesslog', '-'), sys.stdout)
         elog = openlog(ui.config('web', 'errorlog', '-'), sys.stderr)
diff --git a/tests/test-https.t b/tests/test-https.t
--- a/tests/test-https.t
+++ b/tests/test-https.t
@@ -399,22 +399,23 @@  Test https with cert problems through pr
 
 Start patched hgweb that requires client certificates:
 
   $ cat << EOT > reqclientcert.py
   > import ssl
   > from mercurial.hgweb import server
   > class _httprequesthandlersslclientcert(server._httprequesthandlerssl):
   >     @staticmethod
-  >     def preparehttpserver(httpserver, ssl_cert):
+  >     def preparehttpserver(httpserver, ui):
+  >         certfile = ui.config('web', 'certificate')
   >         sslcontext = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
   >         sslcontext.verify_mode = ssl.CERT_REQUIRED
-  >         sslcontext.load_cert_chain(ssl_cert)
+  >         sslcontext.load_cert_chain(certfile)
   >         # verify clients by server certificate
-  >         sslcontext.load_verify_locations(ssl_cert)
+  >         sslcontext.load_verify_locations(certfile)
   >         httpserver.socket = sslcontext.wrap_socket(httpserver.socket,
   >                                                    server_side=True)
   > server._httprequesthandlerssl = _httprequesthandlersslclientcert
   > EOT
   $ cd test
   $ hg serve -p $HGPORT -d --pid-file=../hg0.pid --certificate=$PRIV \
   > --config extensions.reqclientcert=../reqclientcert.py
   $ cat ../hg0.pid >> $DAEMON_PIDS