Skip to content

Commit f3900f7

Browse files
dopeydopey
authored
Ignore non certificates in ReadCertificate and ReadCertificateBundle (#483)
* Ignore non-certificates in ReadCertificate and ReadCertificateBundle
1 parent cb64f07 commit f3900f7

File tree

3 files changed

+67
-27
lines changed

3 files changed

+67
-27
lines changed

‎pemutil/pem.go‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -280,29 +280,23 @@ func ParseCertificateRequest(pemData []byte) (*x509.CertificateRequest, error) {
280280
// ReadCertificate returns a *x509.Certificate from the given filename. It
281281
// supports certificates formats PEM and DER.
282282
func ReadCertificate(filename string, opts ...Options) (*x509.Certificate, error) {
283-
b, err := utils.ReadFile(filename)
284-
if err != nil {
283+
// Populate options
284+
ctx := newContext(filename)
285+
if err := ctx.apply(opts); err != nil {
285286
return nil, err
286287
}
287288

288-
// PEM format
289-
if bytes.Contains(b, PEMBlockHeader) {
290-
var crt interface{}
291-
crt, err = Read(filename, opts...)
292-
if err != nil {
293-
return nil, err
294-
}
295-
switch crt := crt.(type) {
296-
case *x509.Certificate:
297-
return crt, nil
298-
default:
299-
return nil, errors.Errorf("error decoding PEM: file '%s' does not contain a certificate", filename)
300-
}
289+
bundle, err := ReadCertificateBundle(filename)
290+
switch {
291+
case err != nil:
292+
return nil, err
293+
case len(bundle) == 0:
294+
return nil, errors.Errorf("file %s does not contain a valid PEM or DER formatted certificate", filename)
295+
case len(bundle) > 1 && !ctx.firstBlock:
296+
return nil, errors.Errorf("error decoding %s: contains more than one PEM encoded block", filename)
297+
default:
298+
return bundle[0], nil
301299
}
302-
303-
// DER format (binary)
304-
crt, err := x509.ParseCertificate(b)
305-
return crt, errors.Wrapf(err, "error parsing %s", filename)
306300
}
307301

308302
// ReadCertificateBundle returns a list of *x509.Certificate from the given
@@ -324,7 +318,7 @@ func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) {
324318
break
325319
}
326320
if block.Type != "CERTIFICATE" {
327-
return nil, errors.Errorf("error decoding PEM: file '%s' is not a certificate bundle", filename)
321+
continue
328322
}
329323
var crt *x509.Certificate
330324
crt, err = x509.ParseCertificate(block.Bytes)
@@ -333,8 +327,8 @@ func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) {
333327
}
334328
bundle = append(bundle, crt)
335329
}
336-
if len(b) > 0 {
337-
return nil, errors.Errorf("error decoding PEM: file '%s' contains unexpected data", filename)
330+
if len(bundle) == 0 {
331+
return nil, errors.Errorf("file %s does not contain a valid PEM formatted certificate", filename)
338332
}
339333
return bundle, nil
340334
}

‎pemutil/pem_test.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -339,14 +339,15 @@ func TestReadCertificate(t *testing.T) {
339339
}{
340340
{"testdata/ca.crt", nil, nil},
341341
{"testdata/nonPEMHeaderCa.crt", nil, nil},
342+
{"testdata/extrajunkbundle.crt", []Options{WithFirstBlock()}, nil},
342343
{"testdata/ca.der", nil, nil},
343344
{"testdata/bundle.crt", []Options{WithFirstBlock()}, nil},
344345
{"testdata/bundle.crt", nil, errors.New("error decoding testdata/bundle.crt: contains more than one PEM encoded block")},
345346
{"testdata/notexists.crt", nil, errors.New("error reading testdata/notexists.crt: no such file or directory")},
346347
{"testdata/badca.crt", nil, errors.New("error parsing testdata/badca.crt")},
347-
{"testdata/badpem.crt", nil, errors.New("error decoding testdata/badpem.crt: not a valid PEM encoded block")},
348+
{"testdata/badpem.crt", nil, errors.New("file testdata/badpem.crt does not contain a valid PEM formatted certificate")},
348349
{"testdata/badder.crt", nil, errors.New("error parsing testdata/badder.crt")},
349-
{"testdata/openssl.p256.pem", nil, errors.New("error decoding PEM: file 'testdata/openssl.p256.pem' does not contain a certificate")},
350+
{"testdata/openssl.p256.pem", nil, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM formatted certificate")},
350351
}
351352

352353
for _, tc := range tests {
@@ -374,17 +375,18 @@ func TestReadCertificateBundle(t *testing.T) {
374375
{"testdata/nonPEMHeaderCa.crt", 1, nil},
375376
{"testdata/ca.der", 1, nil},
376377
{"testdata/bundle.crt", 2, nil},
378+
{"testdata/extrajunkbundle.crt", 2, nil},
377379
{"testdata/notexists.crt", 0, errors.New("error reading testdata/notexists.crt: no such file or directory")},
378380
{"testdata/badca.crt", 0, errors.New("error parsing testdata/badca.crt")},
379-
{"testdata/badpem.crt", 0, errors.New("error decoding PEM: file 'testdata/badpem.crt' contains unexpected data")},
381+
{"testdata/badpem.crt", 0, errors.New("file testdata/badpem.crt does not contain a valid PEM formatted certificate")},
380382
{"testdata/badder.crt", 0, errors.New("error parsing testdata/badder.crt")},
381-
{"testdata/openssl.p256.pem", 0, errors.New("error decoding PEM: file 'testdata/openssl.p256.pem' is not a certificate bundle")},
383+
{"testdata/openssl.p256.pem", 0, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM formatted certificate")},
382384
}
383385

384386
for _, tc := range tests {
385387
certs, err := ReadCertificateBundle(tc.fn)
386388
if tc.err != nil {
387-
if assert.Error(t, err, tc.fn) {
389+
if assert.Error(t, err, tc.fn, "- expected error but got nil") {
388390
assert.HasPrefix(t, err.Error(), tc.err.Error())
389391
}
390392
} else {

‎pemutil/testdata/extrajunkbundle.crt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
2+
This is junk
3+
-----BEGIN CERTIFICATE-----
4+
MIICYzCCAgmgAwIBAgIQXS+eYTIGURSsxpqk0JL1sjAKBggqhkjOPQQDAjBKMRsw
5+
GQYDVQQKExJFbnRyeVBvaW50TmV0d29ya3MxKzApBgNVBAMTIkVudHJ5UG9pbnRO
6+
ZXR3b3JrcyBJbnRlcm1lZGlhdGUgQ0EwHhcNMjQwNDEyMDQxODMwWhcNMjQwNDEz
7+
MDQxOTMwWjBBMRAwDgYDVQQGEwdFc3RvbmlhMQwwCgYDVQQDEwNmb28xHzAdBgkq
8+
hkiG9w0BCQEWEGphbmVAZXhhbXBsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMB
9+
BwNCAAQfwDdJQSLvCSmxIURwmrhH7cypActF5D69Eqks4ifDrI6JlMt1UEGEYjMA
10+
6QxhbwmoYOTlT2FKwuOno4b6HNgxo4HZMIHWMB8GA1UdIwQYMBaAFLA+ZPEOXQ60
11+
VW4C9z0yADaawPTxMA4GA1UdDwEB/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcD
12+
AQYIKwYBBQUHAwIwHQYDVR0OBBYEFPCfF90MSKLBT9td1DYvywcKqn1hMA4GA1Ud
13+
EQQHMAWCA2ZvbzBVBgwrBgEEAYKkZMYoQAEERTBDAgEBBBFtYXhAc21hbGxzdGVw
14+
LmNvbQQrM2lxcWtabWhTQmxLZ0RuU25SN3NUQndWVHlvdUU3NzRJam5KTy05VGd0
15+
RTAKBggqhkjOPQQDAgNIADBFAiAXk5RA2ZkeSBSQv/ECZIhVDJ/55DllYzD3KaLL
16+
pMZNoQIhAMEnUi5YcKjhmwFHgwOfWQGO2n+K+bOzmlfX3rJ0dnxQ
17+
-----END CERTIFICATE-----
18+
19+
-----BEGIN EC PRIVATE KEY-----
20+
MHcCAQEEIN6JaoHeDwxnp7pAh27vTdtjxpWYCzm005DcU3a/bKOIoAoGCCqGSM49
21+
AwEHoUQDQgAEH8A3SUEi7wkpsSFEcJq4R+3MqQHLReQ+vRKpLOInw6yOiZTLdVBB
22+
hGIzAOkMYW8JqGDk5U9hSsLjp6OG+hzYMQ==
23+
-----END EC PRIVATE KEY-----
24+
25+
This is more junk
26+
-----BEGIN CERTIFICATE-----
27+
MIIB7zCCAZagAwIBAgIQbo4vKv5fIjI6G3q/yg4m/jAKBggqhkjOPQQDAjBCMRsw
28+
GQYDVQQKExJFbnRyeVBvaW50TmV0d29ya3MxIzAhBgNVBAMTGkVudHJ5UG9pbnRO
29+
ZXR3b3JrcyBSb290IENBMB4XDTIyMDIxNTE3MDYwMVoXDTMyMDIxMzE3MDYwMVow
30+
SjEbMBkGA1UEChMSRW50cnlQb2ludE5ldHdvcmtzMSswKQYDVQQDEyJFbnRyeVBv
31+
aW50TmV0d29ya3MgSW50ZXJtZWRpYXRlIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0D
32+
AQcDQgAEiRaPNJDxVKKe8yapu4d35KhPRpvsfwE+OqmJg4w027fILyUmoNE4DCqj
33+
dnSl7RUks34P3eU4/F+A3PEEkt5J66NmMGQwDgYDVR0PAQH/BAQDAgEGMBIGA1Ud
34+
EwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFLA+ZPEOXQ60VW4C9z0yADaawPTxMB8G
35+
A1UdIwQYMBaAFMi91HWiK5mnfzS7MkfI1cMrTUe/MAoGCCqGSM49BAMCA0cAMEQC
36+
ICfmZ03AxBJQ93DzcAgROH2T/M8eWBEKJQzLM7lNVMlpAiB4xDPbXPaRzGpCzXSq
37+
HIn8lljM3V0hfX4DjVf4B1Mayg==
38+
-----END CERTIFICATE-----
39+
///
40+
-----BEGIN EC PARAMETERS-----
41+
BgUrgQQACg==
42+
-----END EC PARAMETERS-----
43+
So much junk in this file
44+

0 commit comments

Comments
 (0)