-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Specify type for matching Subject Alternative Name. #18628
Changes from 57 commits
3c47ebb
a554236
9d3a7e9
3d6272e
f3c87a6
ff9cd5f
7287a34
3aae6db
ab5cd11
135391e
2a65d30
086ece5
42c2f75
e028388
cebac65
e8226dd
b083021
af0c18b
bdc7c4e
67d05d1
b4bb871
0d880c0
7970de4
35265c6
d5e2ec4
9901676
4af6264
f6eefd6
caec188
b1b0261
831ad94
947da60
711b024
c0113a9
7bcd271
d0e365b
bb5dc55
14b6c0f
5ee23dd
b4d8863
cd8d388
e8c36bb
6adfb56
d94cc3e
b1bfe77
d4dd073
3bc3cb6
e034baf
d8aaafb
de0525a
cc2b181
00f151c
36226ea
133a6a8
c5b05ab
05fb9b0
d97b59f
6ceb50d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import "envoy/type/matcher/v3/string.proto"; | |
import "google/protobuf/any.proto"; | ||
import "google/protobuf/wrappers.proto"; | ||
|
||
import "envoy/annotations/deprecation.proto"; | ||
import "udpa/annotations/migrate.proto"; | ||
import "udpa/annotations/sensitive.proto"; | ||
import "udpa/annotations/status.proto"; | ||
|
@@ -268,7 +269,26 @@ message CertificateProviderPluginInstance { | |
string certificate_name = 2; | ||
} | ||
|
||
// [#next-free-field: 15] | ||
// Matcher for subject alternative names, to match both type and value of the SAN. | ||
message SubjectAltNameMatcher { | ||
// Indicates the choice of GeneralName as defined in section 4.2.1.5 of RFC 5280 to match | ||
// against. | ||
enum SanType { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a comment here with the link to the relevant RFC section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added comment. |
||
SAN_TYPE_UNSPECIFIED = 0; | ||
EMAIL = 1; | ||
DNS = 2; | ||
URI = 3; | ||
IP_ADDRESS = 4; | ||
} | ||
|
||
// Specification of type of SAN. Note that the default enum value is an invalid choice. | ||
SanType san_type = 1 [(validate.rules).enum = {defined_only: true not_in: 0}]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why make the default value be invalid? Can't you just make the field required? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The required pgv rule is not available for enums. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you delete enum value 0? Would that result in un-set triggering a violation of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately the first enum number HAS to be 0 in proto3. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From https://github.com/envoyproxy/protoc-gen-validate, you can specify 0 as not valid for this field. Example: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current validation for san_type does include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I apologize; I totally missed that while reading this. Looks good; sorry for the hassle. |
||
|
||
// Matcher for SAN value. | ||
type.matcher.v3.StringMatcher matcher = 2 [(validate.rules).message = {required: true}]; | ||
} | ||
|
||
// [#next-free-field: 16] | ||
message CertificateValidationContext { | ||
option (udpa.annotations.versioning).previous_message_type = | ||
"envoy.api.v2.auth.CertificateValidationContext"; | ||
|
@@ -298,8 +318,8 @@ message CertificateValidationContext { | |
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.verify_certificate_spki>`, | ||
// :ref:`verify_certificate_hash | ||
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.verify_certificate_hash>`, or | ||
// :ref:`match_subject_alt_names | ||
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.match_subject_alt_names>`) is also | ||
// :ref:`match_typed_subject_alt_names | ||
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.match_typed_subject_alt_names>`) is also | ||
// specified. | ||
// | ||
// It can optionally contain certificate revocation lists, in which case Envoy will verify | ||
|
@@ -406,6 +426,8 @@ message CertificateValidationContext { | |
|
||
// An optional list of Subject Alternative name matchers. If specified, Envoy will verify that the | ||
// Subject Alternative Name of the presented certificate matches one of the specified matchers. | ||
// The matching uses "any" semantics, that is to say, the SAN is verified if at least one matcher is | ||
// matched. | ||
// | ||
// When a certificate has wildcard DNS SAN entries, to match a specific client, it should be | ||
// configured with exact match type in the :ref:`string matcher <envoy_v3_api_msg_type.matcher.v3.StringMatcher>`. | ||
|
@@ -414,15 +436,22 @@ message CertificateValidationContext { | |
// | ||
// .. code-block:: yaml | ||
// | ||
// match_subject_alt_names: | ||
// exact: "api.example.com" | ||
// match_typed_subject_alt_names: | ||
// - san_type: DNS | ||
// matcher: | ||
// exact: "api.example.com" | ||
// | ||
// .. attention:: | ||
// | ||
// Subject Alternative Names are easily spoofable and verifying only them is insecure, | ||
// therefore this option must be used together with :ref:`trusted_ca | ||
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.trusted_ca>`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you improve the comment by mentioning that this has an Any semantics (the SAN is verified if at least one of the matchers is matched) |
||
repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9; | ||
repeated SubjectAltNameMatcher match_typed_subject_alt_names = 15; | ||
|
||
// This field is deprecated in favor of ref:`match_typed_subject_alt_names | ||
// <envoy_v3_api_field_extensions.transport_sockets.tls.v3.CertificateValidationContext.match_typed_subject_alt_names>` | ||
repeated type.matcher.v3.StringMatcher match_subject_alt_names = 9 | ||
[deprecated = true, (envoy.annotations.deprecated_at_minor_version) = "3.0"]; | ||
|
||
// [#not-implemented-hide:] Must present signed certificate time-stamp. | ||
google.protobuf.BoolValue require_signed_certificate_timestamp = 6; | ||
|
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 add comments describing the message and field.