diff --git a/changelog/27435.txt b/changelog/27435.txt new file mode 100644 index 000000000000..916a81c9017e --- /dev/null +++ b/changelog/27435.txt @@ -0,0 +1,3 @@ +```release-note:bug +helper/pkcs7: Fix parsing certain messages containing only certificates +``` diff --git a/helper/pkcs7/ber.go b/helper/pkcs7/ber.go index eb6b1d0af660..898cf262ee06 100644 --- a/helper/pkcs7/ber.go +++ b/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, diff --git a/helper/pkcs7/ber_test.go b/helper/pkcs7/ber_test.go index d3908f6bc32c..d93bcf57b225 100644 --- a/helper/pkcs7/ber_test.go +++ b/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"}, diff --git a/helper/pkcs7/sign_test.go b/helper/pkcs7/sign_test.go index e08d737563f8..2a4829a7e873 100644 --- a/helper/pkcs7/sign_test.go +++ b/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.