sds: add validation callback for all secret providers#8310
Merged
lizan merged 4 commits intoenvoyproxy:masterfrom Sep 26, 2019
Merged
sds: add validation callback for all secret providers#8310lizan merged 4 commits intoenvoyproxy:masterfrom
lizan merged 4 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
…fig provider Signed-off-by: Asra Ali <asraa@google.com>
Contributor
Author
Signed-off-by: Asra Ali <asraa@google.com>
JimmyCYJ
reviewed
Sep 24, 2019
| * @return CallbackHandle the handle which can remove that validation callback. | ||
| */ | ||
| virtual Common::CallbackHandle* addValidationCallback( | ||
| std::function<void(const envoy::api::v2::auth::CertificateValidationContext&)> callback) PURE; |
Member
There was a problem hiding this comment.
Why is it envoy::api::v2::auth::CertificateValidationContext, not SecretType? This SecretProvider only fetches secret of a specified SecretType, it will call the callback the verify if that received secret could be initialized successfully. @lizan
Signed-off-by: Asra Ali <asraa@google.com>
Contributor
Author
|
/retest |
|
🤷♀️ nothing to rebuild. |
JimmyCYJ
reviewed
Sep 26, 2019
Member
JimmyCYJ
left a comment
There was a problem hiding this comment.
/lgtm
I am curious, is this file test/server/server_corpus/clusterfuzz-testcase-config_fuzz_test-5729922022113280 auto generated by some tools?
lizan
approved these changes
Sep 26, 2019
Member
|
@JimmyCYJ yes it is generated by oss-fuzz |
danzh2010
pushed a commit
to danzh2010/envoy
that referenced
this pull request
Oct 4, 2019
This adds an implementation of addValidationCallback for all secret providers. This is a no-op, unless the secret provider is a `CertificateValidationContextSdsApi. Envoy crashes when trying to add a validation callback to CertificateValidationContextProvider due to a bad dynamic cast. When certificate_validation_context_provider_ isn't a CertificateValidationContextSdsApi, we no-op. Fixes: envoyproxy#7892 and OSS-Fuzz Issues https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17001 https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=16766 Testing: add corpus entry Signed-off-by: Asra Ali <asraa@google.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This adds an implementation of
addValidationCallbackfor all secret providers. This is a no-op, unless the secret provider is a `CertificateValidationContextSdsApi.Envoy crashes when trying to add a validation callback to CertificateValidationContextProvider due to a bad dynamic cast. When
certificate_validation_context_provider_isn't aCertificateValidationContextSdsApi, we no-op.Fixes: #7892 and OSS-Fuzz Issues
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=17001
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=16766
Testing: add corpus entry
Signed-off-by: Asra Ali asraa@google.com