Allow extra content in Certificate and CSR PEM files (#488)

* Introduce new InvalidPEMError that can be validated by clients

Co-authored-by: Mariano Cano <[email protected]>

---------

Co-authored-by: Mariano Cano <[email protected]>

Author: dopey

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)
 		}
 	}
 

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 {