Skip to content
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

Env Var allowing proxies to utilize OS CA Cert #33472

Merged
merged 39 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
de601e8
Env Var allowing proxies to utilize OS CA Cert
Kmoneal Jun 16, 2021
f47e5b9
Move OS cert replace logic to proxy
Kmoneal Jun 28, 2021
e65a149
Fix typo
Kmoneal Jun 29, 2021
2be2230
Move function in pilot code and check for OS cert once
Kmoneal Jul 14, 2021
d32fc51
Linter and change setting off by default
Kmoneal Jul 14, 2021
0fdb534
Move OS CA Path find to option generation
Kmoneal Jul 15, 2021
728f085
Add release notes
Kmoneal Jul 15, 2021
d455ee4
Lock pointers when accessing or using them
Kmoneal Jul 16, 2021
c73f8b5
Remove mutex inside Options
Kmoneal Jul 19, 2021
239055a
Fix secret resource choice
Kmoneal Jul 20, 2021
84364c1
Add unit tests
Kmoneal Aug 12, 2021
7303343
Fix tests and linter errors
Kmoneal Aug 12, 2021
efa1037
Improve tests, releasenotes and logging
Kmoneal Aug 16, 2021
87de455
Add back code that was needed
Kmoneal Aug 16, 2021
f1c0d4d
Fix linter issues
Kmoneal Aug 17, 2021
d6024f4
Use string constant in place of file-root:system
Kmoneal Aug 17, 2021
23d71ea
Update to remove proxy from ClusterBuilder
Kmoneal Aug 17, 2021
b78aa67
Fix auto import location
Kmoneal Aug 17, 2021
8109d7d
Refactor code
Kmoneal Aug 18, 2021
a17c34f
Refactor SdsCertificateConfig to common package
Kmoneal Aug 19, 2021
9429ebf
Move security tests from util.go
Kmoneal Aug 19, 2021
05e6850
Fix releasenotes
Kmoneal Aug 23, 2021
2f76981
Add documentation to public methods
Kmoneal Aug 23, 2021
b3dd4f8
Improve VERIFY_CERT_AT_CLIENT testing
Kmoneal Aug 23, 2021
69755af
initialize CAFilePath in security instead of main
Kmoneal Aug 23, 2021
62b31a7
Refactor and remove dead code
Kmoneal Aug 24, 2021
73115d5
Refactor to move cafile package
Kmoneal Aug 24, 2021
491c46f
Refactor toEnvoySecret funciton
Kmoneal Aug 24, 2021
8105066
Revert changes that were needed for istiod to know the OS CA file path
Kmoneal Aug 25, 2021
ea811c0
Fix linter problems
Kmoneal Aug 26, 2021
d015d8f
Correct tests and introduce caRootPath to SecretManager
Kmoneal Aug 26, 2021
ebeaab3
Fix missing argument for mock initializaiton
Kmoneal Aug 26, 2021
6018a08
Reverse mock changes and replace string with const
Kmoneal Aug 26, 2021
9d4b904
Fix SdsCertificateConfig to use file-root:system cert.
Kmoneal Aug 30, 2021
c9f4d28
Fix tests by removing file-root: prefix
Kmoneal Aug 30, 2021
c10d2fc
Fix empty OS CA cert causing an error
Kmoneal Aug 31, 2021
6561a24
Remove testing print code
Kmoneal Aug 31, 2021
b4dd302
Rebase cluster_builder_test update struct
Kmoneal Aug 31, 2021
c461a76
Fix rebase changes made to DestiantionRule
Kmoneal Sep 10, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions pilot/cmd/pilot-agent/options/security.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"istio.io/istio/pkg/jwt"
"istio.io/istio/pkg/security"
"istio.io/istio/security/pkg/credentialfetcher"
"istio.io/istio/security/pkg/nodeagent/cafile"
"istio.io/istio/security/pkg/nodeagent/plugin/providers/google/stsclient"
"istio.io/istio/security/pkg/stsservice/tokenmanager"
"istio.io/pkg/log"
Expand All @@ -52,6 +53,7 @@ func NewSecurityOptions(proxyConfig *meshconfig.ProxyConfig, stsPort int, tokenM
SecretRotationGracePeriodRatio: secretRotationGracePeriodRatioEnv,
STSPort: stsPort,
CertSigner: certSigner.Get(),
CARootPath: cafile.CACertFilePath,
}

