L46: C-core: New TLS Credentials API #205

ZhenLian · 2020-09-08T06:38:14Z

No description provided.

…al-api

dfawley · 2020-09-08T16:45:33Z

Please include the language in the title for "L" proposals. Thanks!

ZhenLian · 2020-09-08T17:05:17Z

Please include the language in the title for "L" proposals. Thanks!

Sure, sounds good!

jiangtaoli2016

A few comments after quick look. Will do detailed review later today.

L46-core-tls-credential-API.md

markdroth

Please let me know if you have any questions about any of this. Thanks!

L46-core-tls-credential-API.md

markdroth · 2020-09-08T21:34:43Z

L46-core-tls-credential-API.md

+
+## Abstract
+
+This proposal aims to enhance the security features provided by [this already implemented proposal](https://github.com/grpc/proposal/pull/98). 


It looks like this proposal is designed to describe just the deltas relative to that earlier PR. However, that earlier PR was never approved or merged, which is why the existing TLS creds API is still marked experimental.

I have closed the earlier PR in favor of this one. This PR should be rewritten such that instead of referring to the earlier one, it completely replaces it. In other words, it should describe the entire TLS creds API, not just the recent changes.

+1 Agree with Mark. Let's describe the entire TLS creds API.

Sure. I revised this PR. Could you please take a look again? Thanks!

L46-core-tls-credential-API.md

yihuazhang · 2020-09-15T21:48:41Z

L46-core-tls-credential-API.md

+        - perform both the certificate and default hostname check(default option)
+        - perform certificate check but skip default hostname check(a custom authorization check is mandatory)
+        - skip both certificate check and default hostname check(a custom authorization check is mandatory)
+        - perform a custom authorization check, if needed 


It is a little confusing to have custom authorization check in both title and bullet items (client and server side).
Also since this gRFC does not propose any API to specify the first three cases (of client-side), it does not make much sense to explain them in details (sorry for the previous comment). WDYT?

Yeah, Jiangtao and Mark wanted to me write a full introduction about the TLS credential to be a replacement of our previous gRFC, so I am trying to include the all features here, no matter whether it is already implemented or not.
I do agree the current abstract contains more information than needed. Revised. Please take a look, thanks!

L46-core-tls-credential-API.md

yihuazhang · 2020-09-15T22:01:52Z

L46-core-tls-credential-API.md

+ */
+GRPCAPI int grpc_tls_credentials_options_set_server_verification_option(
+    grpc_tls_credentials_options* options,
+    grpc_tls_server_verification_option server_verification_option);


Do you plan to introduce grpc_tls_client_verification_option enum for the client-side custom authorization check?

I can do either way. Having a grpc_tls_client_verification_option is what we are doing in Go.

If server side doesn't want to verify the certificate, it can choose the corresponding GRPC_SSL_REQUEST_AND_REQUIRE_CLIENT_CERTIFICATE_BUT_DONT_VERIFY value in grpc_ssl_client_certificate_request_type, which I think is equivalent here. But I am also fine adding a grpc_tls_client_verification_option to make it (probably) easier for users to understand, etc.

+1 on having grpc_tls_client_verification_option for extendability.

L46-core-tls-credential-API.md

yihuazhang · 2020-09-15T22:22:25Z

L46-core-tls-credential-API.md

+  std::map<std::string /*cert_name*/, CertsWatching> certs_watching_;
+};
+```
+### Custom Authorization


@jiangtaoli2016 What's the plan for supporting async authorization check? The current server authorization check supports both sync and async, which is pretty complicated due to async support and goes against the goal of simple C-core API. The API's in this section can be simplified a lot if we decide to only support sync authorization check.

I think async authz check has its values, especially considering the fact that a check can take a long time and we don't want to block the main thread.
I was thinking for a long time if we can make grpc_tls_authorization_check_arg a class instead of a struct? Currently it contains a lot of information that the caller(from the wrap languages) doesn't need to know. That may simplify the logic a little bit, but I am not sure how long the refactoring would take...

If we decide to support async, we first need to flush out some details on how to wrap the C-core API's related to server authorization check (e.g., grpc_tls_authorization_check_arg and others).

OK. How about me writing a one pager briefly describe the necessary changes this would take?

Yeah, but it also needs to be signed off by TL's of each wrapped language, which may take longer than we expect. Do we have an immediate use case for async authorization check @jiangtaoli2016? If we do not, how about we start with a simple sync version in this gRFC and introduce an async version in another gRFC when its design is mature?

We need to support async authz check, e.g., if customer needs to validate the cert chain with a remote server or fetching server authz ACLs remotely. gRPC core cannot be blocked. This is a requirement from @markdroth .

Yihua: we can re-use your previous server authorization check cb API, which was reviewed by Mark. Any concern with it?

At the time I worked on async server authz check, I did not consider much about whether it can be easily wrapped by languages other than C++, while gRFC aims for the API simplicity for ALL wrapped languages. I am not sure if struct and function pointer fields in grpc_tls_authorization_check_arg can be easily wrapped.

jtattermusch · 2020-10-05T08:26:56Z

CC @jtattermusch

7hacker · 2020-10-07T18:53:12Z

Hi, this looks great, however we were wondering if this can also support TLS-PSK (Pre-Shared Keys)?

For TLS 1.3 the TLS-PSK mode enables 0-RTT handshakes, reducing overhead for resource constrained use-cases

ZhenLian · 2020-10-12T20:37:25Z

Hi, this looks great, however we were wondering if this can also support TLS-PSK (Pre-Shared Keys)?

For TLS 1.3 the TLS-PSK mode enables 0-RTT handshakes, reducing overhead for resource constrained use-cases

No, I don't think so. This API is mainly for "common" TLS with asymmetric keys.

mankhakb · 2023-03-06T04:25:15Z

Proposal - grpc library to provide support to configure TLS version at run time? Such that application can opt TLS version (1.2 or 1.3) at run time. Basically user can configure TLS version to use.

rockspore · 2023-03-06T17:19:43Z

This proposal has APIs for setting TLS version and JFYI I have grpc/grpc#31368 pending review for this.

yihuazhang and others added 15 commits September 6, 2018 13:52

propose a gRFC that creates a new TLS credential API.

20bdabc

add more explanation on server authorization check.

2825346

fixed some errors

9c4e64f

add correct suffix

b15f234

minor changes to API's

2cc5fac

updated grpc_tls_key_materials_config struct

0091a1a

Merge remote-tracking branch 'upstream/master' into grfc-tls-credenti…

1ffbebb

…al-api

remove TLS context customization from proposal

ab94624

Merge remote-tracking branch 'upstream/master' into grfc-tls-credenti…

b77fbd1

…al-api

Change to

4b53d8e

rename the proposal with L

b4e5602

Merge remote-tracking branch 'upstream/master' into grfc-tls-credenti…

4dfaa98

…al-api

Updated L# and synced with #17549

5de6a09

explicitly mention that the proposal does not work for Java and Go.

0f0c87b

first edition: redundant version

ca3e872

ZhenLian requested review from yashykt, jiangtaoli2016 and markdroth September 8, 2020 06:38

ZhenLian changed the title ~~L46: Update the TLS Credential API to Support More Use Cases~~ L46: Update the TLS Credential API to Support More Use Cases in C Core Sep 8, 2020

ZhenLian changed the title ~~L46: Update the TLS Credential API to Support More Use Cases in C Core~~ L46: Update the TLS Credential API in C Core to Support More Use Cases Sep 8, 2020

jiangtaoli2016 reviewed Sep 8, 2020

View reviewed changes

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

jiangtaoli2016 requested a review from yihuazhang September 8, 2020 21:02

markdroth mentioned this pull request Sep 8, 2020

L46: Create a new TLS credential API that supports SPIFFE mutual TLS #98

Closed

markdroth changed the title ~~L46: Update the TLS Credential API in C Core to Support More Use Cases~~ L46: C-core: New TLS Credentials API Sep 8, 2020

ZhenLian force-pushed the grfc-tls-credential-api branch from e580c31 to 59a25d1 Compare September 8, 2020 21:39

markdroth reviewed Sep 8, 2020

View reviewed changes

add APIs to select names

d7cb92b

ZhenLian force-pushed the grfc-tls-credential-api branch from 59a25d1 to d7cb92b Compare September 8, 2020 21:53

ZhenLian force-pushed the grfc-tls-credential-api branch from 3e80a61 to 2e257bd Compare September 11, 2020 06:20

small fix

53440db

yihuazhang reviewed Sep 15, 2020

View reviewed changes

L46-core-tls-credential-API.md Show resolved Hide resolved

yihuazhang reviewed Sep 15, 2020

View reviewed changes

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

yihuazhang reviewed Sep 15, 2020

View reviewed changes

L46-core-tls-credential-API.md Outdated Show resolved Hide resolved

yihuazhang reviewed Sep 15, 2020

View reviewed changes

small changes as of 09/16

ad7d3b0

jiangtaoli2016 mentioned this pull request Sep 21, 2020

Enable TLS 1.3 in gRPC core grpc/grpc#22304

Closed

jiangtaoli2016 mentioned this pull request Oct 2, 2020

TLS-PSK grpc/grpc#24238

Closed

jtattermusch mentioned this pull request Oct 27, 2020

Add StaticDataCertificateProvider for TLS credentials grpc/grpc#24227

Merged

This was referenced Nov 20, 2020

Add FileWatcher CertificateProvider grpc/grpc#24600

Merged

Certificate validation occurs on every request grpc/grpc#23498

Closed

ZhenLian mentioned this pull request Dec 4, 2020

TlsServerAuthorizationCheckConfig may cause SIGSEGV grpc/grpc#24684

Closed

ZhenLian mentioned this pull request Jan 6, 2021

Ability to override TLS certificate name check in C++ library grpc/grpc#23278

Closed

ZhenLian mentioned this pull request Mar 13, 2021

assert in check_ssl_ctx_x509_method when using TlsCredentialsOptions to reload server certificate grpc/grpc#25688

Closed

BusyJay mentioned this pull request Jun 11, 2021

Set TLS versions to enable tikv/grpc-rs#533

Open

ZhenLian mentioned this pull request Aug 16, 2021

Make verify_peer_callback more powerful grpc/grpc#21588

Closed

markdroth mentioned this pull request Sep 2, 2021

L83: TLS Session Key Export in gRPC C++ #252

Open

jairaju mentioned this pull request Dec 17, 2021

cmake does not seem to install ssl option specific header files to set tls_min and tls_max for TLS version grpc/grpc#28383

Closed

sanchayanghosh mentioned this pull request Feb 23, 2022

Cannot force use of only TLS 1.3 ciphers grpc/grpc#28382

Closed

rockspore mentioned this pull request Mar 29, 2023

Add TLS verify callback for Python grpc/grpc#32635

Open

gtcooke94 mentioned this pull request Mar 22, 2024

L46: C-core: New TLS Credentials API #422

Open

gtcooke94 closed this Mar 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

L46: C-core: New TLS Credentials API #205

L46: C-core: New TLS Credentials API #205

ZhenLian commented Sep 8, 2020

dfawley commented Sep 8, 2020

ZhenLian commented Sep 8, 2020

jiangtaoli2016 left a comment

markdroth left a comment

markdroth Sep 8, 2020

jiangtaoli2016 Sep 8, 2020

ZhenLian Sep 10, 2020

yihuazhang Sep 15, 2020

ZhenLian Sep 16, 2020

yihuazhang Sep 15, 2020

ZhenLian Sep 16, 2020

yihuazhang Sep 17, 2020

yihuazhang Sep 15, 2020

ZhenLian Sep 16, 2020

yihuazhang Sep 17, 2020

ZhenLian Sep 17, 2020

yihuazhang Sep 17, 2020

jiangtaoli2016 Sep 17, 2020

yihuazhang Sep 17, 2020

jtattermusch commented Oct 5, 2020

7hacker commented Oct 7, 2020

ZhenLian commented Oct 12, 2020

mankhakb commented Mar 6, 2023

rockspore commented Mar 6, 2023


		## Abstract

		This proposal aims to enhance the security features provided by [this already implemented proposal](https://github.com/grpc/proposal/pull/98).

L46: C-core: New TLS Credentials API #205

L46: C-core: New TLS Credentials API #205

Conversation

ZhenLian commented Sep 8, 2020

dfawley commented Sep 8, 2020

ZhenLian commented Sep 8, 2020

jiangtaoli2016 left a comment

Choose a reason for hiding this comment

markdroth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtattermusch commented Oct 5, 2020

7hacker commented Oct 7, 2020

ZhenLian commented Oct 12, 2020

mankhakb commented Mar 6, 2023

rockspore commented Mar 6, 2023