From f3900f774022530377d8790409c2328c73320585 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 15 Apr 2024 15:39:49 -0700 Subject: [PATCH] Ignore non certificates in ReadCertificate and ReadCertificateBundle (#483) * Ignore non-certificates in ReadCertificate and ReadCertificateBundle --- pemutil/pem.go | 38 ++++++++++-------------- pemutil/pem_test.go | 12 ++++---- pemutil/testdata/extrajunkbundle.crt | 44 ++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 pemutil/testdata/extrajunkbundle.crt diff --git a/pemutil/pem.go b/pemutil/pem.go index c1d28931..7d1d3fc5 100644 --- a/pemutil/pem.go +++ b/pemutil/pem.go @@ -280,29 +280,23 @@ func ParseCertificateRequest(pemData []byte) (*x509.CertificateRequest, error) { // ReadCertificate returns a *x509.Certificate from the given filename. It // supports certificates formats PEM and DER. func ReadCertificate(filename string, opts ...Options) (*x509.Certificate, error) { - b, err := utils.ReadFile(filename) - if err != nil { + // Populate options + ctx := newContext(filename) + if err := ctx.apply(opts); err != nil { return nil, err } - // PEM format - if bytes.Contains(b, PEMBlockHeader) { - var crt interface{} - crt, err = Read(filename, opts...) - if err != nil { - return nil, err - } - switch crt := crt.(type) { - case *x509.Certificate: - return crt, nil - default: - return nil, errors.Errorf("error decoding PEM: file '%s' does not contain a certificate", filename) - } + bundle, err := ReadCertificateBundle(filename) + switch { + case err != nil: + return nil, err + case len(bundle) == 0: + return nil, errors.Errorf("file %s does not contain a valid PEM or DER formatted certificate", filename) + case len(bundle) > 1 && !ctx.firstBlock: + return nil, errors.Errorf("error decoding %s: contains more than one PEM encoded block", filename) + default: + return bundle[0], nil } - - // DER format (binary) - crt, err := x509.ParseCertificate(b) - return crt, errors.Wrapf(err, "error parsing %s", filename) } // ReadCertificateBundle returns a list of *x509.Certificate from the given @@ -324,7 +318,7 @@ func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) { break } if block.Type != "CERTIFICATE" { - return nil, errors.Errorf("error decoding PEM: file '%s' is not a certificate bundle", filename) + continue } var crt *x509.Certificate crt, err = x509.ParseCertificate(block.Bytes) @@ -333,8 +327,8 @@ func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) { } bundle = append(bundle, crt) } - if len(b) > 0 { - return nil, errors.Errorf("error decoding PEM: file '%s' contains unexpected data", filename) + if len(bundle) == 0 { + return nil, errors.Errorf("file %s does not contain a valid PEM formatted certificate", filename) } return bundle, nil } diff --git a/pemutil/pem_test.go b/pemutil/pem_test.go index 73c6b557..666af40b 100644 --- a/pemutil/pem_test.go +++ b/pemutil/pem_test.go @@ -339,14 +339,15 @@ func TestReadCertificate(t *testing.T) { }{ {"testdata/ca.crt", nil, nil}, {"testdata/nonPEMHeaderCa.crt", nil, nil}, + {"testdata/extrajunkbundle.crt", []Options{WithFirstBlock()}, nil}, {"testdata/ca.der", nil, nil}, {"testdata/bundle.crt", []Options{WithFirstBlock()}, nil}, {"testdata/bundle.crt", nil, errors.New("error decoding testdata/bundle.crt: contains more than one PEM encoded block")}, {"testdata/notexists.crt", nil, errors.New("error reading testdata/notexists.crt: no such file or directory")}, {"testdata/badca.crt", nil, errors.New("error parsing testdata/badca.crt")}, - {"testdata/badpem.crt", nil, errors.New("error decoding testdata/badpem.crt: not a valid PEM encoded block")}, + {"testdata/badpem.crt", nil, errors.New("file testdata/badpem.crt does not contain a valid PEM formatted certificate")}, {"testdata/badder.crt", nil, errors.New("error parsing testdata/badder.crt")}, - {"testdata/openssl.p256.pem", nil, errors.New("error decoding PEM: file 'testdata/openssl.p256.pem' does not contain a certificate")}, + {"testdata/openssl.p256.pem", nil, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM formatted certificate")}, } for _, tc := range tests { @@ -374,17 +375,18 @@ func TestReadCertificateBundle(t *testing.T) { {"testdata/nonPEMHeaderCa.crt", 1, nil}, {"testdata/ca.der", 1, nil}, {"testdata/bundle.crt", 2, nil}, + {"testdata/extrajunkbundle.crt", 2, nil}, {"testdata/notexists.crt", 0, errors.New("error reading testdata/notexists.crt: no such file or directory")}, {"testdata/badca.crt", 0, errors.New("error parsing testdata/badca.crt")}, - {"testdata/badpem.crt", 0, errors.New("error decoding PEM: file 'testdata/badpem.crt' contains unexpected data")}, + {"testdata/badpem.crt", 0, errors.New("file testdata/badpem.crt does not contain a valid PEM formatted certificate")}, {"testdata/badder.crt", 0, errors.New("error parsing testdata/badder.crt")}, - {"testdata/openssl.p256.pem", 0, errors.New("error decoding PEM: file 'testdata/openssl.p256.pem' is not a certificate bundle")}, + {"testdata/openssl.p256.pem", 0, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM formatted certificate")}, } for _, tc := range tests { certs, err := ReadCertificateBundle(tc.fn) if tc.err != nil { - if assert.Error(t, err, tc.fn) { + if assert.Error(t, err, tc.fn, "- expected error but got nil") { assert.HasPrefix(t, err.Error(), tc.err.Error()) } } else { diff --git a/pemutil/testdata/extrajunkbundle.crt b/pemutil/testdata/extrajunkbundle.crt new file mode 100644 index 00000000..59de2e5d --- /dev/null +++ b/pemutil/testdata/extrajunkbundle.crt @@ -0,0 +1,44 @@ + +This is junk +-----BEGIN CERTIFICATE----- +MIICYzCCAgmgAwIBAgIQXS+eYTIGURSsxpqk0JL1sjAKBggqhkjOPQQDAjBKMRsw +GQYDVQQKExJFbnRyeVBvaW50TmV0d29ya3MxKzApBgNVBAMTIkVudHJ5UG9pbnRO +ZXR3b3JrcyBJbnRlcm1lZGlhdGUgQ0EwHhcNMjQwNDEyMDQxODMwWhcNMjQwNDEz +MDQxOTMwWjBBMRAwDgYDVQQGEwdFc3RvbmlhMQwwCgYDVQQDEwNmb28xHzAdBgkq +hkiG9w0BCQEWEGphbmVAZXhhbXBsZS5jb20wWTATBgcqhkjOPQIBBggqhkjOPQMB +BwNCAAQfwDdJQSLvCSmxIURwmrhH7cypActF5D69Eqks4ifDrI6JlMt1UEGEYjMA +6QxhbwmoYOTlT2FKwuOno4b6HNgxo4HZMIHWMB8GA1UdIwQYMBaAFLA+ZPEOXQ60 +VW4C9z0yADaawPTxMA4GA1UdDwEB/wQEAwIHgDAdBgNVHSUEFjAUBggrBgEFBQcD +AQYIKwYBBQUHAwIwHQYDVR0OBBYEFPCfF90MSKLBT9td1DYvywcKqn1hMA4GA1Ud +EQQHMAWCA2ZvbzBVBgwrBgEEAYKkZMYoQAEERTBDAgEBBBFtYXhAc21hbGxzdGVw +LmNvbQQrM2lxcWtabWhTQmxLZ0RuU25SN3NUQndWVHlvdUU3NzRJam5KTy05VGd0 +RTAKBggqhkjOPQQDAgNIADBFAiAXk5RA2ZkeSBSQv/ECZIhVDJ/55DllYzD3KaLL +pMZNoQIhAMEnUi5YcKjhmwFHgwOfWQGO2n+K+bOzmlfX3rJ0dnxQ +-----END CERTIFICATE----- + +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIN6JaoHeDwxnp7pAh27vTdtjxpWYCzm005DcU3a/bKOIoAoGCCqGSM49 +AwEHoUQDQgAEH8A3SUEi7wkpsSFEcJq4R+3MqQHLReQ+vRKpLOInw6yOiZTLdVBB +hGIzAOkMYW8JqGDk5U9hSsLjp6OG+hzYMQ== +-----END EC PRIVATE KEY----- + +This is more junk +-----BEGIN CERTIFICATE----- +MIIB7zCCAZagAwIBAgIQbo4vKv5fIjI6G3q/yg4m/jAKBggqhkjOPQQDAjBCMRsw +GQYDVQQKExJFbnRyeVBvaW50TmV0d29ya3MxIzAhBgNVBAMTGkVudHJ5UG9pbnRO +ZXR3b3JrcyBSb290IENBMB4XDTIyMDIxNTE3MDYwMVoXDTMyMDIxMzE3MDYwMVow +SjEbMBkGA1UEChMSRW50cnlQb2ludE5ldHdvcmtzMSswKQYDVQQDEyJFbnRyeVBv +aW50TmV0d29ya3MgSW50ZXJtZWRpYXRlIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0D +AQcDQgAEiRaPNJDxVKKe8yapu4d35KhPRpvsfwE+OqmJg4w027fILyUmoNE4DCqj +dnSl7RUks34P3eU4/F+A3PEEkt5J66NmMGQwDgYDVR0PAQH/BAQDAgEGMBIGA1Ud +EwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFLA+ZPEOXQ60VW4C9z0yADaawPTxMB8G +A1UdIwQYMBaAFMi91HWiK5mnfzS7MkfI1cMrTUe/MAoGCCqGSM49BAMCA0cAMEQC +ICfmZ03AxBJQ93DzcAgROH2T/M8eWBEKJQzLM7lNVMlpAiB4xDPbXPaRzGpCzXSq +HIn8lljM3V0hfX4DjVf4B1Mayg== +-----END CERTIFICATE----- +/// +-----BEGIN EC PARAMETERS----- +BgUrgQQACg== +-----END EC PARAMETERS----- +So much junk in this file +