Skip to content
This repository was archived by the owner on Sep 23, 2025. It is now read-only.

Commit ee576e2

Browse files
codysoylandcodysoyland
committed
Improve error handling
Signed-off-by: Cody Soyland <codysoyland@github.com>
1 parent 6e683d9 commit ee576e2

6 files changed

Lines changed: 72 additions & 29 deletions

File tree

‎cmd/tester/main.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@ func main() {
154154
log.Fatal(err)
155155
}
156156

157-
c := config.ConvertSigstoreKeys(context.Background(), tr.Spec.SigstoreKeys)
157+
c, err := config.ConvertSigstoreKeys(context.Background(), tr.Spec.SigstoreKeys)
158+
if err != nil {
159+
log.Fatal(err)
160+
}
158161
maps := make(map[string]*config.SigstoreKeys, 0)
159162

160163
maps[tr.Name] = c

‎hack/gentestdata/gentestdata.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,10 @@ func genCertChain(keyUsage x509.KeyUsage) [][]byte {
188188

189189
func genTrustRoot(sigstoreKeysMap map[string]string) (marshalledEntry []byte, err error) {
190190
trustRoot := testing.NewTrustRoot("test-trustroot", testing.WithSigstoreKeys(sigstoreKeysMap))
191-
sigstoreKeys := config.ConvertSigstoreKeys(context.Background(), trustRoot.Spec.SigstoreKeys)
191+
sigstoreKeys, err := config.ConvertSigstoreKeys(context.Background(), trustRoot.Spec.SigstoreKeys)
192+
if err != nil {
193+
return nil, err
194+
}
192195
err = populateLogIDs(sigstoreKeys)
193196
if err != nil {
194197
return nil, err
@@ -254,10 +257,14 @@ func genTrustedRoot(sigstoreKeysMap map[string]string) ([]byte, error) {
254257
if err != nil {
255258
return nil, err
256259
}
260+
certChain, err := config.DeserializeCertChain([]byte(sigstoreKeysMap["fulcio"]))
261+
if err != nil {
262+
return nil, err
263+
}
257264

258265
trustRoot := &config.SigstoreKeys{
259266
CertificateAuthorities: []*config.CertificateAuthority{{
260-
CertChain: config.DeserializeCertChain([]byte(sigstoreKeysMap["fulcio"])),
267+
CertChain: certChain,
261268
ValidFor: &config.TimeRange{
262269
Start: &config.Timestamp{},
263270
},

‎pkg/apis/config/sigstore_keys.go‎

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -97,58 +97,74 @@ func parseSigstoreKeys(entry string, out *pbtrustroot.TrustedRoot) error {
9797

9898
// ConvertSigstoreKeys takes a source and converts into a SigstoreKeys suitable
9999
// for serialization into a ConfigMap entry.
100-
func ConvertSigstoreKeys(_ context.Context, source *v1alpha1.SigstoreKeys) *SigstoreKeys {
101-
sk := &SigstoreKeys{}
100+
func ConvertSigstoreKeys(_ context.Context, source *v1alpha1.SigstoreKeys) (sk *SigstoreKeys, err error) {
101+
sk = &SigstoreKeys{}
102102
sk.MediaType = "application/vnd.dev.sigstore.trustedroot+json;version=0.1"
103103
sk.CertificateAuthorities = make([]*pbtrustroot.CertificateAuthority, len(source.CertificateAuthorities))
104104
for i := range source.CertificateAuthorities {
105-
sk.CertificateAuthorities[i] = ConvertCertificateAuthority(source.CertificateAuthorities[i])
105+
sk.CertificateAuthorities[i], err = ConvertCertificateAuthority(source.CertificateAuthorities[i])
106+
if err != nil {
107+
return nil, fmt.Errorf("failed to convert certificate authority: %w", err)
108+
}
106109
}
107110

108111
sk.Tlogs = make([]*pbtrustroot.TransparencyLogInstance, len(source.TLogs))
109112
for i := range source.TLogs {
110-
sk.Tlogs[i] = ConvertTransparencyLogInstance(source.TLogs[i])
113+
sk.Tlogs[i], err = ConvertTransparencyLogInstance(source.TLogs[i])
114+
if err != nil {
115+
return nil, fmt.Errorf("failed to convert transparency log instance: %w", err)
116+
}
111117
}
112118

113119
sk.Ctlogs = make([]*pbtrustroot.TransparencyLogInstance, len(source.CTLogs))
114120
for i := range source.CTLogs {
115-
sk.Ctlogs[i] = ConvertTransparencyLogInstance(source.CTLogs[i])
121+
sk.Ctlogs[i], err = ConvertTransparencyLogInstance(source.CTLogs[i])
122+
if err != nil {
123+
return nil, fmt.Errorf("failed to convert ct log instance: %w", err)
124+
}
116125
}
117126

118127
sk.TimestampAuthorities = make([]*pbtrustroot.CertificateAuthority, len(source.TimeStampAuthorities))
119128
for i := range source.TimeStampAuthorities {
120-
sk.TimestampAuthorities[i] = ConvertCertificateAuthority(source.TimeStampAuthorities[i])
129+
sk.TimestampAuthorities[i], err = ConvertCertificateAuthority(source.TimeStampAuthorities[i])
130+
if err != nil {
131+
return nil, fmt.Errorf("failed to convert timestamp authority: %w", err)
132+
}
121133
}
122-
return sk
134+
return sk, nil
123135
}
124136

125137
// ConvertCertificateAuthority converts public into private CertificateAuthority
126-
func ConvertCertificateAuthority(source v1alpha1.CertificateAuthority) *pbtrustroot.CertificateAuthority {
138+
func ConvertCertificateAuthority(source v1alpha1.CertificateAuthority) (*pbtrustroot.CertificateAuthority, error) {
139+
certChain, err := DeserializeCertChain(source.CertChain)
140+
if err != nil {
141+
return nil, err
142+
}
127143
return &pbtrustroot.CertificateAuthority{
128144
Subject: &pbcommon.DistinguishedName{
129145
Organization: source.Subject.Organization,
130146
CommonName: source.Subject.CommonName,
131147
},
132148
Uri: source.URI.String(),
133-
CertChain: DeserializeCertChain(source.CertChain),
149+
CertChain: certChain,
134150
ValidFor: &pbcommon.TimeRange{
135151
Start: &timestamppb.Timestamp{
136152
Seconds: 0, // TODO: Add support for time range to v1alpha1.CertificateAuthority
137153
},
138154
},
139-
}
155+
}, nil
140156
}
141157

142158
// ConvertTransparencyLogInstance converts public into private
143159
// TransparencyLogInstance.
144-
func ConvertTransparencyLogInstance(source v1alpha1.TransparencyLogInstance) *pbtrustroot.TransparencyLogInstance {
160+
func ConvertTransparencyLogInstance(source v1alpha1.TransparencyLogInstance) (*pbtrustroot.TransparencyLogInstance, error) {
145161
pbpk, pk, err := DeserializePublicKey(source.PublicKey)
146162
if err != nil {
147-
return nil // TODO: log error? Add return error?
163+
return nil, err
148164
}
149165
logID, err := cosign.GetTransparencyLogID(pk)
150166
if err != nil {
151-
return nil // TODO: log error? Add return error?
167+
return nil, err
152168
}
153169

154170
return &pbtrustroot.TransparencyLogInstance{
@@ -158,7 +174,7 @@ func ConvertTransparencyLogInstance(source v1alpha1.TransparencyLogInstance) *pb
158174
LogId: &pbcommon.LogId{
159175
KeyId: []byte(logID),
160176
},
161-
}
177+
}, nil
162178
}
163179

164180
func HashStringToHashAlgorithm(hash string) pbcommon.HashAlgorithm {
@@ -195,17 +211,17 @@ func SerializePublicKey(publicKey *pbcommon.PublicKey) []byte {
195211
return pem.EncodeToMemory(block)
196212
}
197213

198-
func DeserializeCertChain(chain []byte) *pbcommon.X509CertificateChain {
214+
func DeserializeCertChain(chain []byte) (*pbcommon.X509CertificateChain, error) {
199215
var certs []*pbcommon.X509Certificate
200-
for {
201-
var block *pem.Block
216+
var block *pem.Block
217+
for len(chain) > 0 {
202218
block, chain = pem.Decode(chain)
203219
if block == nil {
204-
break
220+
return nil, fmt.Errorf("failed to decode certificate chain PEM")
205221
}
206222
certs = append(certs, &pbcommon.X509Certificate{RawBytes: block.Bytes})
207223
}
208-
return &pbcommon.X509CertificateChain{Certificates: certs}
224+
return &pbcommon.X509CertificateChain{Certificates: certs}, nil
209225
}
210226

211227
func DeserializePublicKey(publicKey []byte) (*pbcommon.PublicKey, crypto.PublicKey, error) {

‎pkg/reconciler/trustroot/trustroot.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, trustroot *v1alpha1.Trus
6868
case trustroot.Spec.Remote != nil:
6969
sigstoreKeys, err = r.getSigstoreKeysFromRemote(ctx, trustroot.Spec.Remote)
7070
case trustroot.Spec.SigstoreKeys != nil:
71-
sigstoreKeys = config.ConvertSigstoreKeys(ctx, trustroot.Spec.SigstoreKeys)
71+
sigstoreKeys, err = config.ConvertSigstoreKeys(ctx, trustroot.Spec.SigstoreKeys)
7272
default:
7373
// This should not happen since the CRD has been validated.
7474
err = fmt.Errorf("invalid TrustRoot entry: %s missing repository,remote, and sigstoreKeys", trustroot.Name)
@@ -272,7 +272,10 @@ func getSigstoreKeysFromTuf(ctx context.Context, tufClient *client.Client) (*con
272272

273273
switch scm.Sigstore.Usage {
274274
case sigstoretuf.Fulcio:
275-
certChain := config.DeserializeCertChain(dl.Bytes())
275+
certChain, err := config.DeserializeCertChain(dl.Bytes())
276+
if err != nil {
277+
return nil, fmt.Errorf("deserializing certificate chain: %w", err)
278+
}
276279
ret.CertificateAuthorities = append(ret.CertificateAuthorities,
277280
&config.CertificateAuthority{
278281
Uri: scm.Sigstore.URI,

‎pkg/reconciler/trustroot/trustroot_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,10 @@ func makeConfigMapWithSigstoreKeys() *corev1.ConfigMap {
412412
Data: make(map[string]string),
413413
}
414414
source := NewTrustRoot(trName, WithSigstoreKeys(sigstoreKeys))
415-
c := config.ConvertSigstoreKeys(context.Background(), source.Spec.SigstoreKeys)
415+
c, err := config.ConvertSigstoreKeys(context.Background(), source.Spec.SigstoreKeys)
416+
if err != nil {
417+
panic("failed to convert test SigstoreKeys")
418+
}
416419
for i := range c.Tlogs {
417420
c.Tlogs[i].LogId = &config.LogID{KeyId: []byte(rekorLogID)}
418421
}
@@ -665,7 +668,10 @@ func TestConvertSigstoreKeys(t *testing.T) {
665668
// to make sure we exercise the path from:
666669
// v1alpha1 => config => configMap => back (this is what reconciler will
667670
// use to call cosign verification functions with).
668-
converted := config.ConvertSigstoreKeys(context.Background(), &source)
671+
converted, err := config.ConvertSigstoreKeys(context.Background(), &source)
672+
if err != nil {
673+
t.Fatalf("Failed to convert entry: %v", err)
674+
}
669675
marshalled, err := resources.Marshal(converted)
670676
if err != nil {
671677
t.Fatalf("Failed to marshal entry: %v", err)

‎pkg/webhook/validator_test.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2954,13 +2954,17 @@ func TestFulcioCertsFromAuthority(t *testing.T) {
29542954
if err != nil {
29552955
t.Fatalf("Failed to deserialize CTLog public key: %v", err)
29562956
}
2957+
certChain, err := config.DeserializeCertChain([]byte(certChain))
2958+
if err != nil {
2959+
t.Fatalf("Failed to deserialize cert chain: %v", err)
2960+
}
29572961
sk := config.SigstoreKeys{
29582962
CertificateAuthorities: []*config.CertificateAuthority{{
29592963
Subject: &config.DistinguishedName{
29602964
Organization: "testorg",
29612965
CommonName: "testcommonname",
29622966
},
2963-
CertChain: config.DeserializeCertChain([]byte(certChain)),
2967+
CertChain: certChain,
29642968
}},
29652969
Ctlogs: []*config.TransparencyLogInstance{{
29662970
LogId: &config.LogID{KeyId: []byte(ctfeLogID)},
@@ -3218,13 +3222,17 @@ func TestCheckOptsFromAuthority(t *testing.T) {
32183222
BaseUrl: "rekor.example.com",
32193223
}},
32203224
}
3225+
certChainPB, err := config.DeserializeCertChain([]byte(certChain))
3226+
if err != nil {
3227+
t.Fatalf("Failed to unmarshal cert chain for testing: %v", err)
3228+
}
32213229
skFulcio := config.SigstoreKeys{
32223230
CertificateAuthorities: []*config.CertificateAuthority{{
32233231
Subject: &config.DistinguishedName{
32243232
Organization: "testorg",
32253233
CommonName: "testcommonname",
32263234
},
3227-
CertChain: config.DeserializeCertChain([]byte(certChain)),
3235+
CertChain: certChainPB,
32283236
}},
32293237
Ctlogs: []*config.TransparencyLogInstance{{
32303238
LogId: &config.LogID{KeyId: []byte(ctfeLogID)},
@@ -3242,7 +3250,7 @@ func TestCheckOptsFromAuthority(t *testing.T) {
32423250
Organization: "testorg",
32433251
CommonName: "testcommonname",
32443252
},
3245-
CertChain: config.DeserializeCertChain([]byte(certChain)),
3253+
CertChain: certChainPB,
32463254
}},
32473255
Ctlogs: []*config.TransparencyLogInstance{{
32483256
LogId: &config.LogID{KeyId: []byte(ctfeLogID)},

0 commit comments

Comments
 (0)