[TLS - Revocation] Crl Provider#33786
Conversation
markdroth
left a comment
There was a problem hiding this comment.
This looks like a good start!
Please let me know if you have any questions.
| using grpc_core::experimental::Crl; // NOLINT(misc-unused-using-decls) | ||
| using grpc_core::experimental::CrlProvider; // NOLINT(misc-unused-using-decls) | ||
| using grpc_core::experimental:: | ||
| grpc_tls_credentials_options_set_crl_provider; // NOLINT(misc-unused-using-decls) |
There was a problem hiding this comment.
We definitely do not want this using statement. The grpc_tls_credentials_options_set_crl_provider() method should not be in the grpc_core::experimental namespace to begin with; it should be at the top level.
There was a problem hiding this comment.
I've taken it out now
…h flattening in CI
…sues with flattening in CI" This reverts commit d3af0b0.
test/cpp/end2end/BUILD
Outdated
| "//src/core/tsi/test_creds:ca.pem", | ||
| "//src/core/tsi/test_creds:client.key", | ||
| "//src/core/tsi/test_creds:client.pem", | ||
| "//src/core/tsi/test_creds:server1.key", | ||
| "//src/core/tsi/test_creds:server1.pem", | ||
| "//test/core/tsi/test_creds/crl_data:ca.pem", | ||
| "//test/core/tsi/test_creds/crl_data:intermediate_ca.key", | ||
| "//test/core/tsi/test_creds/crl_data:intermediate_ca.pem", | ||
| "//test/core/tsi/test_creds/crl_data:leaf_and_intermediate_chain.pem", | ||
| "//test/core/tsi/test_creds/crl_data:leaf_signed_by_intermediate.key", | ||
| "//test/core/tsi/test_creds/crl_data:leaf_signed_by_intermediate.pem", | ||
| "//test/core/tsi/test_creds/crl_data:revoked.key", | ||
| "//test/core/tsi/test_creds/crl_data:revoked.pem", | ||
| "//test/core/tsi/test_creds/crl_data:valid.key", | ||
| "//test/core/tsi/test_creds/crl_data:valid.pem", | ||
| "//test/core/tsi/test_creds/crl_data/crls:ab06acdd.r0", | ||
| "//test/core/tsi/test_creds/crl_data/crls:b9322cac.r0", | ||
| "//test/core/tsi/test_creds/crl_data/crls:current.crl", | ||
| "//test/core/tsi/test_creds/crl_data/crls:intermediate.crl", | ||
| "//test/core/tsi/test_creds/crl_data/crls_missing_intermediate:ab06acdd.r0", | ||
| "//test/core/tsi/test_creds/crl_data/crls_missing_root:b9322cac.r0", |
There was a problem hiding this comment.
Looks like file path is not supported for ios rules.
A quick fix would be only add files that are actually used in your test, looks like they passes with fe118ac in #34707 which I only removed the ones that conflicts. From what I see in you test, only 6 of those files are used.
--
@sampajano We probably can just remove job grpc_bazel_cpp_ios_event_engine_experiment_tests.cfg as per our previous discussion, 0 test was actually executed on iOS.
There was a problem hiding this comment.
That'll work for this PR, but there's some coming work where directory structure for CRLs matters - I'm not 100% sure what this is doing with the flattening and bundling, but I suppose we'll just figure it out as it happens?
There was a problem hiding this comment.
@HannahShiSFB I'm not against removing grpc_bazel_cpp_ios_event_engine_experiment_tests.cfg since the tests are not really effective :)
This reverts commit 0f0396a.
This reverts commit 7af5efc.
The basic APIs for the CRL Reloading features.
This adds external types to represent CRL Providers, CRLs, and CertificateInfo.
Internally we will use
CrlImpl- this layer is needed to hide OpenSSL details from the user.TODO - link to associated gRFC.
Things Done
CrlProvider,Crl,CertInfo(CertInfois used during CRL lookup rather than passing the entire certificate).ssl_transport_securityto utilize CRL providersStaticCrlProvidercrl_ssl_transport_security_test.ccso it is more extensible and can be used with providers