Add TLS client authentication and server verification using custom CA
Review Request #11047 — Created June 18, 2020 and submitted
This patch adds two new TLS-related features.
Allow the client to provide a certificate for TLS client authentication.
Previously, connecting to a server which requires clients to authenticate using
a TLS certificate was difficult and required using external tools such as stunnel.
Requiring client authentication adds security, because it prevents unauthorized users
without a certificate to even connect to the reviewboard instance. Furthermore,
it optionally allows for using the CN as the user's identity.
After this change, the user can specify a client certificate (and key) to present
to the server at the time of connection.Allow the client to validate server certificates signed from custom CAs.
Previously, the user had to disable verification of the server certificate
when connecting to a server using a certificate signed by a custom CA.
After this change, the user can provide additional CAs that can be used to perform
server certificate validation, without disabling verification and thus providing a
more secure connection.
Minor testing using a private server signed with custom CA and requiring client certificate.
| Description | From | Last Updated |
|---|---|---|
|
This looks like a great addition! I think your description ended up being from a fixup commit. Can you flesh … |
 chipx86
|
|
|
I get this stacktrace now. How can I fix that? Traceback (most recent call last): File "/home/andre/git/ExtendedApproval/contrib/mercurial_git_push.py", line 1389, in … |
 misery
|
|
|
F401 'six.moves.http_client.UNAUTHORIZED' imported but unused |
 reviewbot
|
|
|
It was already there |
 iro
|
|
|
E501 line too long (97 > 79 characters) |
reviewbot
|
|
|
E501 line too long (103 > 79 characters) |
reviewbot
|
-
-
This looks like a great addition!
I think your description ended up being from a fixup commit. Can you flesh it out to go into some detail on this change?
There's some work that needs to be done on docstrings and code format. Do you want me to send some info your way to continue working on that, or do you want us to take this on?
- Change Summary:
-
After reading the standard library code I've greatly simplifyied the implementation
- Summary:
-
Implement TLS client authentication and custom server CAAdd TLS client authentication and server verification using custom CA
- Description:
-
~ Do not import from six.moves.http_client twice
~ This patch adds two new TLS-related features.
~ ~ ~ -
Allow the client to provide a certificate for TLS client authentication.
Previously, connecting to a server which requires clients to authenticate using
a TLS certificate was difficult and required using external tools such as stunnel.
Requiring client authentication adds security, because it prevents unauthorized users
without a certificate to even connect to the reviewboard instance. Furthermore,
it optionally allows for using the CN as the user's identity.
After this change, the user can specify a client certificate (and key) to present
to the server at the time of connection.
~ -
Allow the client to validate server certificates signed from custom CAs.
Previously, the user had to disable verification of the server certificate
when connecting to a server using a certificate signed by a custom CA.
After this change, the user can provide additional CAs that can be used to perform
server certificate validation, without disabling verification and thus providing a
more secure connection.
- Add rcfile config keys
-
- Testing Done:
-
+ Minor testing using a private server signed with custom CA and requiring client certificate.
- Commit:
-
596f1224166dd3469a83ee9712df92e164f93aee680ad79312689c26ebb778b93d292e1384775bab
Checks run (2 succeeded)
-
-
I get this stacktrace now. How can I fix that?
Traceback (most recent call last): File "/home/andre/git/ExtendedApproval/contrib/mercurial_git_push.py", line 1389, in hook return process_mercurial_hook(stdin, log) File "/home/andre/git/ExtendedApproval/contrib/mercurial_git_push.py", line 1350, in process_mercurial_hook return h.push_to_reviewboard(node) File "/home/andre/git/ExtendedApproval/contrib/mercurial_git_push.py", line 1254, in push_to_reviewboard self._set_root() File "/home/andre/git/ExtendedApproval/contrib/mercurial_git_push.py", line 1102, in _set_root raise e AttributeError: 'MercurialGitHookCmd' object has no attribute 'transport_cls'