From 03fcfce97f396153906855640ba13d5da7c313a1 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 22 Apr 2024 14:08:31 -0700 Subject: [PATCH] Allow extra content in Certificate and CSR PEM files (#488) * Introduce new InvalidPEMError that can be validated by clients Co-authored-by: Mariano Cano --------- Co-authored-by: Mariano Cano --- pemutil/pem.go | 83 ++++++++++++++++++++++++++++++++++++++++----- pemutil/pem_test.go | 8 ++--- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/pemutil/pem.go b/pemutil/pem.go index 7d1d3fc5..0ca385c4 100644 --- a/pemutil/pem.go +++ b/pemutil/pem.go @@ -16,6 +16,7 @@ import ( "fmt" "math/big" "os" + "strings" "github.com/pkg/errors" "go.step.sm/crypto/internal/utils" @@ -277,6 +278,60 @@ func ParseCertificateRequest(pemData []byte) (*x509.CertificateRequest, error) { return nil, errors.New("error parsing certificate request: no certificate found") } +// PEMType represents a PEM block type. (e.g., CERTIFICATE, CERTIFICATE REQUEST, etc.) +type PEMType int + +func (pt PEMType) String() string { + switch pt { + case PEMTypeCertificate: + return "certificate" + case PEMTypeCertificateRequest: + return "certificate request" + default: + return "undefined" + } +} + +const ( + // PEMTypeUndefined undefined + PEMTypeUndefined = iota + // PEMTypeCertificate CERTIFICATE + PEMTypeCertificate + // PEMTypeCertificateRequest CERTIFICATE REQUEST + PEMTypeCertificateRequest +) + +// InvalidPEMError represents an error that occurs when parsing a file with +// PEM encoded data. +type InvalidPEMError struct { + Type PEMType + File string + Message string + Err error +} + +func (e *InvalidPEMError) Error() string { + switch { + case e.Message != "": + return e.Message + 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 fmt.Sprintf("%s does not contain a valid PEM encoded %s", prefix, e.Type) + } +} + +func (e *InvalidPEMError) Unwrap() error { + return e.Err +} + // 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) { @@ -328,7 +383,7 @@ func ReadCertificateBundle(filename string) ([]*x509.Certificate, error) { bundle = append(bundle, crt) } if len(bundle) == 0 { - return nil, errors.Errorf("file %s does not contain a valid PEM formatted certificate", filename) + return nil, &InvalidPEMError{File: filename, Type: PEMTypeCertificate} } return bundle, nil } @@ -351,15 +406,25 @@ func ReadCertificateRequest(filename string) (*x509.CertificateRequest, error) { // PEM format if bytes.Contains(b, PEMBlockHeader) { - csr, err := Parse(b, WithFilename(filename)) - if err != nil { - return nil, err - } - switch csr := csr.(type) { - case *x509.CertificateRequest: + 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 - default: - return nil, errors.Errorf("error decoding PEM: file '%s' does not contain a certificate request", filename) } } diff --git a/pemutil/pem_test.go b/pemutil/pem_test.go index 666af40b..bce677a3 100644 --- a/pemutil/pem_test.go +++ b/pemutil/pem_test.go @@ -345,9 +345,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 formatted certificate")}, + {"testdata/badpem.crt", nil, errors.New("file 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 formatted certificate")}, + {"testdata/openssl.p256.pem", nil, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM encoded certificate")}, } for _, tc := range tests { @@ -378,9 +378,9 @@ 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 formatted certificate")}, + {"testdata/badpem.crt", 0, errors.New("file 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 formatted certificate")}, + {"testdata/openssl.p256.pem", 0, errors.New("file testdata/openssl.p256.pem does not contain a valid PEM encoded certificate")}, } for _, tc := range tests {