cert validation: use Android cert validation APIs #2525
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| load( | ||
| "@envoy//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_extension", | ||
| "envoy_cc_library", | ||
| "envoy_extension_package", | ||
| ) | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| envoy_extension_package() | ||
|
|
||
| envoy_cc_library( | ||
| name = "c_types_lib", | ||
| hdrs = ["c_types.h"], | ||
| repository = "@envoy", | ||
| deps = [ | ||
| "//library/common/data:utility_lib", | ||
| ], | ||
| ) | ||
|
|
||
| envoy_cc_library( | ||
| name = "platform_bridge_cert_validator_lib", | ||
| srcs = ["platform_bridge_cert_validator.cc"], | ||
| hdrs = [ | ||
| "platform_bridge_cert_validator.h", | ||
| ], | ||
| repository = "@envoy", | ||
| deps = [ | ||
| ":c_types_lib", | ||
| "@envoy//source/extensions/transport_sockets/tls/cert_validator:cert_validator_lib", | ||
| ], | ||
| ) | ||
|
|
||
| envoy_cc_extension( | ||
| name = "config", | ||
| srcs = ["config.cc"], | ||
| hdrs = [ | ||
| "c_types.h", | ||
| "config.h", | ||
| ], | ||
| repository = "@envoy", | ||
| deps = [ | ||
| ":platform_bridge_cert_validator_lib", | ||
| "//library/common/api:external_api_lib", | ||
| "//library/common/data:utility_lib", | ||
| "@envoy//envoy/registry", | ||
| ], | ||
| ) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| #pragma once | ||
|
|
||
| #include "library/common/types/c_types.h" | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| /** | ||
| * Certification validation binary result with corresponding boring SSL alert | ||
| * and error details if the result indicates failure. | ||
| */ | ||
| typedef struct { | ||
|
There was a problem hiding this comment. Comments on all the structs/functions, please |
||
| envoy_status_t result; | ||
| uint8_t tls_alert; | ||
| const char* error_details; | ||
| } envoy_cert_validation_result; | ||
|
|
||
| #ifdef __cplusplus | ||
|
There was a problem hiding this comment. when would we expect this to be false? Asking mostly so that I understand why it's needed. There was a problem hiding this comment. I actually don't know, but I saw this has been used in all c_types.h to wrap the C interface. There was a problem hiding this comment. It's necessary because the public headers are both valid C++ and pure C headers. When called publicly (e.g. from the built-in iOS and Android platform layers), they're bound as C functions. When the core layer is compiled though, everything is C++. |
||
| extern "C" { // function pointers | ||
| #endif | ||
|
|
||
| /** | ||
| * Function signature for calling into platform APIs to validate certificates. | ||
| */ | ||
| typedef envoy_cert_validation_result (*envoy_validate_cert_f)(const envoy_data* certs, uint8_t size, | ||
| const char* host_name); | ||
|
|
||
| /** | ||
| * Function signature for calling into platform APIs to clean up after validation completion. | ||
| */ | ||
| typedef void (*envoy_release_validator_f)(); | ||
|
|
||
| #ifdef __cplusplus | ||
| } // function pointers | ||
| #endif | ||
|
|
||
| /** | ||
| * A bag of function pointers to be registered in the platform registry. | ||
| */ | ||
| typedef struct { | ||
| envoy_validate_cert_f validate_cert; | ||
| envoy_release_validator_f release_validator; | ||
| } envoy_cert_validator; | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #include "library/common/extensions/cert_validator/platform_bridge/config.h" | ||
|
|
||
| #include "library/common/api/external.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Extensions { | ||
| namespace TransportSockets { | ||
| namespace Tls { | ||
|
|
||
| CertValidatorPtr PlatformBridgeCertValidatorFactory::createCertValidator( | ||
| const Envoy::Ssl::CertificateValidationContextConfig* config, SslStats& stats, | ||
| TimeSource& /*time_source*/) { | ||
| platform_validator_ = | ||
| static_cast<envoy_cert_validator*>(Api::External::retrieveApi("platform_cert_validator")); | ||
|
There was a problem hiding this comment. Considering you may want your own guard here to ensure that the implementation is found, just for safety.There was a problem hiding this comment. This would be the bad config case. Would it make sense to throw or PANIC as this is still during start up? |
||
| return std::make_unique<PlatformBridgeCertValidator>(config, stats, platform_validator_); | ||
| } | ||
|
|
||
| REGISTER_FACTORY(PlatformBridgeCertValidatorFactory, CertValidatorFactory); | ||
|
|
||
| } // namespace Tls | ||
| } // namespace TransportSockets | ||
| } // namespace Extensions | ||
| } // namespace Envoy | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if we enable this feature on iOS? Will Envoy work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that I did not suggest the use of
if defined(__ANDROID_API__)even thought that would be probably more correct technically speaking. That's because it would make testing of this code harder than necessary (see #2450).If you agree with a proposed change, we can make it and drop a TODO (you can assign to me) for changing
#if defined(__APPLE__)todefined(__ANDROID_API__)once is addressed #2450There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that I didn't even register the platform API in this engine. Adding it right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I moved the API registration from JniLibrary interface to JNI_OnLoad() which is also called when using C++ engine.