From d5295b297cd65df70b9d7160938727fcaa1922cf Mon Sep 17 00:00:00 2001 From: cornfeedhobo Date: Wed, 21 Dec 2022 22:07:40 -0600 Subject: [PATCH 1/7] Add attribute support for certificate subject Fixes #128 Signed-off-by: cornfeedhobo --- pkg/apis/v1alpha1/types.go | 12 +++++- pkg/apis/validation/validation_test.go | 19 +++++++++ pkg/requestgen/generator.go | 59 +++++++++++++++++++------- pkg/requestgen/generator_test.go | 35 +++++++++++++++ 4 files changed, 108 insertions(+), 17 deletions(-) diff --git a/pkg/apis/v1alpha1/types.go b/pkg/apis/v1alpha1/types.go index 3802f8c8..4ba05ce5 100644 --- a/pkg/apis/v1alpha1/types.go +++ b/pkg/apis/v1alpha1/types.go @@ -21,7 +21,17 @@ const ( IssuerKindKey = "csi.cert-manager.io/issuer-kind" IssuerGroupKey = "csi.cert-manager.io/issuer-group" - CommonNameKey = "csi.cert-manager.io/common-name" + LiteralSubjectKey = "csi.cert-manager.io/literal-subject" + CommonNameKey = "csi.cert-manager.io/common-name" + OrganizationsKey = "csi.cert-manager.io/organizations" + OrganizationalUnitsKey = "csi.cert-manager.io/organizationalunits" + CountriesKey = "csi.cert-manager.io/countries" + ProvincesKey = "csi.cert-manager.io/provinces" + LocalitiesKey = "csi.cert-manager.io/localities" + StreetAddressesKey = "csi.cert-manager.io/streetaddresses" + PostalCodesKey = "csi.cert-manager.io/postalcodes" + SerialNumberKey = "csi.cert-manager.io/serialnumber" + DNSNamesKey = "csi.cert-manager.io/dns-names" IPSANsKey = "csi.cert-manager.io/ip-sans" URISANsKey = "csi.cert-manager.io/uri-sans" diff --git a/pkg/apis/validation/validation_test.go b/pkg/apis/validation/validation_test.go index 79bae924..bbb0f688 100644 --- a/pkg/apis/validation/validation_test.go +++ b/pkg/apis/validation/validation_test.go @@ -27,6 +27,13 @@ import ( ) func Test_ValidateAttributes(t *testing.T) { + type vaT struct { + attr map[string]string + expError error + } + + var literalSubject = "CN=${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local,OU=0:${POD_NAME}\\;1:${POD_NAMESPACE}\\;2:my-region\\;4:unittest,O=foo.bar.com" + tests := map[string]struct { attr map[string]string expErr field.ErrorList @@ -201,6 +208,18 @@ func Test_ValidateAttributes(t *testing.T) { field.Invalid(field.NewPath("volumeAttributes", "csi.cert-manager.io/privatekey-file"), "/foobar", "filename must not include '/'"), }, }, + "correct literal-subject should not error": { + attr: map[string]string{ + csiapi.IssuerNameKey: "test-issuer", + csiapi.LiteralSubjectKey: literalSubject, + csiapi.CAFileKey: "ca.crt", + csiapi.CertFileKey: "crt.tls", + csiapi.KeyFileKey: "key.tls", + csiapi.DNSNamesKey: "foo.bar.com", + csiapi.KeyEncodingKey: "PKCS8", + }, + expErr: nil, + }, } for name, test := range tests { diff --git a/pkg/requestgen/generator.go b/pkg/requestgen/generator.go index 5999e8ae..dfe832a3 100644 --- a/pkg/requestgen/generator.go +++ b/pkg/requestgen/generator.go @@ -29,6 +29,7 @@ import ( cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/csi-lib/manager" "github.com/cert-manager/csi-lib/metadata" @@ -57,21 +58,54 @@ func RequestForMetadata(meta metadata.Metadata) (*manager.CertificateRequestBund } } - commonName, err := expand(meta, attrs[csiapi.CommonNameKey]) - if err != nil { - return nil, fmt.Errorf("%q: %w", csiapi.CommonNameKey, err) + var request = &x509.CertificateRequest{} + if lSubjStr, ok := attrs[csiapi.LiteralSubjectKey]; ok && len(lSubjStr) > 0 { + lSubjStr, err = expand(meta, lSubjStr) + if err != nil { + return nil, fmt.Errorf("%q: %w", csiapi.LiteralSubjectKey, err) + } + request.RawSubject, err = cmpki.ParseSubjectStringToRawDerBytes(lSubjStr) + if err != nil { + return nil, fmt.Errorf("%q: %w", csiapi.LiteralSubjectKey, err) + } + } else { + request.Subject = pkix.Name{} + request.Subject.CommonName, err = expand(meta, attrs[csiapi.CommonNameKey]) + if err != nil { + return nil, fmt.Errorf("%q: %w", csiapi.CommonNameKey, err) + } + if len(attrs[csiapi.SerialNumberKey]) > 0 { + request.Subject.SerialNumber = attrs[csiapi.SerialNumberKey] + } + for k, v := range map[*[]string]string{ + &request.Subject.Organization: csiapi.OrganizationsKey, + &request.Subject.OrganizationalUnit: csiapi.OrganizationalUnitsKey, + &request.Subject.Country: csiapi.CountriesKey, + &request.Subject.Province: csiapi.ProvincesKey, + &request.Subject.Locality: csiapi.LocalitiesKey, + &request.Subject.StreetAddress: csiapi.StreetAddressesKey, + &request.Subject.PostalCode: csiapi.PostalCodesKey, + } { + if len(attrs[v]) > 0 { + var e, err = expand(meta, attrs[v]) + if err != nil { + return nil, fmt.Errorf("%q: %w", v, err) + } + *k = strings.Split(e, ",") + } + } } - dns, err := parseDNSNames(meta, attrs[csiapi.DNSNamesKey]) + request.DNSNames, err = parseDNSNames(meta, attrs[csiapi.DNSNamesKey]) if err != nil { return nil, fmt.Errorf("%q: %w", csiapi.DNSNamesKey, err) } - uris, err := parseURIs(meta, attrs[csiapi.URISANsKey]) + request.IPAddresses, err = parseIPAddresses(attrs[csiapi.IPSANsKey]) if err != nil { - return nil, fmt.Errorf("%q: %w", csiapi.URISANsKey, err) + return nil, fmt.Errorf("%q: %w", csiapi.IPSANsKey, err) } - ips, err := parseIPAddresses(attrs[csiapi.IPSANsKey]) + request.URIs, err = parseURIs(meta, attrs[csiapi.URISANsKey]) if err != nil { - return nil, fmt.Errorf("%q: %w", csiapi.IPSANsKey, err) + return nil, fmt.Errorf("%q: %w", csiapi.URISANsKey, err) } annotations := make(map[string]string) @@ -88,14 +122,7 @@ func RequestForMetadata(meta metadata.Metadata) (*manager.CertificateRequestBund } return &manager.CertificateRequestBundle{ - Request: &x509.CertificateRequest{ - Subject: pkix.Name{ - CommonName: commonName, - }, - DNSNames: dns, - IPAddresses: ips, - URIs: uris, - }, + Request: request, IsCA: strings.ToLower(attrs[csiapi.IsCAKey]) == "true", Namespace: attrs[csiapi.K8sVolumeContextKeyPodNamespace], Duration: duration, diff --git a/pkg/requestgen/generator_test.go b/pkg/requestgen/generator_test.go index 19ebb020..f9824af8 100644 --- a/pkg/requestgen/generator_test.go +++ b/pkg/requestgen/generator_test.go @@ -22,14 +22,18 @@ import ( "errors" "net" "net/url" + "strings" "testing" "time" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/csi-lib/manager" "github.com/cert-manager/csi-lib/metadata" "github.com/stretchr/testify/assert" + csiapi "github.com/cert-manager/csi-driver/pkg/apis/v1alpha1" + ) func Test_RequestForMetadata(t *testing.T) { @@ -51,6 +55,12 @@ func Test_RequestForMetadata(t *testing.T) { return puri } + var literalSubject = "CN=my-pod.my-namespace.svc.cluster.local,OU=0:my-pod\\;1:my-namespace\\;2:my-region\\;4:unittest,O=foo.bar.com" + var rawLiteralSubject, err = cmpki.ParseSubjectStringToRawDerBytes(literalSubject) + if err != nil { + assert.NoError(t, err) + } + tests := map[string]struct { meta metadata.Metadata expRequest *manager.CertificateRequestBundle @@ -175,6 +185,31 @@ func Test_RequestForMetadata(t *testing.T) { }, expErr: false, }, + "a metadata with literal subject set should be returned": { + meta: baseMetadataWith(metadata.Metadata{VolumeContext: map[string]string{ + csiapi.IssuerNameKey: "my-issuer", + csiapi.LiteralSubjectKey: literalSubject, + }}), + expRequest: &manager.CertificateRequestBundle{ + Request: &x509.CertificateRequest{RawSubject: rawLiteralSubject}, + Usages: cmapi.DefaultKeyUsages(), + Namespace: "my-namespace", + IssuerRef: cmmeta.ObjectReference{ + Name: "my-issuer", + Kind: "Issuer", + Group: "cert-manager.io", + }, + Duration: cmapi.DefaultCertificateDuration, + }, + expErr: false, + }, + "a metadata with incorrect literal subject set should error": { + meta: baseMetadataWith(metadata.Metadata{VolumeContext: map[string]string{ + csiapi.IssuerNameKey: "my-issuer", + csiapi.LiteralSubjectKey: strings.Replace(literalSubject, ";", "&", -1), + }}), + expErr: true, + }, } for name, test := range tests { From 89d8d00247ec4e6ddc713758523540810853b426 Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 27 Mar 2024 16:38:03 +0100 Subject: [PATCH 2/7] fix: replace usage of ParseSubjectStringToRawDerBytes Signed-off-by: nzbr --- pkg/requestgen/generator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/requestgen/generator.go b/pkg/requestgen/generator.go index dfe832a3..952ca889 100644 --- a/pkg/requestgen/generator.go +++ b/pkg/requestgen/generator.go @@ -64,7 +64,11 @@ func RequestForMetadata(meta metadata.Metadata) (*manager.CertificateRequestBund if err != nil { return nil, fmt.Errorf("%q: %w", csiapi.LiteralSubjectKey, err) } - request.RawSubject, err = cmpki.ParseSubjectStringToRawDerBytes(lSubjStr) + rdnSequence, err := cmpki.UnmarshalSubjectStringToRDNSequence(lSubjStr) + if err != nil { + return nil, fmt.Errorf("%q: %w", csiapi.LiteralSubjectKey, err) + } + request.RawSubject, err = cmpki.MarshalRDNSequenceToRawDERBytes(rdnSequence) if err != nil { return nil, fmt.Errorf("%q: %w", csiapi.LiteralSubjectKey, err) } From c501bb50b4c1860115337911792c436727bef340 Mon Sep 17 00:00:00 2001 From: nzbr Date: Wed, 27 Mar 2024 17:09:49 +0100 Subject: [PATCH 3/7] Use CSV split function instead of strings.Split Signed-off-by: nzbr --- pkg/requestgen/generator.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/requestgen/generator.go b/pkg/requestgen/generator.go index 952ca889..40419e67 100644 --- a/pkg/requestgen/generator.go +++ b/pkg/requestgen/generator.go @@ -29,6 +29,7 @@ import ( cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + cmutil "github.com/cert-manager/cert-manager/pkg/util" cmpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/csi-lib/manager" "github.com/cert-manager/csi-lib/metadata" @@ -95,7 +96,10 @@ func RequestForMetadata(meta metadata.Metadata) (*manager.CertificateRequestBund if err != nil { return nil, fmt.Errorf("%q: %w", v, err) } - *k = strings.Split(e, ",") + *k, err = cmutil.SplitWithEscapeCSV(e) + if err != nil { + return nil, fmt.Errorf("%q: %w", v, err) + } } } } From b19503a3a134444af1610cdeb2f1edf4c8c090f9 Mon Sep 17 00:00:00 2001 From: nzbr Date: Fri, 10 May 2024 18:33:35 +0200 Subject: [PATCH 4/7] test: unit test used removed function Signed-off-by: nzbr --- pkg/requestgen/generator_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/requestgen/generator_test.go b/pkg/requestgen/generator_test.go index f9824af8..830e3806 100644 --- a/pkg/requestgen/generator_test.go +++ b/pkg/requestgen/generator_test.go @@ -56,7 +56,12 @@ func Test_RequestForMetadata(t *testing.T) { } var literalSubject = "CN=my-pod.my-namespace.svc.cluster.local,OU=0:my-pod\\;1:my-namespace\\;2:my-region\\;4:unittest,O=foo.bar.com" - var rawLiteralSubject, err = cmpki.ParseSubjectStringToRawDerBytes(literalSubject) + rdnSequence, err := cmpki.UnmarshalSubjectStringToRDNSequence(literalSubject) + if err != nil { + assert.NoError(t, err) + } + + rawLiteralSubject, err := cmpki.MarshalRDNSequenceToRawDERBytes(rdnSequence) if err != nil { assert.NoError(t, err) } From 873c5041706912e7aa869e51f1d39bca08246df1 Mon Sep 17 00:00:00 2001 From: nzbr Date: Fri, 10 May 2024 18:38:22 +0200 Subject: [PATCH 5/7] test: Annotations field was not initialized Signed-off-by: nzbr --- pkg/requestgen/generator_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/requestgen/generator_test.go b/pkg/requestgen/generator_test.go index 830e3806..9f981d62 100644 --- a/pkg/requestgen/generator_test.go +++ b/pkg/requestgen/generator_test.go @@ -205,6 +205,7 @@ func Test_RequestForMetadata(t *testing.T) { Group: "cert-manager.io", }, Duration: cmapi.DefaultCertificateDuration, + Annotations: make(map[string]string), }, expErr: false, }, From b90736528088ac7fdf4fb33af27bc1c91eb68986 Mon Sep 17 00:00:00 2001 From: nzbr Date: Fri, 10 May 2024 18:58:27 +0200 Subject: [PATCH 6/7] test: remove unused struct vaT Signed-off-by: nzbr --- pkg/apis/validation/validation_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/pkg/apis/validation/validation_test.go b/pkg/apis/validation/validation_test.go index bbb0f688..6a6987f7 100644 --- a/pkg/apis/validation/validation_test.go +++ b/pkg/apis/validation/validation_test.go @@ -27,11 +27,6 @@ import ( ) func Test_ValidateAttributes(t *testing.T) { - type vaT struct { - attr map[string]string - expError error - } - var literalSubject = "CN=${POD_NAME}.${POD_NAMESPACE}.svc.cluster.local,OU=0:${POD_NAME}\\;1:${POD_NAMESPACE}\\;2:my-region\\;4:unittest,O=foo.bar.com" tests := map[string]struct { From 41fcd832fed470fa85f9e319047aff5c55a800b8 Mon Sep 17 00:00:00 2001 From: nzbr Date: Fri, 10 May 2024 18:58:59 +0200 Subject: [PATCH 7/7] test: Use ReplaceAll, format Signed-off-by: nzbr --- pkg/requestgen/generator_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/requestgen/generator_test.go b/pkg/requestgen/generator_test.go index 9f981d62..33d75e7b 100644 --- a/pkg/requestgen/generator_test.go +++ b/pkg/requestgen/generator_test.go @@ -28,12 +28,12 @@ import ( cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" - cmpki "github.com/cert-manager/cert-manager/pkg/util/pki" + cmpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/csi-lib/manager" "github.com/cert-manager/csi-lib/metadata" "github.com/stretchr/testify/assert" - csiapi "github.com/cert-manager/csi-driver/pkg/apis/v1alpha1" + csiapi "github.com/cert-manager/csi-driver/pkg/apis/v1alpha1" ) func Test_RequestForMetadata(t *testing.T) { @@ -204,7 +204,7 @@ func Test_RequestForMetadata(t *testing.T) { Kind: "Issuer", Group: "cert-manager.io", }, - Duration: cmapi.DefaultCertificateDuration, + Duration: cmapi.DefaultCertificateDuration, Annotations: make(map[string]string), }, expErr: false, @@ -212,7 +212,7 @@ func Test_RequestForMetadata(t *testing.T) { "a metadata with incorrect literal subject set should error": { meta: baseMetadataWith(metadata.Metadata{VolumeContext: map[string]string{ csiapi.IssuerNameKey: "my-issuer", - csiapi.LiteralSubjectKey: strings.Replace(literalSubject, ";", "&", -1), + csiapi.LiteralSubjectKey: strings.ReplaceAll(literalSubject, ";", "&"), }}), expErr: true, },