-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add support for multiple sds configs in DownstramTlsContext #24900
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| [req] | ||
| distinguished_name = req_distinguished_name | ||
| req_extensions = v3_req | ||
|
|
||
| [req_distinguished_name] | ||
| countryName = US | ||
| countryName_default = US | ||
| stateOrProvinceName = California | ||
| stateOrProvinceName_default = California | ||
| localityName = San Francisco | ||
| localityName_default = San Francisco | ||
| organizationName = Lyft | ||
| organizationName_default = Lyft | ||
| organizationalUnitName = Lyft Engineering | ||
| organizationalUnitName_default = Lyft Engineering | ||
| commonName = Test Backend Team | ||
| commonName_default = Test Backend Team | ||
| commonName_max = 64 | ||
| emailAddress = backend-team@lyft.com | ||
| emailAddress_default = backend-team@lyft.com | ||
|
|
||
| [v3_req] | ||
| basicConstraints = CA:FALSE | ||
| keyUsage = nonRepudiation, digitalSignature, keyEncipherment | ||
| extendedKeyUsage = clientAuth, serverAuth | ||
| subjectAltName = @alt_names | ||
| subjectKeyIdentifier = hash | ||
|
|
||
| [v3_ca] | ||
| basicConstraints = critical, CA:FALSE | ||
| keyUsage = nonRepudiation, digitalSignature, keyEncipherment | ||
| extendedKeyUsage = clientAuth, serverAuth | ||
| subjectAltName = @alt_names | ||
| subjectKeyIdentifier = hash | ||
| authorityKeyIdentifier = keyid:always | ||
|
|
||
| [alt_names] | ||
| URI.1 = spiffe://lyft.com/backend-team | ||
| URI.2 = http://backend.lyft.com | ||
| DNS.1 = lyft2.com | ||
| DNS.2 = www.lyft2.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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIEhzCCA2+gAwIBAgIUP0pvp6i48a1geD54z7MUaSOZiI4wDQYJKoZIhvcNAQEL | ||
| BQAwdjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM | ||
| DVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsMEEx5ZnQgRW5n | ||
| aW5lZXJpbmcxEDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjMwMTEyMDMzNTE4WhcNMjUw | ||
| MTExMDMzNTE4WjCBpjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWEx | ||
| FjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBEx5ZnQxGTAXBgNVBAsM | ||
| EEx5ZnQgRW5naW5lZXJpbmcxGjAYBgNVBAMMEVRlc3QgQmFja2VuZCBUZWFtMSQw | ||
| IgYJKoZIhvcNAQkBFhViYWNrZW5kLXRlYW1AbHlmdC5jb20wggEiMA0GCSqGSIb3 | ||
| DQEBAQUAA4IBDwAwggEKAoIBAQC/k7DASyUBfPAVIiVHP1V1SlIrEthU3Ak8KqE3 | ||
| FFRJefWR/Du59uMd+Q0Zy4Yv01tH47DPVepzHHKBx/9Mj7PZVGfHk/hXflkPsvqj | ||
| DkaRiajTlSSXOSDjlqOFqHf0zgw/UPtvlw9hefGJmlB+yGKUKXgU/Y079a8Pkfys | ||
| U0zf4RwJ7puu/RMGzTcsB8eSE/oHvV5/ar7IXhCTU8tILimLSRnfN8heHzFl1Pg1 | ||
| xNL3IuMgM71JVzM02J26rM2/3Mc9Ma9Rqyvkr4g+cn3u00gLqwelvMzZErxWVO8p | ||
| kC2toW1GJSjKccUZR5cPXO8qtPYPWrmJfLV3LVt0C+POXHQnAgMBAAGjgdswgdgw | ||
| DAYDVR0TAQH/BAIwADALBgNVHQ8EBAMCBeAwHQYDVR0lBBYwFAYIKwYBBQUHAwIG | ||
| CCsGAQUFBwMBMFwGA1UdEQRVMFOGHnNwaWZmZTovL2x5ZnQuY29tL2JhY2tlbmQt | ||
| dGVhbYYXaHR0cDovL2JhY2tlbmQubHlmdC5jb22CCWx5ZnQyLmNvbYINd3d3Lmx5 | ||
| ZnQyLmNvbTAdBgNVHQ4EFgQUx0rD8uUklrtn3hhlsA2MmOppcFMwHwYDVR0jBBgw | ||
| FoAUHQ05mHTs+06sDKaDpYtw5o+/MoIwDQYJKoZIhvcNAQELBQADggEBADGB8seV | ||
| lshkhCrF6b/+UlHsVhA0zaT5ReaSebpyFmVDdqCVqb9TXwmLsYVPmdANmSrNt4/8 | ||
| ctk+SDRRLl0pz+ciy9d2Lw0pcF4wuy4WQFunCwSjO30H47k4GTx6AhTNk8tLBQOa | ||
| K6FysykLmNz5Dr5jijPtOfnlpIDX0cBduAYAHX6BSkkYmyXVoAj0Ln+mBjsGCvq5 | ||
| GeGB6hRQomLcTFyBi4dj2w4nYY/XKDDa8j9C+MKDRgsE8+QxV/mKLzSsJyrZR2Xz | ||
| KY3zVbGwMNS/4fJOxX5e71ajZRRDznrJl8ZUAiYT1dLnPqsW20DL8ARlFH1+4pML | ||
| nam4zqTUbaBaasA= | ||
| -----END CERTIFICATE----- |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| // NOLINT(namespace-envoy) | ||
| constexpr char TEST_SERVER2_CERT_HASH[] = "0E:A1:1B:78:09:0B:D1:D6:F4:4A:8E:D2:48:7F:C3:B8:06:A7:" | ||
| "4C:8C:E4:7A:60:3D:3F:15:B0:99:03:11:2B:20"; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // NOLINT(namespace-envoy) | ||
| constexpr char TEST_SERVER2_CERT_256_HASH[] = | ||
| "0ea11b78090bd1d6f44a8ed2487fc3b806a74c8ce47a603d3f15b09903112b20"; | ||
| constexpr char TEST_SERVER2_CERT_1_HASH[] = "78912aeddf93afb00f2348dc13d1edabc2e71b2c"; | ||
| constexpr char TEST_SERVER2_CERT_SPKI[] = "J/kyBd/otG9+t94S1SbU3jj4lMyjNLvUQHZK2T/8Lbs="; | ||
| constexpr char TEST_SERVER2_CERT_SERIAL[] = "3f4a6fa7a8b8f1ad60783e78cfb314692399888e"; | ||
| constexpr char TEST_SERVER2_CERT_NOT_BEFORE[] = "Jan 12 03:35:18 2023 GMT"; | ||
| constexpr char TEST_SERVER2_CERT_NOT_AFTER[] = "Jan 11 03:35:18 2025 GMT"; |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| -----BEGIN RSA PRIVATE KEY----- | ||
| MIIEpAIBAAKCAQEAv5OwwEslAXzwFSIlRz9VdUpSKxLYVNwJPCqhNxRUSXn1kfw7 | ||
| ufbjHfkNGcuGL9NbR+Owz1Xqcxxygcf/TI+z2VRnx5P4V35ZD7L6ow5GkYmo05Uk | ||
| lzkg45ajhah39M4MP1D7b5cPYXnxiZpQfshilCl4FP2NO/WvD5H8rFNM3+EcCe6b | ||
| rv0TBs03LAfHkhP6B71ef2q+yF4Qk1PLSC4pi0kZ3zfIXh8xZdT4NcTS9yLjIDO9 | ||
| SVczNNiduqzNv9zHPTGvUasr5K+IPnJ97tNIC6sHpbzM2RK8VlTvKZAtraFtRiUo | ||
| ynHFGUeXD1zvKrT2D1q5iXy1dy1bdAvjzlx0JwIDAQABAoIBAD9vozZ5a36LpWAK | ||
| F3f5I84b1wuGSPYIilJO92UqqSJPbR5y/D/+3YO+RTVGbF+HyBEM9y4gj+qf/9az | ||
| p/jtGKudRGQUvkYSg2EsbcvyXTGx+KJfH2enthlGd0051MbON8X7hdaUmbY4T9+1 | ||
| pnPk3Kec97NTpiG1n8szFwzk5G65I/SSIziB63gFsf9eIZLdSF85fY7kyWVhl39E | ||
| loea+3FXekolIvjxh0Ui9MSzAV+0GU89wKcRp4w+reStOKkNC1RWkewMcrBPvXlI | ||
| WHBtPpKpbe+rxwmDfFs54RLcX+8hzoe+e7DbcOkl1Ear6R9LNBYjtrPW+jU2+OAp | ||
| Ry0oiDECgYEA5TpeNEOxkngmlE22dSyOVY7Tx01FUnC4KBn5SN+BwLnx+iCsdkHl | ||
| TVdh70vaRjpGNZLaMXoPkV3Eq3lqbpIYiireFRMHFJ6b2B7MgeDH2pMyrFoufYxX | ||
| MwkX38OG1U6kdAdkGNRfrEG1rvphb4FJb27RE9JC/93+1ucVYMHFF4kCgYEA1fOa | ||
| FGj1IIholdsehqftZD7Bo0Vq/CFZGe1rkd8/bli4WEpRdOLORGFgnw3mFvmM6Jcv | ||
| /9sYspq8M21sus/wbKqumWBQ8GsT/EVgVxe1uYNeyUA8BP0rZOODh7FIRWtpxEWA | ||
| b1hh/Kok2ltD0JizRNU5XGSL01jsSkL45xR5ki8CgYEAjEu4Aob39Kxi+FvD811M | ||
| 1CKxjYbGidmBbNHQ2AAr2vgzmKJMy6gzHq3/u8USF+9srzdtIeESDCd2ynhqYrg6 | ||
| Gr535DAcIkudGJ5pDoiz6Rw0ZHZhg+fUKuFC4mo6aO1UC8vGQMgisjwZZbnKzsMz | ||
| XedWQapr5UxKv975H5oY9fkCgYEAu+CU90NLrmva2211mu2v6w2o4QU+c3WdVsVE | ||
| Zu5SN0Y1a2KnsSrR0v5n6IQ9/wLZGA5bmiP9xilXBxoaFtou2F3xwMHxkY2WGTua | ||
| 5B/v9p5ECBoeGotvJcMcZ3Xobv1p/W7C4AQO199ZSV5HaNAO7G0JV/b7SnaV2IiD | ||
| Jfi24QUCgYBwnSwSMu029ujRVwrDzs0NOj3scpoFti9sKWjsm4ljG7DtHBTHz1fQ | ||
| CUvsG910WCw77njpx1DCbXmDL+kdhT4TRy2cgdGGRvpn8Od12n6pF0skzz2GMkQZ | ||
| +cmiewbDUUJbjehloWJ+OxHBQwEL3jwgSFMZZcvnP9D1CAcyLQJ0+w== | ||
| -----END RSA PRIVATE KEY----- |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| // NOLINT(namespace-envoy) | ||
| constexpr char TEST_SERVER_CERT_HASH[] = "DC:E2:2B:65:90:43:9A:36:1C:8E:6D:CA:42:8A:8C:37:C7:A1:77:" | ||
| "00:5B:C1:3E:33:8A:B9:2D:04:2C:B1:3F:0A"; | ||
| constexpr char TEST_SERVER_CERT_NOT_BEFORE[] = "Apr 7 16:46:35 2022 GMT"; | ||
| constexpr char TEST_SERVER_CERT_NOT_AFTER[] = "Apr 6 16:46:35 2024 GMT"; | ||
| constexpr char TEST_SERVER_CERT_256_HASH[] = | ||
| "dce22b6590439a361c8e6dca428a8c37c7a177005bc13e338ab92d042cb13f0a"; | ||
| constexpr char TEST_SERVER_CERT_1_HASH[] = "c777412ff69717898a3ffc61358094b6d431055a"; | ||
| constexpr char TEST_SERVER_CERT_SPKI[] = "E4cAEJmJCuF+bG3vK9LvNUaZ3Z8g+kcRKvQoJwplWAY="; | ||
| constexpr char TEST_SERVER_CERT_SERIAL[] = "4fd5b37b416fc3fa4cbea0263c98e50fb1cd8d8e"; | ||
| constexpr char TEST_SERVER_CERT_NOT_BEFORE[] = "Apr 7 16:46:35 2022 GMT"; | ||
| constexpr char TEST_SERVER_CERT_NOT_AFTER[] = "Apr 6 16:46:35 2024 GMT"; | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1805,16 +1805,28 @@ TEST_F(ServerContextConfigImplTest, TlsCertificatesAndSdsConfig) { | |
| "SDS and non-SDS TLS certificates may not be mixed in server contexts"); | ||
| } | ||
|
|
||
| TEST_F(ServerContextConfigImplTest, MultiSdsConfig) { | ||
| TEST_F(ServerContextConfigImplTest, SdsConfigNoName) { | ||
|
Author
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 old test does not make sense. The validation failure is caused by not setting name instread of dual configs. |
||
| envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; | ||
| tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs(); | ||
| tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs(); | ||
| EXPECT_THROW_WITH_REGEX( | ||
| TestUtility::validate<envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext>( | ||
| tls_context), | ||
| EnvoyException, "Proto constraint validation failed"); | ||
| } | ||
|
|
||
| TEST_F(ServerContextConfigImplTest, MultiSdsConfig) { | ||
| envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; | ||
| tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs()->set_name( | ||
| "server_cert1"); | ||
| tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs()->set_name( | ||
| "server_cert2"); | ||
| tls_context.mutable_common_tls_context()->add_tls_certificate_sds_secret_configs()->set_name( | ||
| "server_cert3"); | ||
| EXPECT_NO_THROW( | ||
| TestUtility::validate<envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext>( | ||
| tls_context)); | ||
| } | ||
|
|
||
| TEST_F(ServerContextConfigImplTest, SecretNotReady) { | ||
| envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; | ||
| NiceMock<LocalInfo::MockLocalInfo> local_info; | ||
|
|
||
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
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
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.
regenerating this header file causes CI failure since it overrides the old content, TEST_SERVER_CERT_NOT_AFTER is not generated but other test still replies on it.
I checked that servercert_info.h was first introduced by #21428.
@daixiang0 How did you generate this file, I use generate_info_header in certs.sh, but the output is different.
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.
From a comment (https://github.com/envoyproxy/envoy/pull/21428/files#r884045881) it sounds like it was supposed to be generated, but it seems like that isn't the case for some reason. If you'd like you can manually add NOT_BEFORE and NOT_AFTER for the new correct values, and file an issue to fix so that they're generated automatically like they should be. Or if you'd like to fix it, that would be great.
Uh oh!
There was an error while loading. Please reload this page.
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.
I see, the command in certs.sh was probably modified downstream to generate header info but not got committed and upstreamed.
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.
When run the script, it will update all contents, so you can add value and update all.
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.
@daixiang0 yes, but we should add changes to certs.sh otherwise when other people run certs.sh the old content is not generated.