Fix PKCS7 parser failing to parse degenerated certificate messages

changelog/27435.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+helper/pkcs7: Fix parsing certain messages containing only certificates
+```

helper/pkcs7/ber.go

@@ -140,25 +140,22 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
 	tagStart := offset
 	b := ber[offset]
 	offset++
-	if offset >= berLen {
-		return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
-	}
 	tag := b & 0x1F // last 5 bits
 	if tag == 0x1F {
 		tag = 0
+		if offset >= berLen {
+			return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
+		}
 		for ber[offset] >= 0x80 {
-			tag = tag*128 + ber[offset] - 0x80
-			offset++
 			if offset >= berLen {
 				return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
 			}
+			tag = tag*128 + ber[offset] - 0x80
+			offset++
 		}
 		// jvehent 20170227: this doesn't appear to be used anywhere...
 		// tag = tag*128 + ber[offset] - 0x80
 		offset++
-		if offset >= berLen {
-			return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
-		}
 	}
 	tagEnd := offset
 
@@ -170,14 +167,17 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
 	}
 	// read length
 	var length int
-	l := ber[offset]
-	offset++
 	if offset >= berLen {
 		return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
 	}
+	l := ber[offset]
+	offset++
 	indefinite := false
 	if l > 0x80 {
 		numberOfBytes := (int)(l & 0x7F)
+		if offset >= berLen {
+			return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
+		}
 		if numberOfBytes > 4 { // int is only guaranteed to be 32bit
 			return nil, 0, errors.New("ber2der: BER tag length too long")
 		}
@@ -217,6 +217,9 @@ func readObject(ber []byte, offset int) (asn1Object, int, error) {
 		return nil, 0, errors.New("ber2der: Indefinite form tag must have constructed encoding")
 	}
 	if kind == 0 {
+		if offset >= berLen {
+			return nil, 0, errors.New("ber2der: cannot move offset forward, end of ber data reached")
+		}
 		obj = asn1Primitive{
 			tagBytes: ber[tagStart:tagEnd],
 			length:   length,

helper/pkcs7/ber_test.go

@@ -80,7 +80,7 @@ func TestBer2Der_Negatives(t *testing.T) {
 		{[]byte{0x30, 0x84, 0x80, 0x0, 0x0, 0x0}, "length is negative"},
 		{[]byte{0x30, 0x82, 0x0, 0x1}, "length has leading zero"},
 		{[]byte{0x30, 0x80, 0x1, 0x2, 0x1, 0x2}, "Invalid BER format"},
-		{[]byte{0x30, 0x80, 0x1, 0x2}, "end of ber data reached"},
+		{[]byte{0x30, 0x80, 0x1, 0x2}, "BER tag length is more than available data"},
 		{[]byte{0x30, 0x03, 0x01, 0x02}, "length is more than available data"},
 		{[]byte{0x30}, "end of ber data reached"},
 		{[]byte("?0"), "end of ber data reached"},

helper/pkcs7/sign_test.go

@@ -256,6 +256,12 @@ func TestDegenerateCertificate(t *testing.T) {
 	}
 	testOpenSSLParse(t, deg)
 	pem.Encode(os.Stdout, &pem.Block{Type: "PKCS7", Bytes: deg})
+
+	// Make sure the library can parse the PKCS7 we generated along with OpenSSL
+	_, err = Parse(deg)
+	if err != nil {
+		t.Fatalf("failed parsing degenerated certificate: %v", err)
+	}
 }
 
 // writes the cert to a temporary file and tests that openssl can read it.