Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add mtls feature to rest transport #731

Merged
merged 5 commits into from
Jan 19, 2021
Merged

feat: add mtls feature to rest transport #731

merged 5 commits into from
Jan 19, 2021

Conversation

arithmetic1728
Copy link
Collaborator

@arithmetic1728 arithmetic1728 commented Jan 12, 2021

Currently only grpc transport support mtls. This PR adds mtls support to http transport and fixes #383. The logic is the same. The only difference is we use different apis for different transports.

The following table shows the apis used for certificates.

transport get user provided cert check default cert existence get default cert add cert to channel
grpc client_options.client_cert_source SslCredentials().is_mtls SslCredentials().ssl_credentials use grpc.ssl_channel_credentials created from client_cert_source
http client_options.client_cert_source mtls.has_default_client_cert_source() mtls.default_client_cert_source() just call AuthorizedSession.configure_mtls_channel(client_cert_source) method

The following code (extracted from the PR) demonstrates the api differences.

http (new code from this PR)

client_cert_source = None
is_mtls = False
if use_client_cert:
    if client_options.client_cert_source:
        is_mtls = True
        client_cert_source = client_options.client_cert_source                
    else:
        is_mtls = mtls.has_default_client_cert_source()
        client_cert_source = mtls.default_client_cert_source() if is_mtls else None

grpc (existing code)

ssl_credentials = None
is_mtls = False
if use_client_cert:
    if client_options.client_cert_source:
        import grpc  # type: ignore
        cert, key = client_options.client_cert_source()
        ssl_credentials = grpc.ssl_channel_credentials(
            certificate_chain=cert, private_key=key
        )
        is_mtls = True
    else:
        creds = SslCredentials()
        is_mtls = creds.is_mtls
        ssl_credentials = creds.ssl_credentials if is_mtls else None

@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jan 12, 2021
@codecov
Copy link

codecov bot commented Jan 12, 2021

Codecov Report

Merging #731 (05bfcb2) into master (e17c551) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #731   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines         1619      1619           
  Branches       328       328           
=========================================
  Hits          1619      1619           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e17c551...05bfcb2. Read the comment docs.

@arithmetic1728 arithmetic1728 marked this pull request as ready for review January 12, 2021 23:23
@arithmetic1728 arithmetic1728 requested a review from a team as a code owner January 12, 2021 23:23
@yon-mg
Copy link
Contributor

yon-mg commented Jan 14, 2021

Thanks for doing this!

Copy link
Contributor

@software-dov software-dov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a minor naming nit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support mTLS for http
4 participants