From ab939cc1765634ba5ff77ef6c1315c3a95bd9d45 Mon Sep 17 00:00:00 2001 From: Slavek Kabrda Date: Wed, 31 Jul 2024 13:01:38 +0200 Subject: [PATCH] Address review, fix linting issues Signed-off-by: Slavek Kabrda --- pkg/root/trusted_root.go | 5 +- pkg/root/trusted_root_create.go | 71 ++++++++++++++++++++-------- pkg/root/trusted_root_create_test.go | 14 ++---- pkg/root/trusted_root_test.go | 14 ++++++ 4 files changed, 73 insertions(+), 31 deletions(-) diff --git a/pkg/root/trusted_root.go b/pkg/root/trusted_root.go index 0b4083ec..cba47e20 100644 --- a/pkg/root/trusted_root.go +++ b/pkg/root/trusted_root.go @@ -51,6 +51,7 @@ type CertificateAuthority struct { Leaf *x509.Certificate ValidityPeriodStart time.Time ValidityPeriodEnd time.Time + URI string } type TransparencyLog struct { @@ -249,7 +250,9 @@ func ParseCertificateAuthority(certAuthority *prototrustroot.CertificateAuthorit return nil, fmt.Errorf("CertificateAuthority cert chain is empty") } - certificateAuthority = &CertificateAuthority{} + certificateAuthority = &CertificateAuthority{ + URI: certAuthority.Uri, + } for i, cert := range certChain.GetCertificates() { parsedCert, err := x509.ParseCertificate(cert.RawBytes) if err != nil { diff --git a/pkg/root/trusted_root_create.go b/pkg/root/trusted_root_create.go index 3d1946fc..aa67b6de 100644 --- a/pkg/root/trusted_root_create.go +++ b/pkg/root/trusted_root_create.go @@ -26,6 +26,7 @@ import ( "encoding/pem" "errors" "fmt" + "sort" "time" protocommon "github.com/sigstore/protobuf-specs/gen/pb-go/common/v1" @@ -87,17 +88,17 @@ func NewTrustedRootFromTargets(mediaType string, targets []RawTrustedRootTarget) case TSATarget: tsaCertChains = append(tsaCertChains, target.GetBytes()) case RekorTarget: - tlInstance, keyId, err := pubkeyToTransparencyLogInstance(target.GetBytes(), now) + tlInstance, keyID, err := pubkeyToTransparencyLogInstance(target.GetBytes(), now) if err != nil { return nil, fmt.Errorf("failed to parse rekor key: %w", err) } - tr.rekorLogs[keyId] = tlInstance + tr.rekorLogs[keyID] = tlInstance case CTFETarget: - tlInstance, keyId, err := pubkeyToTransparencyLogInstance(target.GetBytes(), now) + tlInstance, keyID, err := pubkeyToTransparencyLogInstance(target.GetBytes(), now) if err != nil { return nil, fmt.Errorf("failed to parse ctlog key: %w", err) } - tr.ctLogs[keyId] = tlInstance + tr.ctLogs[keyID] = tlInstance } } @@ -121,7 +122,7 @@ func NewTrustedRootFromTargets(mediaType string, targets []RawTrustedRootTarget) } func pubkeyToTransparencyLogInstance(keyBytes []byte, tm time.Time) (*TransparencyLog, string, error) { - logId := sha256.Sum256(keyBytes) + logID := sha256.Sum256(keyBytes) der, _ := pem.Decode(keyBytes) key, keyDetails, err := getKeyWithDetails(der.Bytes) if err != nil { @@ -130,12 +131,12 @@ func pubkeyToTransparencyLogInstance(keyBytes []byte, tm time.Time) (*Transparen return &TransparencyLog{ BaseURL: "", - ID: logId[:], + ID: logID[:], ValidityPeriodStart: tm, HashFunc: crypto.SHA256, // we can't get this from the keyBytes, assume SHA256 PublicKey: key, SignatureHashFunc: keyDetails, - }, hex.EncodeToString(logId[:]), nil + }, hex.EncodeToString(logID[:]), nil } func getKeyWithDetails(key []byte) (crypto.PublicKey, crypto.Hash, error) { @@ -213,9 +214,8 @@ func certsToAuthority(certChainPem []byte) (*CertificateAuthority, error) { } } - // TODO: should this rather be the innermost cert? - ca.ValidityPeriodStart = ca.Root.NotBefore - ca.ValidityPeriodEnd = ca.Root.NotAfter + ca.ValidityPeriodStart = certChain[0].NotBefore + ca.ValidityPeriodEnd = certChain[0].NotAfter return &ca, nil } @@ -224,20 +224,46 @@ func (tr *TrustedRoot) constructProtoTrustRoot() error { tr.trustedRoot = &prototrustroot.TrustedRoot{} tr.trustedRoot.MediaType = TrustedRootMediaType01 - for logId, transparencyLog := range tr.rekorLogs { + for logID, transparencyLog := range tr.rekorLogs { tlProto, err := transparencyLogToProtobufTL(*transparencyLog) if err != nil { - return fmt.Errorf("failed converting rekor log %s to protobuf: %w", logId, err) + return fmt.Errorf("failed converting rekor log %s to protobuf: %w", logID, err) } tr.trustedRoot.Tlogs = append(tr.trustedRoot.Tlogs, tlProto) + // ensure stable sorting of the slice + sort.Slice(tr.trustedRoot.Tlogs, func(i, j int) bool { + iTime := time.Unix(0, 0) + jTime := time.Unix(0, 0) + + if tr.trustedRoot.Tlogs[i].PublicKey.ValidFor.Start != nil { + iTime = tr.trustedRoot.Tlogs[i].PublicKey.ValidFor.Start.AsTime() + } + if tr.trustedRoot.Tlogs[j].PublicKey.ValidFor.Start != nil { + iTime = tr.trustedRoot.Tlogs[j].PublicKey.ValidFor.Start.AsTime() + } + return iTime.Before(jTime) + }) } - for logId, ctLog := range tr.ctLogs { + for logID, ctLog := range tr.ctLogs { ctProto, err := transparencyLogToProtobufTL(*ctLog) if err != nil { - return fmt.Errorf("failed converting ctlog %s to protobuf: %w", logId, err) + return fmt.Errorf("failed converting ctlog %s to protobuf: %w", logID, err) } tr.trustedRoot.Ctlogs = append(tr.trustedRoot.Ctlogs, ctProto) + // ensure stable sorting of the slice + sort.Slice(tr.trustedRoot.Ctlogs, func(i, j int) bool { + iTime := time.Unix(0, 0) + jTime := time.Unix(0, 0) + + if tr.trustedRoot.Ctlogs[i].PublicKey.ValidFor.Start != nil { + iTime = tr.trustedRoot.Ctlogs[i].PublicKey.ValidFor.Start.AsTime() + } + if tr.trustedRoot.Ctlogs[j].PublicKey.ValidFor.Start != nil { + iTime = tr.trustedRoot.Ctlogs[j].PublicKey.ValidFor.Start.AsTime() + } + return iTime.Before(jTime) + }) } for _, ca := range tr.fulcioCertAuthorities { @@ -277,20 +303,23 @@ func certificateAuthorityToProtobufCA(ca CertificateAuthority) (*prototrustroot. allCerts = append(allCerts, &protocommon.X509Certificate{RawBytes: ca.Root.Raw}) caProto := prototrustroot.CertificateAuthority{ - Uri: "", + Uri: ca.URI, Subject: &protocommon.DistinguishedName{ Organization: org, CommonName: ca.Root.Subject.CommonName, }, ValidFor: &protocommon.TimeRange{ Start: timestamppb.New(ca.ValidityPeriodStart), - End: timestamppb.New(ca.ValidityPeriodEnd), }, CertChain: &protocommon.X509CertificateChain{ Certificates: allCerts, }, } + if !ca.ValidityPeriodEnd.IsZero() { + caProto.ValidFor.End = timestamppb.New(ca.ValidityPeriodEnd) + } + return &caProto, nil } @@ -299,7 +328,7 @@ func transparencyLogToProtobufTL(tl TransparencyLog) (*prototrustroot.Transparen if err != nil { return nil, fmt.Errorf("failed converting hash algorithm to protobuf: %w", err) } - publicKey, err := publicKeyToProtobufPublicKey(tl.PublicKey, tl.ValidityPeriodStart) + publicKey, err := publicKeyToProtobufPublicKey(tl.PublicKey, tl.ValidityPeriodStart, tl.ValidityPeriodEnd) if err != nil { return nil, fmt.Errorf("failed converting public key to protobuf: %w", err) } @@ -332,13 +361,17 @@ func hashAlgorithmToProtobufHashAlgorithm(hashAlgorithm crypto.Hash) (protocommo } } -func publicKeyToProtobufPublicKey(publicKey crypto.PublicKey, tm time.Time) (*protocommon.PublicKey, error) { +func publicKeyToProtobufPublicKey(publicKey crypto.PublicKey, start time.Time, end time.Time) (*protocommon.PublicKey, error) { pkd := protocommon.PublicKey{ ValidFor: &protocommon.TimeRange{ - Start: timestamppb.New(tm), + Start: timestamppb.New(start), }, } + if !end.IsZero() { + pkd.ValidFor.End = timestamppb.New(end) + } + rawBytes, err := x509.MarshalPKIXPublicKey(publicKey) if err != nil { return nil, fmt.Errorf("failed marshalling public key: %w", err) diff --git a/pkg/root/trusted_root_create_test.go b/pkg/root/trusted_root_create_test.go index 24a095ff..e62808a0 100644 --- a/pkg/root/trusted_root_create_test.go +++ b/pkg/root/trusted_root_create_test.go @@ -41,8 +41,6 @@ HOOVHVkwCgYIKoZIzj0EAwIDSAAwRQIhAJkNZmP6sKA+8EebRXFkBa9DPjacBpTc OljJotvKidRhAiAuNrIazKEw2G4dw8x1z6EYk9G+7fJP5m93bjm/JfMBtA== -----END CERTIFICATE-----` - fulcioRootCertBase64DER = `MIICNzCCAd2gAwIBAgITPLBoBQhl1hqFND9S+SGWbfzaRTAKBggqhkjOPQQDAjBoMQswCQYDVQQGEwJVSzESMBAGA1UECBMJV2lsdHNoaXJlMRMwEQYDVQQHEwpDaGlwcGVuaGFtMQ8wDQYDVQQKEwZSZWRIYXQxDDAKBgNVBAsTA0NUTzERMA8GA1UEAxMIdGVzdGNlcnQwHhcNMjEwMzEyMjMyNDQ5WhcNMzEwMjI4MjMyNDQ5WjBoMQswCQYDVQQGEwJVSzESMBAGA1UECBMJV2lsdHNoaXJlMRMwEQYDVQQHEwpDaGlwcGVuaGFtMQ8wDQYDVQQKEwZSZWRIYXQxDDAKBgNVBAsTA0NUTzERMA8GA1UEAxMIdGVzdGNlcnQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQRn+Alyof6xP3GQClSwgV0NFuYYEwmKP/WLWr/LwB6LUYzt5v49RlqG83KuaJSpeOj7G7MVABdpIZYWwqAiZV3o2YwZDAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0TAQH/BAgwBgEB/wIBATAdBgNVHQ4EFgQUT8Jwm6JuVb0dsiuHUROiHOOVHVkwHwYDVR0jBBgwFoAUT8Jwm6JuVb0dsiuHUROiHOOVHVkwCgYIKoZIzj0EAwIDSAAwRQIhAJkNZmP6sKA+8EebRXFkBa9DPjacBpTcOljJotvKidRhAiAuNrIazKEw2G4dw8x1z6EYk9G+7fJP5m93bjm/JfMBtA==` - ctlogPublicKey = `-----BEGIN RSA PUBLIC KEY----- MIICCgKCAgEAu1Ah4n2P8JGt92Qg86FdR8f1pou43yndggMuRCX0JB+bLn1rUFRA KQVd+xnnd4PXJLLdml8ZohCr0lhBuMxZ7zBzt0T98kblUCxBgABPNpWIkTgacyC8 @@ -83,8 +81,6 @@ HvD4ejCZJOWQnqAlkqURllvu9M8+VqLbiRK+zSfZCZwsiljRn8MQQRSkXEE5AjEA g+VxqtojfVfu8DhzzhCx9GKETbJHb19iV72mMKUbDAFmzZ6bQ8b54Zb8tidy5aWe -----END CERTIFICATE-----` - tsaLeafCertBase64DER = `MIIB3DCCAWKgAwIBAgIUchkNsH36Xa04b1LqIc+qr9DVecMwCgYIKoZIzj0EAwMwMjEVMBMGA1UEChMMR2l0SHViLCBJbmMuMRkwFwYDVQQDExBUU0EgaW50ZXJtZWRpYXRlMB4XDTIzMDQxNDAwMDAwMFoXDTI0MDQxMzAwMDAwMFowMjEVMBMGA1UEChMMR2l0SHViLCBJbmMuMRkwFwYDVQQDExBUU0EgVGltZXN0YW1waW5nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEUD5ZNbSqYMd6r8qpOOEX9ibGnZT9GsuXOhr/f8U9FJugBGExKYp40OULS0erjZW7xV9xV52NnJf5OeDq4e5ZKqNWMFQwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMIMAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgwFoAUaW1RudOgVt0leqY0WKYbuPr47wAwCgYIKoZIzj0EAwMDaAAwZQIwbUH9HvD4ejCZJOWQnqAlkqURllvu9M8+VqLbiRK+zSfZCZwsiljRn8MQQRSkXEE5AjEAg+VxqtojfVfu8DhzzhCx9GKETbJHb19iV72mMKUbDAFmzZ6bQ8b54Zb8tidy5aWe` - tsaIntermedCert = `-----BEGIN CERTIFICATE----- MIICEDCCAZWgAwIBAgIUX8ZO5QXP7vN4dMQ5e9sU3nub8OgwCgYIKoZIzj0EAwMw ODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZJbnRlcm5hbCBTZXJ2 @@ -100,8 +96,6 @@ AK1B185ygCrIYFlIs3GjswjnwSMG6LY8woLVdakKDZxVa8f8cqMs1DhcxJ0+09w9 HdNmyA== -----END CERTIFICATE-----` - tsaIntermedCertBase64DER = `MIICEDCCAZWgAwIBAgIUX8ZO5QXP7vN4dMQ5e9sU3nub8OgwCgYIKoZIzj0EAwMwODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZJbnRlcm5hbCBTZXJ2aWNlcyBSb290MB4XDTIzMDQxNDAwMDAwMFoXDTI4MDQxMjAwMDAwMFowMjEVMBMGA1UEChMMR2l0SHViLCBJbmMuMRkwFwYDVQQDExBUU0EgaW50ZXJtZWRpYXRlMHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEvMLY/dTVbvIJYANAuszEwJnQE1llftynyMKIMhh48HmqbVr5ygybzsLRLVKbBWOdZ21aeJz+gZiytZetqcyF9WlER5NEMf6JV7ZNojQpxHq4RHGoGSceQv/qvTiZxEDKo2YwZDAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0TAQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQUaW1RudOgVt0leqY0WKYbuPr47wAwHwYDVR0jBBgwFoAU9NYYlobnAG4c0/qjxyH/lq/wz+QwCgYIKoZIzj0EAwMDaQAwZgIxAK1B185ygCrIYFlIs3GjswjnwSMG6LY8woLVdakKDZxVa8f8cqMs1DhcxJ0+09w95QIxAO+tBzZk7vjUJ9iJgD4R6ZWTxQWKqNm74jO99o+o9sv4FI/SZTZTFyMn0IJEHdNmyA==` - tsaRootCert = `-----BEGIN CERTIFICATE----- MIIB9DCCAXqgAwIBAgIUa/JAkdUjK4JUwsqtaiRJGWhqLSowCgYIKoZIzj0EAwMw ODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZJbnRlcm5hbCBTZXJ2 @@ -115,14 +109,12 @@ z+QwCgYIKoZIzj0EAwMDaAAwZQIxALZLZ8BgRXzKxLMMN9VIlO+e4hrBnNBgF7tz 7Hnrowv2NetZErIACKFymBlvWDvtMAIwZO+ki6ssQ1bsZo98O8mEAf2NZ7iiCgDD U0Vwjeco6zyeh0zBTs9/7gV6AHNQ53xD -----END CERTIFICATE-----` - - tsaRootCertBase64DER = `MIIB9DCCAXqgAwIBAgIUa/JAkdUjK4JUwsqtaiRJGWhqLSowCgYIKoZIzj0EAwMwODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZJbnRlcm5hbCBTZXJ2aWNlcyBSb290MB4XDTIzMDQxNDAwMDAwMFoXDTMzMDQxMTAwMDAwMFowODEVMBMGA1UEChMMR2l0SHViLCBJbmMuMR8wHQYDVQQDExZJbnRlcm5hbCBTZXJ2aWNlcyBSb290MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEf9jFAXxz4kx68AHRMOkFBhflDcMTvzaXz4x/FCcXjJ/1qEKon/qPIGnaURskDtyNbNDOpeJTDDFqt48iMPrnzpx6IZwqemfUJN4xBEZfza+pYt/iyod+9tZr20RRWSv/o0UwQzAOBgNVHQ8BAf8EBAMCAQYwEgYDVR0TAQH/BAgwBgEB/wIBAjAdBgNVHQ4EFgQU9NYYlobnAG4c0/qjxyH/lq/wz+QwCgYIKoZIzj0EAwMDaAAwZQIxALZLZ8BgRXzKxLMMN9VIlO+e4hrBnNBgF7tz7Hnrowv2NetZErIACKFymBlvWDvtMAIwZO+ki6ssQ1bsZo98O8mEAf2NZ7iiCgDDU0Vwjeco6zyeh0zBTs9/7gV6AHNQ53xD` ) func TestConstructTrustedRootJSON(t *testing.T) { var targets []RawTrustedRootTarget - var tsaCertChain []byte = []byte{} + var tsaCertChain = []byte{} tsaCertChain = append(tsaCertChain, []byte(tsaLeafCert)...) tsaCertChain = append(tsaCertChain, byte('\n')) tsaCertChain = append(tsaCertChain, []byte(tsaIntermedCert)...) @@ -210,8 +202,8 @@ func TestConstructTrustedRootJSON(t *testing.T) { Root: parsedTSARootCert, Intermediates: []*x509.Certificate{parsedTSAIntermedCert}, Leaf: parsedTSALeafCert, - ValidityPeriodStart: parsedTSARootCert.NotBefore, - ValidityPeriodEnd: parsedTSARootCert.NotAfter, + ValidityPeriodStart: parsedTSALeafCert.NotBefore, + ValidityPeriodEnd: parsedTSALeafCert.NotAfter, }, }, fromJS.timestampingAuthorities) } diff --git a/pkg/root/trusted_root_test.go b/pkg/root/trusted_root_test.go index dfabe248..6a04f9a5 100644 --- a/pkg/root/trusted_root_test.go +++ b/pkg/root/trusted_root_test.go @@ -21,6 +21,7 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" + "encoding/json" "encoding/pem" "os" "testing" @@ -165,3 +166,16 @@ func TestTrustedMaterialCollectionRSA(t *testing.T) { assert.NoError(t, err) assert.Equal(t, verifier, verifier2) } + +func TestFromJSONToJSON(t *testing.T) { + trustedrootJSON, err := os.ReadFile("../../examples/trusted-root-public-good.json") + assert.NoError(t, err) + + trustedRoot, err := NewTrustedRootFromJSON(trustedrootJSON) + assert.NoError(t, err) + + jsonBytes, err := json.Marshal(trustedRoot) + assert.NoError(t, err) + + assert.JSONEq(t, string(trustedrootJSON), string(jsonBytes)) +}