From 3b999658f078449a8c208d99f3d259d43aa332bd Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 11 Jul 2024 19:35:08 -0700 Subject: [PATCH] Split ReadCertificate functions into parsing files and bytes (#546) * Update ParseCertificateRequest and ParseCertificateBundle - Call ParseXXX functions from ReadXXX functions. --- pemutil/pem.go | 196 ++++++++++++++++++++-------------------- pemutil/pem_test.go | 212 +++++++++++++++++++++++++++----------------- 2 files changed, 228 insertions(+), 180 deletions(-) diff --git a/pemutil/pem.go b/pemutil/pem.go index 2ad4ce70..d40b622a 100644 --- a/pemutil/pem.go +++ b/pemutil/pem.go @@ -231,52 +231,93 @@ func ParseCertificate(pemData []byte) (*x509.Certificate, error) { return nil, errors.New("error parsing certificate: no certificate found") } -// ParseCertificateBundle extracts all the certificates in the given data. -func ParseCertificateBundle(pemData []byte) ([]*x509.Certificate, error) { - var block *pem.Block - var certs []*x509.Certificate - for len(pemData) > 0 { - block, pemData = pem.Decode(pemData) - if block == nil { - return nil, errors.New("error decoding pem block") +// ParseCertificateBundle returns a list of *x509.Certificate parsed from +// the given bytes. +// +// - supports PEM and DER certificate formats +// - If a DER-formatted file is given only one certificate will be returned. +func ParseCertificateBundle(data []byte) ([]*x509.Certificate, error) { + var err error + + // PEM format + if bytes.Contains(data, PEMBlockHeader) { + var block *pem.Block + var bundle []*x509.Certificate + for len(data) > 0 { + block, data = pem.Decode(data) + if block == nil { + break + } + if block.Type != "CERTIFICATE" || len(block.Headers) != 0 { + continue + } + var crt *x509.Certificate + crt, err = x509.ParseCertificate(block.Bytes) + if err != nil { + return nil, &InvalidPEMError{ + Err: err, + Type: PEMTypeCertificate, + } + } + bundle = append(bundle, crt) } - if block.Type != "CERTIFICATE" || len(block.Headers) != 0 { - continue + if len(bundle) == 0 { + return nil, &InvalidPEMError{ + Type: PEMTypeCertificate, + } } + return bundle, nil + } - cert, err := x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, errors.Wrap(err, "error parsing certificate") + // DER format (binary) + crt, err := x509.ParseCertificate(data) + if err != nil { + return nil, &InvalidPEMError{ + Message: fmt.Sprintf("error parsing certificate as DER format: %v", err), + Type: PEMTypeCertificate, } - certs = append(certs, cert) - } - if len(certs) == 0 { - return nil, errors.New("error parsing certificate: no certificate found") } - return certs, nil + return []*x509.Certificate{crt}, nil } -// ParseCertificateRequest extracts the first certificate from the given pem. -func ParseCertificateRequest(pemData []byte) (*x509.CertificateRequest, error) { - var block *pem.Block - for len(pemData) > 0 { - block, pemData = pem.Decode(pemData) - if block == nil { - return nil, errors.New("error decoding pem block") - } - if (block.Type != "CERTIFICATE REQUEST" && block.Type != "NEW CERTIFICATE REQUEST") || - len(block.Headers) != 0 { - continue - } +// ParseCertificateRequest extracts the first *x509.CertificateRequest +// from the given data. +// +// - supports PEM and DER certificate formats +// - If a DER-formatted file is given only one certificate will be returned. +func ParseCertificateRequest(data []byte) (*x509.CertificateRequest, error) { + // PEM format + if bytes.Contains(data, PEMBlockHeader) { + var block *pem.Block + for len(data) > 0 { + block, data = pem.Decode(data) + if block == nil { + break + } + if !strings.HasSuffix(block.Type, "CERTIFICATE REQUEST") { + continue + } + csr, err := x509.ParseCertificateRequest(block.Bytes) + if err != nil { + return nil, &InvalidPEMError{ + Type: PEMTypeCertificateRequest, + Err: err, + } + } - csr, err := x509.ParseCertificateRequest(block.Bytes) - if err != nil { - return nil, errors.Wrap(err, "error parsing certificate request") + return csr, nil } - return csr, nil } - return nil, errors.New("error parsing certificate request: no certificate found") + // DER format (binary) + csr, err := x509.ParseCertificateRequest(data) + if err != nil { + return nil, &InvalidPEMError{ + Message: fmt.Sprintf("error parsing certificate request as DER format: %v", err), + Type: PEMTypeCertificateRequest, + } + } + return csr, nil } // PEMType represents a PEM block type. (e.g., CERTIFICATE, CERTIFICATE REQUEST, etc.) @@ -318,14 +359,10 @@ func (e *InvalidPEMError) Error() string { case e.Err != nil: return fmt.Sprintf("error decoding PEM data: %v", e.Err) default: - var prefix = "input" - if e.File != "" { - prefix = fmt.Sprintf("file %s", e.File) - } if e.Type == PEMTypeUndefined { - return fmt.Sprintf("%s does not contain valid PEM encoded data", prefix) + return "does not contain valid PEM encoded data" } - return fmt.Sprintf("%s does not contain a valid PEM encoded %s", prefix, e.Type) + return fmt.Sprintf("does not contain a valid PEM encoded %s", e.Type) } } @@ -355,83 +392,40 @@ func ReadCertificate(filename string, opts ...Options) (*x509.Certificate, error } } -// ReadCertificateBundle returns a list of *x509.Certificate from the given -// filename. It supports certificates formats PEM and DER. If a DER-formatted -// file is given only one certificate will be returned. +// ReadCertificateBundle reads the given filename and returns a list of +// *x509.Certificate. +// +// - supports PEM and DER certificate formats +// - If a DER-formatted file is given only one certificate will be returned. func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) { b, err := utils.ReadFile(filename) if err != nil { return nil, err } - // PEM format - if bytes.Contains(b, PEMBlockHeader) { - var block *pem.Block - var bundle []*x509.Certificate - for len(b) > 0 { - block, b = pem.Decode(b) - if block == nil { - break - } - if block.Type != "CERTIFICATE" { - continue - } - var crt *x509.Certificate - crt, err = x509.ParseCertificate(block.Bytes) - if err != nil { - return nil, errors.Wrapf(err, "error parsing %s", filename) - } - bundle = append(bundle, crt) - } - if len(bundle) == 0 { - return nil, &InvalidPEMError{File: filename, Type: PEMTypeCertificate} - } - return bundle, nil - } - - // DER format (binary) - crt, err := x509.ParseCertificate(b) + bundle, err := ParseCertificateBundle(b) if err != nil { - return nil, errors.Wrapf(err, "error parsing %s", filename) + return nil, fmt.Errorf("error parsing %s: %w", filename, err) } - return []*x509.Certificate{crt}, nil + return bundle, nil } -// ReadCertificateRequest returns a *x509.CertificateRequest from the given -// filename. It supports certificates formats PEM and DER. +// ReadCertificateRequest reads the given filename and returns a +// *x509.CertificateRequest. +// +// - supports PEM and DER Certificate formats. +// - supports reading from STDIN with filename `-`. func ReadCertificateRequest(filename string) (*x509.CertificateRequest, error) { b, err := utils.ReadFile(filename) if err != nil { return nil, err } - // PEM format - if bytes.Contains(b, PEMBlockHeader) { - var block *pem.Block - for len(b) > 0 { - block, b = pem.Decode(b) - if block == nil { - break - } - if !strings.HasSuffix(block.Type, "CERTIFICATE REQUEST") { - continue - } - csr, err := x509.ParseCertificateRequest(block.Bytes) - if err != nil { - return nil, &InvalidPEMError{ - File: filename, Type: PEMTypeCertificateRequest, - Message: fmt.Sprintf("error parsing %s: CSR PEM block is invalid: %v", filename, err), - Err: err, - } - } - - return csr, nil - } + cr, err := ParseCertificateRequest(b) + if err != nil { + return nil, fmt.Errorf("error parsing %s: %w", filename, err) } - - // DER format (binary) - csr, err := x509.ParseCertificateRequest(b) - return csr, errors.Wrapf(err, "error parsing %s", filename) + return cr, nil } // Parse returns the key or certificate PEM-encoded in the given bytes. diff --git a/pemutil/pem_test.go b/pemutil/pem_test.go index 95f0959d..65dcd9ea 100644 --- a/pemutil/pem_test.go +++ b/pemutil/pem_test.go @@ -266,71 +266,6 @@ func TestParseCertificate(t *testing.T) { } } -func TestParseCertificateBundle(t *testing.T) { - tests := []struct { - fn string - len int - err error - }{ - {"testdata/ca.crt", 1, nil}, - {"testdata/bundle.crt", 2, nil}, - {"testdata/badca.crt", 0, errors.New("error parsing certificate")}, - {"testdata/badpem.crt", 0, errors.New("error decoding pem block")}, - {"testdata/badder.crt", 0, errors.New("error decoding pem block")}, - {"testdata/openssl.p256.pem", 0, errors.New("error parsing certificate: no certificate found")}, - } - - for _, tc := range tests { - t.Run(tc.fn, func(t *testing.T) { - b, err := os.ReadFile(tc.fn) - if err != nil { - t.Fatal(err) - } - crts, err := ParseCertificateBundle(b) - if tc.err != nil { - if assert.Error(t, err) { - assert.HasPrefix(t, err.Error(), tc.err.Error()) - } - } else { - assert.NoError(t, err) - assert.Type(t, []*x509.Certificate{}, crts) - } - assert.Len(t, tc.len, crts) - }) - } -} - -func TestParseCertificateRequest(t *testing.T) { - tests := []struct { - fn string - opts []Options - err error - }{ - {"testdata/test.csr", nil, nil}, - {"testdata/badpem.csr", nil, errors.New("error parsing certificate request")}, - {"testdata/bad.csr", nil, errors.New("error decoding pem block")}, - {"testdata/ca.crt", nil, errors.New("error parsing certificate request: no certificate found")}, - } - - for _, tc := range tests { - t.Run(tc.fn, func(t *testing.T) { - b, err := os.ReadFile(tc.fn) - if err != nil { - t.Fatal(err) - } - csr, err := ParseCertificateRequest(b) - if tc.err != nil { - if assert.Error(t, err) { - assert.HasPrefix(t, err.Error(), tc.err.Error()) - } - } else { - assert.NoError(t, err) - assert.Type(t, &x509.CertificateRequest{}, csr) - } - }) - } -} - func TestReadCertificate(t *testing.T) { tests := []struct { fn string @@ -345,9 +280,9 @@ func TestReadCertificate(t *testing.T) { {"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("file testdata/badpem.crt does not contain a valid PEM encoded certificate")}, + {"testdata/badpem.crt", nil, errors.New("error parsing testdata/badpem.crt: does not contain a valid PEM encoded certificate")}, {"testdata/badder.crt", nil, errors.New("error parsing testdata/badder.crt")}, - {"testdata/openssl.p256.pem", nil, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM encoded certificate")}, + {"testdata/openssl.p256.pem", nil, errors.New("error parsing testdata/openssl.p256.pem: does not contain a valid PEM encoded certificate")}, } for _, tc := range tests { @@ -378,24 +313,67 @@ func TestReadCertificateBundle(t *testing.T) { {"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("file testdata/badpem.crt does not contain a valid PEM encoded certificate")}, + {"testdata/badpem.crt", 0, errors.New("error parsing testdata/badpem.crt: does not contain a valid PEM encoded certificate")}, {"testdata/badder.crt", 0, errors.New("error parsing testdata/badder.crt")}, - {"testdata/openssl.p256.pem", 0, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM encoded certificate")}, + {"testdata/openssl.p256.pem", 0, errors.New("error parsing testdata/openssl.p256.pem: does not contain a valid PEM encoded certificate")}, } for _, tc := range tests { - certs, err := ReadCertificateBundle(tc.fn) - if tc.err != nil { - if assert.Error(t, err, tc.fn, "- expected error but got nil") { - assert.HasPrefix(t, err.Error(), tc.err.Error()) + t.Run(tc.fn, func(t *testing.T) { + certs, err := ReadCertificateBundle(tc.fn) + if tc.err != nil { + if assert.Error(t, err, tc.fn, "- expected error but got nil") { + assert.HasPrefix(t, err.Error(), tc.err.Error()) + } + } else { + assert.NoError(t, err) + assert.Len(t, tc.len, certs, tc.fn) + for i := range certs { + assert.Type(t, &x509.Certificate{}, certs[i]) + } } - } else { - assert.NoError(t, err) - assert.Len(t, tc.len, certs, tc.fn) - for i := range certs { - assert.Type(t, &x509.Certificate{}, certs[i]) + }) + } +} + +func TestParseCertificateBundle(t *testing.T) { + tests := []struct { + name string + fn string + len int + err error + }{ + {"ok cert", "testdata/ca.crt", 1, nil}, + {"ok non PEM header", "testdata/nonPEMHeaderCa.crt", 1, nil}, + {"ok der", "testdata/ca.der", 1, nil}, + {"ok bundle", "testdata/bundle.crt", 2, nil}, + {"ok extra junk in bundle", "testdata/extrajunkbundle.crt", 2, nil}, + {"fail bad cert w/ file", "testdata/badca.crt", 0, errors.New("error decoding PEM data: x509: trailing data")}, + {"fail no PEM", "testdata/badpem.crt", 0, errors.New("does not contain a valid PEM encoded certificate")}, + {"fail no PEM", "testdata/badpem.crt", 0, errors.New("does not contain a valid PEM encoded certificate")}, + {"fail bad der", "testdata/badder.crt", 0, errors.New("error parsing certificate as DER format: x509: malformed certificate")}, + {"fail no valid PEM", "testdata/openssl.p256.pem", 0, errors.New("does not contain a valid PEM encoded certificate")}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + b, err := os.ReadFile(tc.fn) + if err != nil { + t.Fatal(err) } - } + certs, err := ParseCertificateBundle(b) + if tc.err != nil { + if assert.Error(t, err, tc.fn, "- expected error but got nil") { + assert.HasPrefix(t, err.Error(), tc.err.Error()) + } + } else { + assert.NoError(t, err) + assert.Len(t, tc.len, certs, tc.fn) + for i := range certs { + assert.Type(t, &x509.Certificate{}, certs[i]) + } + } + }) } } @@ -1107,6 +1085,82 @@ func TestRead_promptPassword(t *testing.T) { } } +func TestParseCertificateRequest(t *testing.T) { + expected := &x509.CertificateRequest{ + Subject: pkix.Name{ + CommonName: "hello.smallstep.com", + Names: []pkix.AttributeTypeAndValue{{Type: asn1.ObjectIdentifier{2, 5, 4, 3}, Value: "hello.smallstep.com"}}, + }, + PublicKey: &ecdsa.PublicKey{ + Curve: elliptic.P256(), + X: new(big.Int).SetBytes([]byte{ + 0x8a, 0x8f, 0x43, 0x2f, 0x2b, 0xa0, 0x94, 0xcc, + 0x5a, 0x91, 0x2d, 0xf0, 0xd3, 0x40, 0xd4, 0x29, + 0xd1, 0x9b, 0x79, 0x75, 0xc1, 0xd8, 0xc7, 0xe0, + 0xea, 0xd5, 0x68, 0x7d, 0xe5, 0xd8, 0x6a, 0x7f, + }), + Y: new(big.Int).SetBytes([]byte{ + 0x51, 0x6e, 0xf7, 0xed, 0x66, 0xe7, 0xe2, 0xca, + 0x92, 0x00, 0x56, 0xa1, 0x99, 0xa8, 0xee, 0xc2, + 0x47, 0xd1, 0x1b, 0x92, 0x8c, 0x87, 0x6f, 0xfe, + 0xc6, 0x70, 0xa4, 0x1a, 0xe4, 0x76, 0x7d, 0xac, + }), + }, + PublicKeyAlgorithm: x509.ECDSA, + SignatureAlgorithm: x509.ECDSAWithSHA256, + Signature: []byte{ + 0x30, 0x44, 0x02, 0x20, 0x66, 0x0c, 0xfd, 0x81, + 0xdc, 0x7d, 0x8a, 0x73, 0xa9, 0xe9, 0xb4, 0x97, + 0xe0, 0x49, 0x18, 0x89, 0x40, 0xb2, 0x2d, 0x5f, + 0x71, 0x1a, 0xf6, 0x9b, 0xa2, 0xfb, 0xb9, 0x0b, + 0xd5, 0x24, 0x46, 0xbf, 0x02, 0x20, 0x11, 0x81, + 0x6e, 0x4a, 0x74, 0x97, 0x8b, 0x5e, 0xb0, 0x02, + 0xa8, 0x5d, 0xe9, 0x6c, 0x2f, 0x28, 0x09, 0x8c, + 0xfb, 0x94, 0x19, 0x12, 0xca, 0xf8, 0xeb, 0x5d, + 0x8c, 0xf0, 0x48, 0x37, 0x56, 0x68, + }, + } + type args struct { + filename string + } + tests := []struct { + name string + args args + want *x509.CertificateRequest + wantErr bool + }{ + {"ok", args{"testdata/test.csr"}, expected, false}, + {"ok der", args{"testdata/test.der"}, expected, false}, + {"ok keytool", args{"testdata/keytool.csr"}, expected, false}, + {"fail bad csr", args{"testdata/bad.csr"}, nil, true}, + {"fail certificate", args{"testdata/ca.crt"}, nil, true}, + {"fail certificate der", args{"testdata/ca.der"}, nil, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := os.ReadFile(tt.args.filename) + if err != nil { + t.Fatal(err) + } + got, err := ParseCertificateRequest(b) + if (err != nil) != tt.wantErr { + t.Errorf("ParseCertificateRequest() error = %v, wantErr %v", err, tt.wantErr) + return + } + // Cleanup raw data + if got != nil { + got.Raw = nil + got.RawSubject = nil + got.RawSubjectPublicKeyInfo = nil + got.RawTBSCertificateRequest = nil + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ParseCertificateRequest() = \n%#v, want \n%#v", got, tt.want) + } + }) + } +} + func TestReadCertificateRequest(t *testing.T) { expected := &x509.CertificateRequest{ Subject: pkix.Name{