bpo-40990: Add HTTPS support to http.server.HTTPServer#20923
Uh oh!
There was an error while loading. Please reload this page.
Conversation
tiran left a comment
tiran left a comment There was a problem hiding this comment.
- ssl module is optional and may not be available. The http.server and its tests should still work when ssl is not present.
- You have to deal with ALPN correctly and refuse connections when ALPN is present and does not contain correct protocol indicator
- keys are usually protected and wrapped with PKCS#8 PBE2
tlsis technically correct but we don't use it in arguments yet- SSLSocket may raise exceptions that are not yet handled by HTTPServer
- HTTPSServer should be a separate class
| .. class:: HTTPServer(server_address, RequestHandlerClass, \ | ||
| bind_and_activate=True, *, tls=None) |
There was a problem hiding this comment.
This should be a separate class, not an argument.
| importsocket# For gethostbyaddr() | ||
| importsocketserver | ||
| importsys | ||
| importssl |
There was a problem hiding this comment.
ssl support is optional, you have to wrap the import in a try/except block.
| """Wrap the socket in SSLSocket if TLS is enabled""" | ||
| super().server_activate() | ||
| ifself.tls_certandself.tls_key: | ||
| context=ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) |
There was a problem hiding this comment.
The default SSL context requires a TLS 1.3 certificate so it doesn't work with ssl_cert.pem:
ssl.SSLError: [SSL: TLSV13_ALERT_CERTIFICATE_REQUIRED] tlsv13 alert certificate required Is this something that we want to force for a development server?
There was a problem hiding this comment.
There is no such thing as a TLS 1.3 certificate. This is a TLS alert with message certificate required. Looks like the certificate does not have correct extensions to handle TLS 1.3.
You cannot use any file from test directory any way. The directory is often not installed or shipped to reduce disk space usage in containers or on small systems.
| def__init__(self, server_address, RequestHandlerClass, | ||
| bind_and_activate=True, *, tls=None): | ||
| iftlsisNone: | ||
| self.tls_cert=self.tls_key=None |
There was a problem hiding this comment.
It's not a cert file, it's a cert chain file.
| parser.add_argument('--tls-cert', | ||
| help='Specify the path to a TLS certificate') |
| elifnotargs.tls_certornotargs.tls_key: | ||
| parser.error('Both --tls-cert and --tls-key must be provided to enable TLS') | ||
| else: | ||
| tls= (args.tls_cert, args.tls_key) |
There was a problem hiding this comment.
It's common to have the private key and the cert chain in the same file. The key argument should be optional.
| importhtml | ||
| importhttp.client | ||
| importurllib.parse | ||
| importssl |
| # BaseHTTPServerTestCase. | ||
| # We have to use the correct path from the folder created by regtest | ||
| tls= ('../../Lib/test/ssl_cert.pem', '../../Lib/test/ssl_key.pem') |
There was a problem hiding this comment.
os.path.join(os.path.dirname(__file__), 'ssl_cert.pem')
| self.assertEqual(b'', data) | ||
| classBaseHTTPSServerTestCase(BaseTestCase): |
There was a problem hiding this comment.
Skip the test if ssl module is not supported
bedevere-bot commented Jun 17, 2020
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
jaswdr commented May 12, 2021
@remilapeyre I'm happy to continue this PR if you are not able to continue it |
remilapeyre commented May 19, 2021
Hi @jaswdr, I've been quite busy but should be able to finish it this week :) |
| .. warning:: | ||
| The HTTPS support is for development and test puposes and must not be used |
gpshead commented Feb 3, 2025
closing this PR as abandoned as there's a new one up. |
8 participants
https://bugs.python.org/issue40990