From 1a6bc03bdc7a18a780b4ed212cc8c26b9e0d791a Mon Sep 17 00:00:00 2001 From: Cody Soyland Date: Tue, 10 Dec 2024 19:10:29 -0500 Subject: [PATCH] Rename GetCertificate to Certificate (#349) * Remove public `Certificate` field from `bundle.Certificate` Signed-off-by: Cody Soyland * Rename GetCertificate to Certificate Getters should not be prefixed with `Get` per standard Go practices (cited by Effective Go). This PR renames `GetCertificate` to `Certificate`. Signed-off-by: Cody Soyland --------- Signed-off-by: Cody Soyland --- pkg/bundle/bundle.go | 4 ++-- pkg/bundle/bundle_test.go | 2 +- pkg/bundle/verification_content.go | 16 ++++++++++------ pkg/fulcio/certificate/summarize_test.go | 6 +++--- pkg/testing/ca/ca.go | 2 +- pkg/verify/interface.go | 2 +- pkg/verify/signature.go | 2 +- pkg/verify/signed_entity.go | 4 ++-- 8 files changed, 21 insertions(+), 17 deletions(-) diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index 21c4c368..f0c86278 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -255,7 +255,7 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) { return nil, ErrValidationError(err) } cert := &Certificate{ - Certificate: parsedCert, + certificate: parsedCert, } return cert, nil case *protobundle.VerificationMaterial_Certificate: @@ -267,7 +267,7 @@ func (b *Bundle) VerificationContent() (verify.VerificationContent, error) { return nil, ErrValidationError(err) } cert := &Certificate{ - Certificate: parsedCert, + certificate: parsedCert, } return cert, nil case *protobundle.VerificationMaterial_PublicKey: diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go index aa4bfbbd..6ef272d6 100644 --- a/pkg/bundle/bundle_test.go +++ b/pkg/bundle/bundle_test.go @@ -814,7 +814,7 @@ func TestVerificationContent(t *testing.T) { } require.NoError(t, gotErr) if tt.wantCertificate { - require.NotNil(t, got.GetCertificate()) + require.NotNil(t, got.Certificate()) return } if tt.wantPublicKey { diff --git a/pkg/bundle/verification_content.go b/pkg/bundle/verification_content.go index 94111150..edb65db3 100644 --- a/pkg/bundle/verification_content.go +++ b/pkg/bundle/verification_content.go @@ -24,7 +24,11 @@ import ( ) type Certificate struct { - *x509.Certificate + certificate *x509.Certificate +} + +func NewCertificate(cert *x509.Certificate) *Certificate { + return &Certificate{certificate: cert} } type PublicKey struct { @@ -41,15 +45,15 @@ func (c *Certificate) CompareKey(key any, _ root.TrustedMaterial) bool { return false } - return c.Certificate.Equal(x509Key) + return c.certificate.Equal(x509Key) } func (c *Certificate) ValidAtTime(t time.Time, _ root.TrustedMaterial) bool { - return !(c.Certificate.NotAfter.Before(t) || c.Certificate.NotBefore.After(t)) + return !(c.certificate.NotAfter.Before(t) || c.certificate.NotBefore.After(t)) } -func (c *Certificate) GetCertificate() *x509.Certificate { - return c.Certificate +func (c *Certificate) Certificate() *x509.Certificate { + return c.certificate } func (c *Certificate) PublicKey() verify.PublicKeyProvider { @@ -79,7 +83,7 @@ func (pk *PublicKey) ValidAtTime(t time.Time, tm root.TrustedMaterial) bool { return verifier.ValidAtTime(t) } -func (pk *PublicKey) GetCertificate() *x509.Certificate { +func (pk *PublicKey) Certificate() *x509.Certificate { return nil } diff --git a/pkg/fulcio/certificate/summarize_test.go b/pkg/fulcio/certificate/summarize_test.go index a891f514..3c761ae7 100644 --- a/pkg/fulcio/certificate/summarize_test.go +++ b/pkg/fulcio/certificate/summarize_test.go @@ -30,7 +30,7 @@ func TestSummarizeCertificateWithActionsBundle(t *testing.T) { t.Fatalf("failed to get verification content: %v", err) } - leaf := vc.GetCertificate() + leaf := vc.Certificate() if leaf == nil { t.Fatalf("expected verification content to be a certificate chain") @@ -79,7 +79,7 @@ func TestSummarizeCertificateWithOauthBundle(t *testing.T) { t.Fatalf("failed to get verification content: %v", err) } - leaf := vc.GetCertificate() + leaf := vc.Certificate() if leaf == nil { t.Fatalf("expected verification content to be a certificate chain") @@ -108,7 +108,7 @@ func TestSummarizeCertificateWithOtherNameSAN(t *testing.T) { t.Fatalf("failed to get verification content: %v", err) } - leaf := vc.GetCertificate() + leaf := vc.Certificate() if leaf == nil { t.Fatalf("expected verification content to be a certificate chain") diff --git a/pkg/testing/ca/ca.go b/pkg/testing/ca/ca.go index d2d010a6..23dcadae 100644 --- a/pkg/testing/ca/ca.go +++ b/pkg/testing/ca/ca.go @@ -516,7 +516,7 @@ type TestEntity struct { } func (e *TestEntity) VerificationContent() (verify.VerificationContent, error) { - return &bundle.Certificate{Certificate: e.certChain[0]}, nil + return bundle.NewCertificate(e.certChain[0]), nil } func (e *TestEntity) HasInclusionPromise() bool { diff --git a/pkg/verify/interface.go b/pkg/verify/interface.go index 6b0ac27f..2ff08034 100644 --- a/pkg/verify/interface.go +++ b/pkg/verify/interface.go @@ -63,7 +63,7 @@ type SignedEntity interface { type VerificationContent interface { CompareKey(any, root.TrustedMaterial) bool ValidAtTime(time.Time, root.TrustedMaterial) bool - GetCertificate() *x509.Certificate + Certificate() *x509.Certificate PublicKey() PublicKeyProvider } diff --git a/pkg/verify/signature.go b/pkg/verify/signature.go index 4e4dc97f..c7056c44 100644 --- a/pkg/verify/signature.go +++ b/pkg/verify/signature.go @@ -94,7 +94,7 @@ func VerifySignatureWithArtifactDigest(sigContent SignatureContent, verification } func getSignatureVerifier(verificationContent VerificationContent, tm root.TrustedMaterial) (signature.Verifier, error) { - if leafCert := verificationContent.GetCertificate(); leafCert != nil { + if leafCert := verificationContent.Certificate(); leafCert != nil { // TODO: Inspect certificate's SignatureAlgorithm to determine hash function return signature.LoadVerifier(leafCert.PublicKey, crypto.SHA256) } else if pk := verificationContent.PublicKey(); pk != nil { diff --git a/pkg/verify/signed_entity.go b/pkg/verify/signed_entity.go index 1501ce6f..ec4aa5b9 100644 --- a/pkg/verify/signed_entity.go +++ b/pkg/verify/signed_entity.go @@ -508,7 +508,7 @@ func (v *SignedEntityVerifier) Verify(entity SignedEntity, pb PolicyBuilder) (*V // If the bundle was signed with a long-lived key, and does not have a Fulcio certificate, // then skip the certificate verification steps - if leafCert := verificationContent.GetCertificate(); leafCert != nil { + if leafCert := verificationContent.Certificate(); leafCert != nil { if policy.WeExpectSigningKey() { return nil, errors.New("expected key signature, not certificate") } @@ -719,7 +719,7 @@ func (v *SignedEntityVerifier) VerifyObserverTimestamps(entity SignedEntity, log return nil, err } - if leafCert := verificationContent.GetCertificate(); leafCert != nil { + if leafCert := verificationContent.Certificate(); leafCert != nil { verifiedTimestamps = append(verifiedTimestamps, TimestampVerificationResult{Type: "LeafCert.NotBefore", URI: "", Timestamp: leafCert.NotBefore}) } else { // no cert? use current time