diff --git a/.pipelines/templates/e2e-test.yaml b/.pipelines/templates/e2e-test.yaml index ae7325bb2..4bca3b309 100644 --- a/.pipelines/templates/e2e-test.yaml +++ b/.pipelines/templates/e2e-test.yaml @@ -72,3 +72,7 @@ steps: CI_KIND_CLUSTER: ${{ parameters.ciKindCluster }} ${{ if parameters.isArcTest }}: IS_ARC_TEST: ${{ parameters.isArcTest }} + # If the image is a released versions (i.e <= v1.3), it still doesn't support the + # split cert/key feature, so we need to skip tests for those versions. + ${{ if parameters.testReleasedVersion }}: + GINKGO_SKIP: WriteCertAndKeyInSeparateFiles diff --git a/cmd/main.go b/cmd/main.go index 75b4a5a87..4404010ba 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -15,7 +15,6 @@ import ( "time" "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/metrics" - "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider" "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/server" "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/utils" "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/version" @@ -46,6 +45,10 @@ var ( metricsBackend = flag.String("metrics-backend", "Prometheus", "Backend used for metrics") prometheusPort = flag.Int("prometheus-port", 8898, "Prometheus port for metrics backend") + + constructPEMChain = flag.Bool("construct-pem-chain", true, "explicitly reconstruct the pem chain in the order: SERVER, INTERMEDIATE, ROOT") + writeCertAndKeyInSeparateFiles = flag.Bool("write-cert-and-key-in-separate-files", false, + "Write cert and key in separate files. The individual files will be named as .crt and .key. These files will be created in addition to the single file.") ) func main() { @@ -89,9 +92,12 @@ func main() { os.Exit(1) } - if *provider.ConstructPEMChain { + if *constructPEMChain { klog.Infof("construct pem chain feature enabled") } + if *writeCertAndKeyInSeparateFiles { + klog.Infof("write cert and key in separate files feature enabled") + } // Add csi-secrets-store user agent to adal requests if err := adal.AddToUserAgent(version.GetUserAgent()); err != nil { klog.ErrorS(err, "failed to add user agent to adal") @@ -124,7 +130,7 @@ func main() { grpc.UnaryInterceptor(utils.LogInterceptor()), } s := grpc.NewServer(opts...) - csiDriverProviderServer := server.New() + csiDriverProviderServer := server.New(*constructPEMChain, *writeCertAndKeyInSeparateFiles) k8spb.RegisterCSIDriverProviderServer(s, csiDriverProviderServer) // Register the health service. grpc_health_v1.RegisterHealthServer(s, csiDriverProviderServer) diff --git a/manifest_staging/charts/csi-secrets-store-provider-azure/README.md b/manifest_staging/charts/csi-secrets-store-provider-azure/README.md index 7d2158827..29b0f4f83 100644 --- a/manifest_staging/charts/csi-secrets-store-provider-azure/README.md +++ b/manifest_staging/charts/csi-secrets-store-provider-azure/README.md @@ -144,3 +144,4 @@ The following table lists the configurable parameters of the csi-secrets-store-p | `rbac.install` | Install default service account | true | | `rbac.pspEnabled` | If `true`, create and use a restricted pod security policy for Secrets Store CSI Driver AKV provider pod(s) | false | | `constructPEMChain` | Explicitly reconstruct the pem chain in the order: SERVER, INTERMEDIATE, ROOT | `true` | +| `writeCertAndKeyInSeparateFiles` | Write cert and key in separate files. The individual files will be named as .crt and .key. These files will be created in addition to the single file. | `false` | diff --git a/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer-windows.yaml b/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer-windows.yaml index e8887bb3e..39ac6fdad 100644 --- a/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer-windows.yaml +++ b/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer-windows.yaml @@ -48,6 +48,9 @@ spec: - --healthz-port={{ .Values.windows.healthzPort }} - --healthz-path={{ .Values.windows.healthzPath }} - --healthz-timeout={{ .Values.windows.healthzTimeout }} + {{- if .Values.writeCertAndKeyInSeparateFiles }} + - --write-cert-and-key-in-separate-files={{ .Values.writeCertAndKeyInSeparateFiles }} + {{- end }} livenessProbe: httpGet: path: {{ .Values.windows.healthzPath }} diff --git a/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer.yaml b/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer.yaml index 02ae87afd..d2c3b6199 100644 --- a/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer.yaml +++ b/manifest_staging/charts/csi-secrets-store-provider-azure/templates/provider-azure-installer.yaml @@ -59,6 +59,9 @@ spec: - --healthz-port={{ .Values.linux.healthzPort }} - --healthz-path={{ .Values.linux.healthzPath }} - --healthz-timeout={{ .Values.linux.healthzTimeout }} + {{- if .Values.writeCertAndKeyInSeparateFiles }} + - --write-cert-and-key-in-separate-files={{ .Values.writeCertAndKeyInSeparateFiles }} + {{- end }} livenessProbe: httpGet: path: {{ .Values.linux.healthzPath }} diff --git a/manifest_staging/charts/csi-secrets-store-provider-azure/values.yaml b/manifest_staging/charts/csi-secrets-store-provider-azure/values.yaml index c609fcc48..b5e7316b5 100644 --- a/manifest_staging/charts/csi-secrets-store-provider-azure/values.yaml +++ b/manifest_staging/charts/csi-secrets-store-provider-azure/values.yaml @@ -167,3 +167,6 @@ rbac: # explicitly reconstruct the pem chain in the order: SERVER, INTERMEDIATE, ROOT constructPEMChain: true + +# Write cert and key in separate files. The individual files will be named as .crt and .key. These files will be created in addition to the single file. +writeCertAndKeyInSeparateFiles: false diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 48c5a1abd..2fb0fba9d 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -8,7 +8,6 @@ import ( "encoding/base64" "encoding/hex" "encoding/pem" - "flag" "fmt" "math/big" "os" @@ -36,10 +35,6 @@ import ( "k8s.io/klog/v2" ) -var ( - ConstructPEMChain = flag.Bool("construct-pem-chain", true, "explicitly reconstruct the pem chain in the order: SERVER, INTERMEDIATE, ROOT") -) - // Provider implements the secrets-store-csi-driver provider interface type Interface interface { GetSecretsStoreObjectContent(ctx context.Context, attrib, secrets map[string]string, targetPath string, defaultFilePermission os.FileMode) ([]types.SecretFile, error) @@ -47,6 +42,9 @@ type Interface interface { type provider struct { reporter metrics.StatsReporter + + constructPEMChain bool + writeCertAndKeyInSeparateFiles bool } // mountConfig holds the information for the mount event @@ -65,10 +63,18 @@ type mountConfig struct { podNamespace string } +type keyvaultObject struct { + content string + fileNameSuffix string + version string +} + // NewProvider creates a new provider -func NewProvider() Interface { +func NewProvider(constructPEMChain, writeCertAndKeyInSeparateFiles bool) Interface { return &provider{ - reporter: metrics.NewStatsReporter(), + reporter: metrics.NewStatsReporter(), + constructPEMChain: constructPEMChain, + writeCertAndKeyInSeparateFiles: writeCertAndKeyInSeparateFiles, } } @@ -241,30 +247,33 @@ func (p *provider) GetSecretsStoreObjectContent(ctx context.Context, attrib, sec for _, resolvedKvObject := range resolvedKvObjects { // fetch the object from Key Vault - content, newObjectVersion, err := p.getKeyVaultObjectContent(ctx, kvClient, resolvedKvObject, *vaultURL) + result, err := p.getKeyVaultObjectContent(ctx, kvClient, resolvedKvObject, *vaultURL) if err != nil { return nil, err } - objectContent, err := getContentBytes(content, resolvedKvObject.ObjectType, resolvedKvObject.ObjectEncoding) - if err != nil { - return nil, err - } + for idx := range result { + r := result[idx] + objectContent, err := getContentBytes(r.content, resolvedKvObject.ObjectType, resolvedKvObject.ObjectEncoding) + if err != nil { + return nil, err + } - // objectUID is a unique identifier in the format / - // This is the object id the user sees in the SecretProviderClassPodStatus - objectUID := resolvedKvObject.GetObjectUID() - file := types.SecretFile{ - Path: resolvedKvObject.GetFileName(), - Content: objectContent, - UID: objectUID, - Version: newObjectVersion, - } - // the validity of file permission is already checked in the validate function above - file.FileMode, _ = resolvedKvObject.GetFilePermission(defaultFilePermission) + // objectUID is a unique identifier in the format / + // This is the object id the user sees in the SecretProviderClassPodStatus + objectUID := resolvedKvObject.GetObjectUID() + file := types.SecretFile{ + Path: resolvedKvObject.GetFileName() + r.fileNameSuffix, + Content: objectContent, + UID: objectUID, + Version: r.version, + } + // the validity of file permission is already checked in the validate function above + file.FileMode, _ = resolvedKvObject.GetFilePermission(defaultFilePermission) - files = append(files, file) - klog.V(5).InfoS("added file to the gRPC response", "file", file.Path, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) + files = append(files, file) + klog.V(5).InfoS("added file to the gRPC response", "file", file.Path, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) + } } } @@ -455,8 +464,8 @@ func getCertificateVersions(ctx context.Context, kvClient *kv.BaseClient, vaultU return certVersions, nil } -// GetKeyVaultObjectContent get content of the keyvault object -func (p *provider) getKeyVaultObjectContent(ctx context.Context, kvClient *kv.BaseClient, kvObject types.KeyVaultObject, vaultURL string) (content, version string, err error) { +// getKeyVaultObjectContent gets content of the keyvault object +func (p *provider) getKeyVaultObjectContent(ctx context.Context, kvClient *kv.BaseClient, kvObject types.KeyVaultObject, vaultURL string) (result []keyvaultObject, err error) { start := time.Now() defer func() { var errMsg string @@ -468,66 +477,77 @@ func (p *provider) getKeyVaultObjectContent(ctx context.Context, kvClient *kv.Ba switch kvObject.ObjectType { case types.VaultObjectTypeSecret: - return getSecret(ctx, kvClient, vaultURL, kvObject) + return p.getSecret(ctx, kvClient, vaultURL, kvObject) case types.VaultObjectTypeKey: - return getKey(ctx, kvClient, vaultURL, kvObject) + return p.getKey(ctx, kvClient, vaultURL, kvObject) case types.VaultObjectTypeCertificate: - return getCertificate(ctx, kvClient, vaultURL, kvObject) + return p.getCertificate(ctx, kvClient, vaultURL, kvObject) default: err := errors.Errorf("Invalid vaultObjectTypes. Should be secret, key, or cert") - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } } // getSecret retrieves the secret from the vault -func getSecret(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) (string, string, error) { +func (p *provider) getSecret(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) ([]keyvaultObject, error) { secret, err := kvClient.GetSecret(ctx, vaultURL, kvObject.ObjectName, kvObject.ObjectVersion) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } if secret.Value == nil { - return "", "", errors.Errorf("secret value is nil") + return nil, errors.Errorf("secret value is nil") } if secret.ID == nil { - return "", "", errors.Errorf("secret id is nil") + return nil, errors.Errorf("secret id is nil") } content := *secret.Value version := getObjectVersion(*secret.ID) + result := []keyvaultObject{} // if the secret is part of a certificate, then we need to convert the certificate and key to PEM format if secret.Kid != nil && len(*secret.Kid) > 0 { switch *secret.ContentType { case types.CertTypePem: - return content, version, nil case types.CertTypePfx: // object format requested is pfx, then return the content as is if strings.EqualFold(kvObject.ObjectFormat, types.ObjectFormatPFX) { - return content, version, err + break } // convert to pem as that's the default object format for this provider - content, err := decodePKCS12(*secret.Value) - if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + if content, err = p.decodePKCS12(*secret.Value); err != nil { + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } - return content, version, nil default: err := errors.Errorf("failed to get certificate. unknown content type '%s'", *secret.ContentType) - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + } + + if p.writeCertAndKeyInSeparateFiles { + // when writeCertAndKeyInSeparateFiles feature flag is enabled, we write the cert and key in separate files + // with suffixes .crt and .key respectively. These files are written in addition to the default file which + // contains the cert and key in a single file to maintain backward compatibility with the existing behavior. + cert, key := splitCertAndKey(content) + result = append(result, + keyvaultObject{version: version, content: cert, fileNameSuffix: ".crt"}, + keyvaultObject{version: version, content: key, fileNameSuffix: ".key"}, + ) } } - return content, version, nil + + result = append(result, keyvaultObject{content: content, version: version}) + return result, nil } // getKey retrieves the key from the vault -func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) (string, string, error) { +func (p *provider) getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) ([]keyvaultObject, error) { keybundle, err := kvClient.GetKey(ctx, vaultURL, kvObject.ObjectName, kvObject.ObjectVersion) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } if keybundle.Key == nil { - return "", "", errors.Errorf("key value is nil") + return nil, errors.Errorf("key value is nil") } if keybundle.Key.Kid == nil { - return "", "", errors.Errorf("key id is nil") + return nil, errors.Errorf("key id is nil") } version := getObjectVersion(*keybundle.Key.Kid) // for object type "key" the public key is written to the file in PEM format @@ -536,12 +556,12 @@ func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObj // decode the base64 bytes for n nb, err := base64.RawURLEncoding.DecodeString(*keybundle.Key.N) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } // decode the base64 bytes for e eb, err := base64.RawURLEncoding.DecodeString(*keybundle.Key.E) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } e := new(big.Int).SetBytes(eb).Int64() pKey := &rsa.PublicKey{ @@ -550,7 +570,7 @@ func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObj } derBytes, err := x509.MarshalPKIXPublicKey(pKey) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } pubKeyBlock := &pem.Block{ Type: "PUBLIC KEY", @@ -558,21 +578,21 @@ func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObj } var pemData []byte pemData = append(pemData, pem.EncodeToMemory(pubKeyBlock)...) - return string(pemData), version, nil + return []keyvaultObject{{content: string(pemData), version: version}}, nil case kv.EC, kv.ECHSM: // decode the base64 bytes for x xb, err := base64.RawURLEncoding.DecodeString(*keybundle.Key.X) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } // decode the base64 bytes for y yb, err := base64.RawURLEncoding.DecodeString(*keybundle.Key.Y) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } crv, err := getCurve(keybundle.Key.Crv) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } pKey := &ecdsa.PublicKey{ X: new(big.Int).SetBytes(xb), @@ -581,7 +601,7 @@ func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObj } derBytes, err := x509.MarshalPKIXPublicKey(pKey) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } pubKeyBlock := &pem.Block{ Type: "PUBLIC KEY", @@ -589,35 +609,35 @@ func getKey(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObj } var pemData []byte pemData = append(pemData, pem.EncodeToMemory(pubKeyBlock)...) - return string(pemData), version, nil + return []keyvaultObject{{content: string(pemData), version: version}}, nil default: err := errors.Errorf("failed to get key. key type '%s' currently not supported", keybundle.Key.Kty) - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } } // getCertificate retrieves the certificate from the vault -func getCertificate(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) (string, string, error) { +func (p *provider) getCertificate(ctx context.Context, kvClient *kv.BaseClient, vaultURL string, kvObject types.KeyVaultObject) ([]keyvaultObject, error) { // for object type "cert" the certificate is written to the file in PEM format certbundle, err := kvClient.GetCertificate(ctx, vaultURL, kvObject.ObjectName, kvObject.ObjectVersion) if err != nil { - return "", "", wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) + return nil, wrapObjectTypeError(err, kvObject.ObjectType, kvObject.ObjectName, kvObject.ObjectVersion) } if certbundle.Cer == nil { - return "", "", errors.Errorf("certificate value is nil") + return nil, errors.Errorf("certificate value is nil") } if certbundle.ID == nil { - return "", "", errors.Errorf("certificate id is nil") + return nil, errors.Errorf("certificate id is nil") } version := getObjectVersion(*certbundle.ID) certBlock := &pem.Block{ - Type: "CERTIFICATE", + Type: types.CertificateType, Bytes: *certbundle.Cer, } var pemData []byte pemData = append(pemData, pem.EncodeToMemory(certBlock)...) - return string(pemData), version, nil + return []keyvaultObject{{content: string(pemData), version: version}}, nil } func wrapObjectTypeError(err error, objectType, objectName, objectVersion string) error { @@ -626,7 +646,7 @@ func wrapObjectTypeError(err error, objectType, objectName, objectVersion string // decodePkcs12 decodes PKCS#12 client certificates by extracting the public certificates, the private // keys and converts it to PEM format -func decodePKCS12(value string) (content string, err error) { +func (p *provider) decodePKCS12(value string) (content string, err error) { pfxRaw, err := base64.StdEncoding.DecodeString(value) if err != nil { return "", err @@ -668,7 +688,7 @@ func decodePKCS12(value string) (content string, err error) { // construct the pem chain in the order // SERVER, INTERMEDIATE, ROOT - if *ConstructPEMChain { + if p.constructPEMChain { pemCertData, err = fetchCertChains(pemCertData) if err != nil { return "", err @@ -858,3 +878,31 @@ func fetchCertChains(data []byte) ([]byte, error) { } return pemData, nil } + +// splitCertAndKey takes the given data and splits it into cert and key +// this function doesn't check if the returned cert and key is not empty as this +// can't be enforced. It is possible the secret in the key vault only contains the +// cert or key. +func splitCertAndKey(certAndKey string) (certs string, privKey string) { + // split the cert and key for PEM format + // This does not handle the case where cert and key is in PFX format + // TODO(aramase) consider adding support for PFX format if there is an ask + var cert, key []byte + data := []byte(certAndKey) + for { + block, rest := pem.Decode(data) + if block == nil { + break + } + if block.Type == types.CertificateType { + cert = append(cert, pem.EncodeToMemory(block)...) + } else { + key = append(key, pem.EncodeToMemory(block)...) + } + data = rest + } + + certs = string(cert) + privKey = string(key) + return certs, privKey +} diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go index 2f0b9c73c..2da5a429d 100644 --- a/pkg/provider/provider_test.go +++ b/pkg/provider/provider_test.go @@ -209,7 +209,8 @@ lKn75l/9h0PwiiPaI0TGKN2O8AwvhGGwDElmFhYtXedbbaST6rbVRDUj for _, tc := range cases { t.Run(tc.desc, func(t *testing.T) { - content, err := decodePKCS12(tc.value) + p := &provider{constructPEMChain: true} + content, err := p.decodePKCS12(tc.value) if err != nil { t.Fatalf("expected nil err, got: %v", err) } @@ -1029,7 +1030,7 @@ func TestGetSecretsStoreObjectContent(t *testing.T) { for _, tc := range cases { t.Run(tc.desc, func(t *testing.T) { - p := NewProvider() + p := NewProvider(false, false) tmpDir, err := os.MkdirTemp("", "ut") assert.NoError(t, err) @@ -1188,3 +1189,106 @@ func TestGetObjectVersion(t *testing.T) { actual := getObjectVersion(id) assert.Equal(t, expectedVersion, actual) } + +func TestSplitCertAndKey(t *testing.T) { + rootCACert := `-----BEGIN CERTIFICATE----- +MIIBeTCCAR6gAwIBAgIRAM3RAPH7k1Q+bICMC0mzKhkwCgYIKoZIzj0EAwIwGjEY +MBYGA1UEAxMPRXhhbXBsZSBSb290IENBMB4XDTIwMTIwMzAwMTAxNFoXDTMwMTIw +MTAwMTAxNFowGjEYMBYGA1UEAxMPRXhhbXBsZSBSb290IENBMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAE1/AGExuSemtxPRzFECpefowtkcOQr7jaq355kfb2hUR2 +LnMn+71fD4mZmMXT0kuxgeE2zC2CxOHdoJ/FmcQJxaNFMEMwDgYDVR0PAQH/BAQD +AgEGMBIGA1UdEwEB/wQIMAYBAf8CAQEwHQYDVR0OBBYEFKTuLl7BATUYGD6ZeUV3 +2f8UAWoqMAoGCCqGSM49BAMCA0kAMEYCIQDEz2XKXPb0Q/Y40Gtxo8r6sa0Ra6U0 +fpTPteqfpl8iGQIhAOo8tpUYiREVSYZu130fN0Gvy4WmJMFAi7JrVeSnZ7uP +-----END CERTIFICATE----- +` + + intermediateCert := `-----BEGIN CERTIFICATE----- +MIIBozCCAUmgAwIBAgIRANEldEfXaQ+L2M1ahC6w4vAwCgYIKoZIzj0EAwIwGjEY +MBYGA1UEAxMPRXhhbXBsZSBSb290IENBMB4XDTIwMTIwMzAwMTAyNFoXDTMwMTIw +MTAwMTAyNFowJDEiMCAGA1UEAxMZRXhhbXBsZSBJbnRlcm1lZGlhdGUgQ0EgMTBZ +MBMGByqGSM49AgEGCCqGSM49AwEHA0IABOhTE8r5NpDIDF/6VLgPT+//0IR59Uzn +78JfV54E0qFA21khrcqc20/RJD+lyUv313gYQD9SxBXXxcGtl1OJ0s2jZjBkMA4G +A1UdDwEB/wQEAwIBBjASBgNVHRMBAf8ECDAGAQH/AgEAMB0GA1UdDgQWBBR+2JY0 +VhjrWsrUng+V8dgeZBOGJzAfBgNVHSMEGDAWgBSk7i5ewQE1GBg+mXlFd9n/FAFq +KjAKBggqhkjOPQQDAgNIADBFAiB9EQB+siuNboL7k78CUzhZJ+5lD0cXUpGYGWYT +rxcX6QIhALGptitzrZ4z/MDMBPkan48bqk6O08e1tQ9dJOIoEKq7 +-----END CERTIFICATE----- +` + + serverCert := `-----BEGIN CERTIFICATE----- +MIIBwjCCAWmgAwIBAgIQGIPRUsQ/sFI1fkxZHCSU6jAKBggqhkjOPQQDAjAkMSIw +IAYDVQQDExlFeGFtcGxlIEludGVybWVkaWF0ZSBDQSAxMB4XDTIwMTIwMzAwMTAz +NloXDTIwMTIwNDAwMTAzNlowFjEUMBIGA1UEAxMLZXhhbXBsZS5jb20wWTATBgcq +hkjOPQIBBggqhkjOPQMBBwNCAAS0FvMzMHAfc6mOIEgijRngeRcNaDdp63AbCVeJ +tuKNX7j4KLbkQcACj6g+hblJu4NCJChFmeEYf8b7xw+q0dPOo4GKMIGHMA4GA1Ud +DwEB/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwHQYDVR0O +BBYEFIRRQ0915ExZz30TeVhCpwgP3SEYMB8GA1UdIwQYMBaAFH7YljRWGOtaytSe +D5Xx2B5kE4YnMBYGA1UdEQQPMA2CC2V4YW1wbGUuY29tMAoGCCqGSM49BAMCA0cA +MEQCIH9NxXnWaip9fZyv9VJcfFz7tcdxTq10SrTO7gKhyJkpAiAljZFFK687kc6J +kzqEt441cQasPp5ohL5U4cJN6lAuwA== +-----END CERTIFICATE----- +` + + privateKey := `-----BEGIN PRIVATE KEY----- +MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQC17SizxfpkpsRR +agUl1J9xihKi9vE8GG2KwQl39T6g4rWMof8cwGWJO/72Xo68iGV223jy/nuaTg/Q +zvYpPy6VdwFqw1X3kM2c+wSkCqzvAquF+2V54Mwo+eTxW9TqOJ0/LpI7o21uny0l +yGtt8zBlbt7bL49XB4o3xDA0QJA9/KAG1fyM/3cIbqhi8r0UNLjlTVRwpR3FMt9e +ZK85E+jQcURcs29nGZ2NKys/ShJtqwl8yvTYNlvn/oxLEJsn8Wc/B2pdAf9wzqtA +MvDYizmJafRBc8Rhkyphu0wAmhjRQRUdUeAhRv8e5xD5ISS23qQbewgYqwouK7KZ +kajNEU2rAgMBAAECggEBAK9MJxUapkxH+RDt1KoAN+aigZSv2ADtFNhHa0VAdal2 +6jLpgbWFmhDjU6i3slfuIb6meePC3PzxTQIJ+l4COHPi6OWj9PkIeWdS5MTgWIQx +kW8Xr08CEhdFu5npv7408SgJSvTWY8Lc9BbdCM84LqD+dRTEvhzA8ikMDNq8f4CJ +hLreFUUl/udHacpMdE8mpB6vgCUliZEjBlHHC9qD2mDKgWb0cm4jkO9PcHxz8CXL +szcRV2vqTwvsJcZWcJwTzjhFxq/lUZrgbwpn60iKlov3BCRoTJBppOXi01giom3v +Wz7Y7DoFbHfizh6jyBrf3ODhKJQ3CGvS65QCS0aJ/kECgYEA4JuGC9DpQYmlzWbV +0CqJYnTcZKqcPQx/A1QZDKot0VWqF61vZIku5XuoGKGfY3eLwVZJJZqxoTlVTbuT +nNzYJe+EHzftRoUxUqXZtIh9VdirJMwCu4RMdwk705FA8+8FcTKXarKWBbAzUmFi +iINR2rlRJHVyh2cOA9hWPbEXX0sCgYEAz1qAYUIMBGnccY9mALGArWTmtyNN3DcB +9tl3/5SzfL1NlcOWsBXdZL61oTl9LhOjqHGZbf164/uFuKNHTsT1E63180UKujmV +TbHL6N6MrMctaJfgru3+XprTMd5pwjzd8huX603OtS8Gvn5gKdBRkG1ZI8CrfTl6 +sJI9YRvl7yECgYEAjUIiptHHsVkhdrIDLL1j1BEM/x6xzk9KnkxIyMdKs4n9xJBm +K0N/xAHmMT+Mn6DyuzBKJqVIq84EETQ0XQYjxpABdyTUTHK+F22JItpogRIYaLcJ +zOcitAaRoriKsh+UO6IGyqrwYTl0vY3Ty2lTlIzSNGzND81HajGn43q56UsCgYEA +pGqArZ3vZXiDgdBQ82/MNrFxd/oYfOtpNVFPI2vHvrtkT8KdM9bCjGXkI4kwR17v +QFuDa4G49hm0+KkPm9f09LvV8CXo0a1jRA4dP/Nn3IC68tqrIEo6js15dWuEtK4K +1zUmC0DRDT3SvS38FmvGoRzzt7PIxyzSqjvrS5sRgcECgYAQ6b0YsM4p+89s4ALK +BPfGIKpoIEMKUcwiT3ovRrwIu1vbu70WRcYAi5do6rwOakp3FyUcQznkeZEOAQmc +xrBy8R64vg83WMuRITAqY6vartSa3oehqUHW0YbhGDVEtSrolXEs5elArUHbpYnX +SIVZww73PTGisLmXfIvKvr8GBA== +-----END PRIVATE KEY----- +` + + cases := []struct { + desc string + certAndKey string + expectedKey string + expectedCert string + }{ + { + desc: "cert and key", + certAndKey: serverCert + privateKey, + expectedCert: serverCert, + expectedKey: privateKey, + }, + { + desc: "cert chain", + certAndKey: rootCACert + intermediateCert + serverCert + privateKey, + expectedCert: rootCACert + intermediateCert + serverCert, + expectedKey: privateKey, + }, + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + cert, key := splitCertAndKey(tc.certAndKey) + if cert != tc.expectedCert { + t.Errorf("splitCertAndKey() = \n%q, want \n%q", cert, tc.expectedCert) + } + if key != tc.expectedKey { + t.Errorf("splitCertAndKey() = \n%q, want \n%q", key, tc.expectedKey) + } + }) + } +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 65e2efc0c..1a3edd759 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -24,9 +24,9 @@ type CSIDriverProviderServer struct { } // New returns an instance of CSIDriverProviderServer -func New() *CSIDriverProviderServer { +func New(constructPEMChain, writeCertAndKeyInSeparateFiles bool) *CSIDriverProviderServer { return &CSIDriverProviderServer{ - provider: provider.NewProvider(), + provider: provider.NewProvider(constructPEMChain, writeCertAndKeyInSeparateFiles), } } diff --git a/test/e2e/certificates_test.go b/test/e2e/certificates_test.go index 8c5c8498f..e250de7c7 100644 --- a/test/e2e/certificates_test.go +++ b/test/e2e/certificates_test.go @@ -168,7 +168,7 @@ var _ = Describe("When fetching certificates and private key from Key Vault", fu cmd = getPodExecCommand("cat /mnt/secrets-store/pemcert1-secret") out, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) Expect(err).To(BeNil()) - certificates.ValidateCertBundle(out, pubKey, "test.domain.com") + certificates.ValidateCertBundle(out, pubKey, out, "test.domain.com") }) It("should read pkcs12 cert, private and public key from pod", func() { @@ -192,7 +192,7 @@ var _ = Describe("When fetching certificates and private key from Key Vault", fu cmd = getPodExecCommand("cat /mnt/secrets-store/pkcs12cert1-secret") out, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) Expect(err).To(BeNil()) - certificates.ValidateCertBundle(out, pubKey, "test.domain.com") + certificates.ValidateCertBundle(out, pubKey, out, "test.domain.com") cmd = getPodExecCommand("cat /mnt/secrets-store/pkcs12cert1-secret-pfx") out, err = exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) @@ -204,7 +204,7 @@ var _ = Describe("When fetching certificates and private key from Key Vault", fu pem, err := openssl.ParsePKCS12(string(pfxRaw), "") Expect(err).To(BeNil()) - certificates.ValidateCertBundle(pem, pubKey, "test.domain.com") + certificates.ValidateCertBundle(pem, pubKey, pem, "test.domain.com") }) It("should read ecc cert, private and public key from pod", func() { @@ -228,7 +228,7 @@ var _ = Describe("When fetching certificates and private key from Key Vault", fu cmd = getPodExecCommand("cat /mnt/secrets-store/ecccert1-secret") out, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) Expect(err).To(BeNil()) - certificates.ValidateCertBundle(out, pubKey, "") + certificates.ValidateCertBundle(out, pubKey, out, "") cmd = getPodExecCommand("cat /mnt/secrets-store/ecccert1-secret-pfx") out, err = exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) @@ -240,6 +240,34 @@ var _ = Describe("When fetching certificates and private key from Key Vault", fu pem, err := openssl.ParsePKCS12(string(pfxRaw), "") Expect(err).To(BeNil()) - certificates.ValidateCertBundle(pem, pubKey, "") + certificates.ValidateCertBundle(pem, pubKey, pem, "") + }) + + Describe("[Feature:WriteCertAndKeyInSeparateFiles] Writing certificates and private key in separate files", func() { + It("should write cert and key in separate files", func() { + pod.WaitFor(pod.WaitForInput{ + Getter: kubeClient, + KubeconfigPath: kubeconfigPath, + Config: config, + PodName: p.Name, + Namespace: ns.Name, + }) + + // validate pemcert1 + cmd := getPodExecCommand("cat /mnt/secrets-store/pemcert1-secret.crt") + cert, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) + Expect(err).To(BeNil()) + certificates.ValidateCert(cert, "test.domain.com") + + cmd = getPodExecCommand("cat /mnt/secrets-store/pemcert1-pub-key") + pubKey, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) + Expect(err).To(BeNil()) + + cmd = getPodExecCommand("cat /mnt/secrets-store/pemcert1-secret.key") + privKey, err := exec.KubectlExec(kubeconfigPath, p.Name, p.Namespace, strings.Split(cmd, " ")) + Expect(err).To(BeNil()) + + certificates.ValidateCertBundle(cert, pubKey, privKey, "test.domain.com") + }) }) }) diff --git a/test/e2e/framework/certificates/certificates.go b/test/e2e/framework/certificates/certificates.go index 6b1650e52..dcbefa9d6 100644 --- a/test/e2e/framework/certificates/certificates.go +++ b/test/e2e/framework/certificates/certificates.go @@ -46,12 +46,12 @@ func ValidateCert(certData, dnsName string) { // ValidateCertBundle validates the certificate, public key and private key returned by the provider match // and are usable -func ValidateCertBundle(data, publicKey, dnsName string) { +func ValidateCertBundle(data, publicKey, privKey, dnsName string) { By(fmt.Sprintf("Ensuring certificate and private key is valid for dns name %s", dnsName)) certPEMBlock, err := getCert([]byte(data)) Expect(err).To(BeNil()) - keyPEMBlock, err := getPrivateKey([]byte(data)) + keyPEMBlock, err := getPrivateKey([]byte(privKey)) Expect(err).To(BeNil()) certs, err := X509KeyPair(certPEMBlock, keyPEMBlock, []byte(publicKey), []byte{}) diff --git a/test/e2e/framework/config.go b/test/e2e/framework/config.go index d18652e93..ee1ed7a42 100644 --- a/test/e2e/framework/config.go +++ b/test/e2e/framework/config.go @@ -18,7 +18,7 @@ type Config struct { KeyvaultName string `envconfig:"KEYVAULT_NAME"` Registry string `envconfig:"REGISTRY" default:"mcr.microsoft.com/oss/azure/secrets-store"` ImageName string `envconfig:"IMAGE_NAME" default:"provider-azure"` - ImageVersion string `envconfig:"IMAGE_VERSION" default:"v1.3.0` + ImageVersion string `envconfig:"IMAGE_VERSION" default:"v1.3.0"` IsSoakTest bool `envconfig:"IS_SOAK_TEST" default:"false"` IsWindowsTest bool `envconfig:"TEST_WINDOWS" default:"false"` IsGPUTest bool `envconfig:"TEST_GPU" default:"false"` diff --git a/test/e2e/framework/deploy/deploy.go b/test/e2e/framework/deploy/deploy.go index 697a79c60..93a6603a1 100644 --- a/test/e2e/framework/deploy/deploy.go +++ b/test/e2e/framework/deploy/deploy.go @@ -102,6 +102,8 @@ func InstallManifest(kubeconfigPath string, config *framework.Config) { // Configure higher log verbosity for debugging CI failures ds.Spec.Template.Spec.Containers[0].Args = append(ds.Spec.Template.Spec.Containers[0].Args, "-v=5") + // Configure writeCertAndKeyInSeparateFiles to true as it's feature on top of default behavior + ds.Spec.Template.Spec.Containers[0].Args = append(ds.Spec.Template.Spec.Containers[0].Args, "--write-cert-and-key-in-separate-files=true") updatedDS, err := yaml.Marshal(ds) Expect(err).To(BeNil()) diff --git a/test/e2e/framework/helm/helm.go b/test/e2e/framework/helm/helm.go index 36c82bc86..8d120b17c 100644 --- a/test/e2e/framework/helm/helm.go +++ b/test/e2e/framework/helm/helm.go @@ -55,6 +55,7 @@ func Install(input InstallInput) { fmt.Sprintf("--set=logVerbosity=5"), fmt.Sprintf("--set=linux.customUserAgent=csi-e2e"), fmt.Sprintf("--set=windows.customUserAgent=csi-e2e"), + fmt.Sprintf("--set=writeCertAndKeyInSeparateFiles=true"), "--dependency-update", "--wait", "--timeout=5m", @@ -115,6 +116,7 @@ func Upgrade(input UpgradeInput) { fmt.Sprintf("--set=logVerbosity=1"), fmt.Sprintf("--set=linux.customUserAgent=csi-e2e"), fmt.Sprintf("--set=windows.customUserAgent=csi-e2e"), + fmt.Sprintf("--set=writeCertAndKeyInSeparateFiles=true"), "--wait", "--timeout=5m", "--debug",