From 425f8bd6c3d69b4ce22faf386ede1b9581f92be5 Mon Sep 17 00:00:00 2001 From: linus-sun Date: Tue, 12 Nov 2024 17:24:29 +0000 Subject: [PATCH] refactor extension functions to support x509 and google_x509 Signed-off-by: linus-sun --- pkg/ct/identity.go | 67 -------------------- pkg/ct/identity_test.go | 56 ----------------- pkg/ct/monitor.go | 9 +-- pkg/ct/monitor_test.go | 4 +- pkg/ct/test_utils.go | 6 +- pkg/identity/identity.go | 113 ++++++++++++++++++++++++---------- pkg/identity/identity_test.go | 32 ++++++++++ 7 files changed, 121 insertions(+), 166 deletions(-) delete mode 100644 pkg/ct/identity.go delete mode 100644 pkg/ct/identity_test.go diff --git a/pkg/ct/identity.go b/pkg/ct/identity.go deleted file mode 100644 index 1ae3c1ef..00000000 --- a/pkg/ct/identity.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright 2024 The Sigstore Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// This file copies some of the functionality in pkg/identity/identity.go -// related to retrieving OID extension values and matching on them, -// but refactors them to use the Google-specific fork of encoding/asn1 and crypto/x509. - -package ct - -import ( - "encoding/asn1" - "fmt" - - google_asn1 "github.com/google/certificate-transparency-go/asn1" - google_x509 "github.com/google/certificate-transparency-go/x509" -) - -// getExtension gets a certificate extension by OID where the extension value is an -// ASN.1-encoded string -func getExtension(cert *google_x509.Certificate, oid google_asn1.ObjectIdentifier) (string, error) { - for _, ext := range cert.Extensions { - if !ext.Id.Equal(oid) { - continue - } - var extValue string - rest, err := asn1.Unmarshal(ext.Value, &extValue) - if err != nil { - return "", fmt.Errorf("%w", err) - } - if len(rest) != 0 { - return "", fmt.Errorf("unmarshalling extension had rest for oid %v", oid) - } - return extValue, nil - } - return "", nil -} - -// OIDMatchesPolicy returns if a certificate contains both a given OID field and a matching value associated with that field -// if true, it returns the OID extension and extension value that were matched on -func OIDMatchesPolicy(cert *google_x509.Certificate, oid google_asn1.ObjectIdentifier, extensionValues []string) (bool, google_asn1.ObjectIdentifier, string, error) { - extValue, err := getExtension(cert, oid) - if err != nil { - return false, nil, "", fmt.Errorf("error getting extension value: %w", err) - } - if extValue == "" { - return false, nil, "", nil - } - - for _, extensionValue := range extensionValues { - if extValue == extensionValue { - return true, oid, extValue, nil - } - } - - return false, nil, "", nil -} diff --git a/pkg/ct/identity_test.go b/pkg/ct/identity_test.go deleted file mode 100644 index 8ba4a89f..00000000 --- a/pkg/ct/identity_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2024 The Sigstore Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package ct - -import ( - "testing" - - google_asn1 "github.com/google/certificate-transparency-go/asn1" - "github.com/google/certificate-transparency-go/x509" -) - -// Test when OID is not present in the certificate -func TestOIDNotPresent(t *testing.T) { - cert := &x509.Certificate{} // No extensions - oid := google_asn1.ObjectIdentifier{2, 5, 29, 17} - extensionValues := []string{"wrong value"} - - matches, _, _, err := OIDMatchesPolicy(cert, oid, extensionValues) - if matches || err != nil { - t.Errorf("Expected false with nil, got %v, error %v", matches, err) - } -} - -// Test when OID is present and matches value -func TestOIDMatchesValue(t *testing.T) { - cert, err := mockCertificateWithExtension(google_asn1.ObjectIdentifier{2, 5, 29, 17}, "test cert value") - if err != nil { - t.Errorf("Expected nil got %v", err) - } - oid := google_asn1.ObjectIdentifier{2, 5, 29, 17} - extValueString := "test cert value" - extensionValues := []string{extValueString} - - matches, matchedOID, extValue, err := OIDMatchesPolicy(cert, oid, extensionValues) - if !matches || err != nil { - t.Errorf("Expected true, got %v, error %v", matches, err) - } - if matchedOID.String() != oid.String() { - t.Errorf("Expected oid to equal 2.5.29.17, got %s", matchedOID.String()) - } - if extValue != extValueString { - t.Errorf("Expected string to equal 'test cert value', got %s", extValue) - } -} diff --git a/pkg/ct/monitor.go b/pkg/ct/monitor.go index 8347d491..81059d61 100644 --- a/pkg/ct/monitor.go +++ b/pkg/ct/monitor.go @@ -23,8 +23,6 @@ import ( ctclient "github.com/google/certificate-transparency-go/client" "github.com/sigstore/rekor-monitor/pkg/fulcio/extensions" "github.com/sigstore/rekor-monitor/pkg/identity" - - "github.com/google/certificate-transparency-go/asn1" ) func GetCTLogEntries(logClient *ctclient.LogClient, startIndex int, endIndex int) ([]ct.LogEntry, error) { @@ -35,7 +33,7 @@ func GetCTLogEntries(logClient *ctclient.LogClient, startIndex int, endIndex int return entries, nil } -func ScanEntryCertSubject(logEntry ct.LogEntry, monitoredSubjects []string) ([]*identity.LogEntry, error) { +func ScanEntrySubject(logEntry ct.LogEntry, monitoredSubjects []string) ([]*identity.LogEntry, error) { subject := logEntry.X509Cert.Subject.String() matchedEntries := []*identity.LogEntry{} for _, monitoredSub := range monitoredSubjects { @@ -59,10 +57,7 @@ func ScanEntryOIDExtensions(logEntry ct.LogEntry, monitoredOIDMatchers []extensi matchedEntries := []*identity.LogEntry{} cert := logEntry.X509Cert for _, monitoredOID := range monitoredOIDMatchers { - // must cast encoding/asn1 objectIdentifier to google/certificate-transparency-go fork of asn1.ObjectIdentifier - oidIntArray := []int(monitoredOID.ObjectIdentifier) - matchingOID := asn1.ObjectIdentifier(oidIntArray) - match, _, extValue, err := OIDMatchesPolicy(cert, matchingOID, monitoredOID.ExtensionValues) + match, _, extValue, err := identity.OIDMatchesPolicy(cert, monitoredOID.ObjectIdentifier, monitoredOID.ExtensionValues) if err != nil { return nil, fmt.Errorf("error with policy matching at index %d: %w", logEntry.Index, err) } diff --git a/pkg/ct/monitor_test.go b/pkg/ct/monitor_test.go index 315a5e6f..3711abb6 100644 --- a/pkg/ct/monitor_test.go +++ b/pkg/ct/monitor_test.go @@ -33,7 +33,7 @@ const ( organizationName = "test-org" ) -func TestScanEntryCertSubject(t *testing.T) { +func TestScanEntrySubject(t *testing.T) { testCases := map[string]struct { inputEntry ct.LogEntry inputSubjects []string @@ -72,7 +72,7 @@ func TestScanEntryCertSubject(t *testing.T) { } for _, tc := range testCases { - logEntries, err := ScanEntryCertSubject(tc.inputEntry, tc.inputSubjects) + logEntries, err := ScanEntrySubject(tc.inputEntry, tc.inputSubjects) if err != nil { t.Errorf("received error scanning entry for subjects: %v", err) } diff --git a/pkg/ct/test_utils.go b/pkg/ct/test_utils.go index 27400028..cfeedcd3 100644 --- a/pkg/ct/test_utils.go +++ b/pkg/ct/test_utils.go @@ -21,7 +21,7 @@ import ( "testing" google_asn1 "github.com/google/certificate-transparency-go/asn1" - "github.com/google/certificate-transparency-go/x509" + google_x509 "github.com/google/certificate-transparency-go/x509" google_pkix "github.com/google/certificate-transparency-go/x509/pkix" ) @@ -56,12 +56,12 @@ func serveRspAt(t *testing.T, path, rsp string) *httptest.Server { }) } -func mockCertificateWithExtension(oid google_asn1.ObjectIdentifier, value string) (*x509.Certificate, error) { +func mockCertificateWithExtension(oid google_asn1.ObjectIdentifier, value string) (*google_x509.Certificate, error) { extValue, err := google_asn1.Marshal(value) if err != nil { return nil, err } - cert := &x509.Certificate{ + cert := &google_x509.Certificate{ Extensions: []google_pkix.Extension{ { Id: oid, diff --git a/pkg/identity/identity.go b/pkg/identity/identity.go index 698b6f29..f471a53a 100644 --- a/pkg/identity/identity.go +++ b/pkg/identity/identity.go @@ -18,6 +18,7 @@ import ( "crypto/x509" "encoding/asn1" "encoding/json" + "errors" "fmt" "regexp" "strconv" @@ -25,6 +26,9 @@ import ( "github.com/sigstore/rekor-monitor/pkg/fulcio/extensions" "github.com/sigstore/sigstore/pkg/cryptoutils" + + google_asn1 "github.com/google/certificate-transparency-go/asn1" + google_x509 "github.com/google/certificate-transparency-go/x509" ) var ( @@ -193,52 +197,99 @@ func MonitoredValuesExist(mvs MonitoredValues) bool { // getExtension gets a certificate extension by OID where the extension value is an // ASN.1-encoded string -func getExtension(cert *x509.Certificate, oid asn1.ObjectIdentifier) (string, error) { - for _, ext := range cert.Extensions { - if !ext.Id.Equal(oid) { - continue - } - var extValue string - rest, err := asn1.Unmarshal(ext.Value, &extValue) - if err != nil { - return "", fmt.Errorf("%w", err) +func getExtension[Certificate *x509.Certificate | *google_x509.Certificate](certificate Certificate, oid asn1.ObjectIdentifier) (string, error) { + switch cert := any(certificate).(type) { + case *x509.Certificate: + for _, ext := range cert.Extensions { + if !ext.Id.Equal(oid) { + continue + } + var extValue string + rest, err := asn1.Unmarshal(ext.Value, &extValue) + if err != nil { + return "", fmt.Errorf("%w", err) + } + if len(rest) != 0 { + return "", fmt.Errorf("unmarshalling extension had rest for oid %v", oid) + } + return extValue, nil } - if len(rest) != 0 { - return "", fmt.Errorf("unmarshalling extension had rest for oid %v", oid) + return "", nil + case *google_x509.Certificate: + for _, ext := range cert.Extensions { + if !ext.Id.Equal((google_asn1.ObjectIdentifier)(oid)) { + continue + } + var extValue string + rest, err := asn1.Unmarshal(ext.Value, &extValue) + if err != nil { + return "", fmt.Errorf("%w", err) + } + if len(rest) != 0 { + return "", fmt.Errorf("unmarshalling extension had rest for oid %v", oid) + } + return extValue, nil } - return extValue, nil + return "", nil } - return "", nil + return "", errors.New("certificate was neither x509 nor google_x509") } // getDeprecatedExtension gets a certificate extension by OID where the extension value is a raw string -func getDeprecatedExtension(cert *x509.Certificate, oid asn1.ObjectIdentifier) (string, error) { - for _, ext := range cert.Extensions { - if ext.Id.Equal(oid) { - return string(ext.Value), nil +func getDeprecatedExtension[Certificate *x509.Certificate | *google_x509.Certificate](certificate Certificate, oid asn1.ObjectIdentifier) (string, error) { + switch cert := any(certificate).(type) { + case *x509.Certificate: + for _, ext := range cert.Extensions { + if ext.Id.Equal(oid) { + return string(ext.Value), nil + } } + return "", nil + case *google_x509.Certificate: + for _, ext := range cert.Extensions { + if ext.Id.Equal((google_asn1.ObjectIdentifier)(oid)) { + return string(ext.Value), nil + } + } + return "", nil } - return "", nil + return "", errors.New("certificate was neither x509 nor google_x509") } // OIDMatchesPolicy returns if a certificate contains both a given OID field and a matching value associated with that field // if true, it returns the OID extension and extension value that were matched on -func OIDMatchesPolicy(cert *x509.Certificate, oid asn1.ObjectIdentifier, extensionValues []string) (bool, asn1.ObjectIdentifier, string, error) { - extValue, err := getExtension(cert, oid) - if err != nil { - return false, nil, "", fmt.Errorf("error getting extension value: %w", err) - } - if extValue == "" { +func OIDMatchesPolicy[Certificate *x509.Certificate | *google_x509.Certificate](certificate Certificate, oid asn1.ObjectIdentifier, extensionValues []string) (bool, asn1.ObjectIdentifier, string, error) { + switch cert := any(certificate).(type) { + case *x509.Certificate: + extValue, err := getExtension(cert, oid) + if err != nil { + return false, nil, "", fmt.Errorf("error getting extension value: %w", err) + } + if extValue == "" { + return false, nil, "", nil + } + for _, extensionValue := range extensionValues { + if extValue == extensionValue { + return true, oid, extValue, nil + } + } return false, nil, "", nil - } - - for _, extensionValue := range extensionValues { - if extValue == extensionValue { - return true, oid, extValue, nil + case *google_x509.Certificate: + extValue, err := getExtension(cert, oid) + if err != nil { + return false, nil, "", fmt.Errorf("error getting extension value: %w", err) + } + if extValue == "" { + return false, nil, "", nil + } + for _, extensionValue := range extensionValues { + if extValue == extensionValue { + return true, oid, extValue, nil + } } + return false, nil, "", nil } - - return false, nil, "", nil + return false, nil, "", errors.New("certificate was neither x509 nor google_x509") } // CertMatchesPolicy returns true if a certificate contains a given subject and optionally a given issuer diff --git a/pkg/identity/identity_test.go b/pkg/identity/identity_test.go index 33e8630e..5dc961d3 100644 --- a/pkg/identity/identity_test.go +++ b/pkg/identity/identity_test.go @@ -23,6 +23,9 @@ import ( "strings" "testing" + google_asn1 "github.com/google/certificate-transparency-go/asn1" + google_x509 "github.com/google/certificate-transparency-go/x509" + google_pkix "github.com/google/certificate-transparency-go/x509/pkix" "github.com/sigstore/rekor-monitor/pkg/fulcio/extensions" ) @@ -382,6 +385,35 @@ func TestOIDMatchesValue(t *testing.T) { } } +// Test when OID is present and matches value +func TestGoogleOIDMatchesValue(t *testing.T) { + oid := asn1.ObjectIdentifier{2, 5, 29, 17} + extValueString := "test cert value" + extensionValues := []string{extValueString} + marshalledExtValue, err := google_asn1.Marshal(extValueString) + if err != nil { + t.Errorf("error marshalling extension value: %v", err) + } + cert := &google_x509.Certificate{ + Extensions: []google_pkix.Extension{ + { + Id: google_asn1.ObjectIdentifier{2, 5, 29, 17}, + Value: marshalledExtValue, + }, + }, + } + matches, matchedOID, extValue, err := OIDMatchesPolicy(cert, oid, extensionValues) + if !matches || err != nil { + t.Errorf("Expected true, got %v, error %v", matches, err) + } + if matchedOID.String() != oid.String() { + t.Errorf("Expected oid to equal 2.5.29.17, got %s", matchedOID.String()) + } + if extValue != extValueString { + t.Errorf("Expected string to equal 'test cert value', got %s", extValue) + } +} + // Test when cert is present but the value does not match func TestCertDoesNotMatch(t *testing.T) { emailAddr := "test@address.com"