o, err := SetupSecurityOptions(proxyConfig, o, jwtPolicy.Get(),
Expand Down
3 changes: 3 additions & 0 deletions pilot/pkg/features/pilot.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,9 @@ var (
EnableQUICListeners = env.RegisterBoolVar("PILOT_ENABLE_QUIC_LISTENERS", false,
"If true, QUIC listeners will be generated wherever there are listeners terminating TLS on gateways "+
"if the gateway service exposes a UDP port with the same number (for example 443/TCP and 443/UDP)").Get()

VerifyCertAtClient = env.RegisterBoolVar("VERIFY_CERTIFICATE_AT_CLIENT", false,
"If enabled, certificates received by the proxy will be verified against the OS CA certificate bundle.").Get()
)

// UnsafeFeaturesEnabled returns true if any unsafe features are enabled.
Expand Down
55 changes: 0 additions & 55 deletions pilot/pkg/model/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,58 +270,3 @@ func getConfigsForWorkload(configsByNamespace map[string][]config.Config,

return configs
}

// SdsCertificateConfig holds TLS certs needed to build SDS TLS context.
type SdsCertificateConfig struct {
CertificatePath string
PrivateKeyPath string
CaCertificatePath string
}

// GetResourceName converts a SdsCertificateConfig to a string to be used as an SDS resource name
func (s SdsCertificateConfig) GetResourceName() string {
if s.IsKeyCertificate() {
return "file-cert:" + s.CertificatePath + ResourceSeparator + s.PrivateKeyPath // Format: file-cert:%s~%s
}
return ""
}

// GetRootResourceName converts a SdsCertificateConfig to a string to be used as an SDS resource name for the root
func (s SdsCertificateConfig) GetRootResourceName() string {
if s.IsRootCertificate() {
return "file-root:" + s.CaCertificatePath // Format: file-root:%s
}
return ""
}

// IsRootCertificate returns true if this config represents a root certificate config.
func (s SdsCertificateConfig) IsRootCertificate() bool {
return s.CaCertificatePath != ""
}

// IsKeyCertificate returns true if this config represents key certificate config.
func (s SdsCertificateConfig) IsKeyCertificate() bool {
return s.CertificatePath != "" && s.PrivateKeyPath != ""
}

// SdsCertificateConfigFromResourceName converts the provided resource name into a SdsCertificateConfig
// If the resource name is not valid, false is returned.
func SdsCertificateConfigFromResourceName(resource string) (SdsCertificateConfig, bool) {
if strings.HasPrefix(resource, "file-cert:") {
filesString := strings.TrimPrefix(resource, "file-cert:")
split := strings.Split(filesString, ResourceSeparator)
if len(split) != 2 {
return SdsCertificateConfig{}, false
}
return SdsCertificateConfig{split[0], split[1], ""}, true
} else if strings.HasPrefix(resource, "file-root:") {
filesString := strings.TrimPrefix(resource, "file-root:")
split := strings.Split(filesString, ResourceSeparator)
if len(split) != 1 {
return SdsCertificateConfig{}, false
}
return SdsCertificateConfig{"", "", split[0]}, true
} else {
return SdsCertificateConfig{}, false
}
}
69 changes: 0 additions & 69 deletions pilot/pkg/model/authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,75 +37,6 @@ const (

var baseTimestamp = time.Date(2020, 2, 2, 2, 2, 2, 0, time.UTC)

func TestSdsCertificateConfigFromResourceName(t *testing.T) {
cases := []struct {
name string
resource string
root bool
key bool
output SdsCertificateConfig
rootResourceName string
resourceName string
}{
{
"cert",
"file-cert:cert~key",
false,
true,
SdsCertificateConfig{"cert", "key", ""},
"",
"file-cert:cert~key",
},
{
"root cert",
"file-root:root",
true,
false,
SdsCertificateConfig{"", "", "root"},
"file-root:root",
"",
},
{
"invalid prefix",
"file:root",
false,
false,
SdsCertificateConfig{"", "", ""},
"",
"",
},
{
"invalid contents",
"file-root:root~extra",
false,
false,
SdsCertificateConfig{"", "", ""},
"",
"",
},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
got, _ := SdsCertificateConfigFromResourceName(tt.resource)
if got != tt.output {
t.Fatalf("got %v, expected %v", got, tt.output)
}
if root := got.IsRootCertificate(); root != tt.root {
t.Fatalf("unexpected isRootCertificate got %v, expected %v", root, tt.root)
}
if key := got.IsKeyCertificate(); key != tt.key {
t.Fatalf("unexpected IsKeyCertificate got %v, expected %v", key, tt.key)
}
if root := got.GetRootResourceName(); root != tt.rootResourceName {
t.Fatalf("unexpected GetRootResourceName got %v, expected %v", root, tt.rootResourceName)
}
if cert := got.GetResourceName(); cert != tt.resourceName {
t.Fatalf("unexpected GetRootResourceName got %v, expected %v", cert, tt.resourceName)
}
})
}
}

func TestGetPoliciesForWorkload(t *testing.T) {
policies := getTestAuthenticationPolicies(createTestConfigs(true /* with mesh peer authn */), t)

Expand Down
5 changes: 5 additions & 0 deletions pilot/pkg/networking/core/v1alpha3/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,11 @@ func selectTrafficPolicyComponents(policy *networking.TrafficPolicy) (
loadBalancer := policy.LoadBalancer
tls := policy.Tls

// Check if CA Certificate should be System CA Certificate
if features.VerifyCertAtClient && tls != nil && tls.CaCertificates == "" {
tls.CaCertificates = "system"
}

return connectionPool, outlierDetection, loadBalancer, tls
}

Expand Down
5 changes: 3 additions & 2 deletions pilot/pkg/networking/core/v1alpha3/cluster_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (
"istio.io/istio/pkg/config/labels"
"istio.io/istio/pkg/config/schema/gvk"
"istio.io/istio/pkg/network"
"istio.io/istio/pkg/security"
"istio.io/istio/pkg/util/gogo"
"istio.io/pkg/log"
)
Expand Down Expand Up @@ -1037,7 +1038,7 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
authn_model.ApplyCustomSDSToClientCommonTLSContext(tlsContext.CommonTlsContext, tls)
} else {
// If CredentialName is not set fallback to files specified in DR.
res := model.SdsCertificateConfig{
res := security.SdsCertificateConfig{
CaCertificatePath: tls.CaCertificates,
}
// If tls.CaCertificate or CaCertificate in Metadata isn't configured don't set up SdsSecretConfig
Expand Down Expand Up @@ -1081,7 +1082,7 @@ func (cb *ClusterBuilder) buildUpstreamClusterTLSContext(opts *buildClusterOpts,
// These are certs being mounted from within the pod and specified in Destination Rules.
// Rather than reading directly in Envoy, which does not support rotation, we will
// serve them over SDS by reading the files.
res := model.SdsCertificateConfig{
res := security.SdsCertificateConfig{
CertificatePath: tls.ClientCertificate,
PrivateKeyPath: tls.PrivateKey,
CaCertificatePath: tls.CaCertificates,
Expand Down