Fix PEM certificate loading that sometimes fails
authorMaximilian Blenk <Maximilian.Blenk@bmw.de>
Tue, 7 Apr 2020 17:33:39 +0000 (19:33 +0200)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 8 May 2020 20:27:47 +0000 (13:27 -0700)
As described in https://github.com/openssl/openssl/issues/9187, the
loading of PEM certificates sometimes fails if a line of base64
content has the length of a multiple of 254.
The problem is in get_header_and_data(). When such a line with a
length of 254 (or a multiple) has been read, the next read will
only read a newline. Due to this get_header_and_data() expects to be
in the header not in the data area. This commit fixes that by checking
if lines have been read completely or only partially. In case of a
previous partial read, a newline will be ignored.

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/11741)

crypto/pem/pem_lib.c
test/recipes/04-test_pem.t
test/recipes/04-test_pem_data/cert-254-chars-at-the-end.pem [new file with mode: 0644]
test/recipes/04-test_pem_data/cert-254-chars-in-the-middle.pem [new file with mode: 0644]
test/recipes/04-test_pem_data/cert-oneline-multiple-of-254.pem [new file with mode: 0644]

index e059328..f5ed70d 100644 (file)
@@ -806,7 +806,7 @@ static int get_header_and_data(BIO *bp, BIO **header, BIO **data, char *name,
 {
     BIO *tmp = *header;
     char *linebuf, *p;
 {
     BIO *tmp = *header;
     char *linebuf, *p;
-    int len, line, ret = 0, end = 0;
+    int len, line, ret = 0, end = 0, prev_partial_line_read = 0, partial_line_read = 0;
     /* 0 if not seen (yet), 1 if reading header, 2 if finished header */
     enum header_status got_header = MAYBE_HEADER;
     unsigned int flags_mask;
     /* 0 if not seen (yet), 1 if reading header, 2 if finished header */
     enum header_status got_header = MAYBE_HEADER;
     unsigned int flags_mask;
@@ -828,6 +828,14 @@ static int get_header_and_data(BIO *bp, BIO **header, BIO **data, char *name,
             goto err;
         }
 
             goto err;
         }
 
+        /*
+         * Check if line has been read completely or if only part of the line
+         * has been read. Keep the previous value to ignore newlines that
+         * appear due to reading a line up until the char before the newline.
+         */
+        prev_partial_line_read = partial_line_read;
+        partial_line_read = len == LINESIZE-1 && linebuf[LINESIZE-2] != '\n';
+
         if (got_header == MAYBE_HEADER) {
             if (memchr(linebuf, ':', len) != NULL)
                 got_header = IN_HEADER;
         if (got_header == MAYBE_HEADER) {
             if (memchr(linebuf, ':', len) != NULL)
                 got_header = IN_HEADER;
@@ -838,13 +846,19 @@ static int get_header_and_data(BIO *bp, BIO **header, BIO **data, char *name,
 
         /* Check for end of header. */
         if (linebuf[0] == '\n') {
 
         /* Check for end of header. */
         if (linebuf[0] == '\n') {
-            if (got_header == POST_HEADER) {
-                /* Another blank line is an error. */
-                PEMerr(PEM_F_GET_HEADER_AND_DATA, PEM_R_BAD_END_LINE);
-                goto err;
+            /*
+             * If previous line has been read only partially this newline is a
+             * regular newline at the end of a line and not an empty line.
+             */
+            if (!prev_partial_line_read) {
+                if (got_header == POST_HEADER) {
+                    /* Another blank line is an error. */
+                    PEMerr(PEM_F_GET_HEADER_AND_DATA, PEM_R_BAD_END_LINE);
+                    goto err;
+                }
+                got_header = POST_HEADER;
+                tmp = *data;
             }
             }
-            got_header = POST_HEADER;
-            tmp = *data;
             continue;
         }
 
             continue;
         }
 
index 0e6e419..d553bec 100644 (file)
@@ -28,6 +28,8 @@ my %cert_expected = (
     "cert-1023line.pem" => 1,
     "cert-1024line.pem" => 1,
     "cert-1025line.pem" => 1,
     "cert-1023line.pem" => 1,
     "cert-1024line.pem" => 1,
     "cert-1025line.pem" => 1,
+    "cert-254-chars-at-the-end.pem" => 1,
+    "cert-254-chars-in-the-middle.pem" => 1,
     "cert-255line.pem" => 1,
     "cert-256line.pem" => 1,
     "cert-257line.pem" => 1,
     "cert-255line.pem" => 1,
     "cert-256line.pem" => 1,
     "cert-257line.pem" => 1,
@@ -43,6 +45,7 @@ my %cert_expected = (
     "cert-misalignedpad.pem" => 0,
     "cert-onecolumn.pem" => 1,
     "cert-oneline.pem" => 1,
     "cert-misalignedpad.pem" => 0,
     "cert-onecolumn.pem" => 1,
     "cert-oneline.pem" => 1,
+    "cert-oneline-multiple-of-254.pem" => 1,
     "cert-shortandlongline.pem" => 1,
     "cert-shortline.pem" => 1,
     "cert-threecolumn.pem" => 1,
     "cert-shortandlongline.pem" => 1,
     "cert-shortline.pem" => 1,
     "cert-threecolumn.pem" => 1,
diff --git a/test/recipes/04-test_pem_data/cert-254-chars-at-the-end.pem b/test/recipes/04-test_pem_data/cert-254-chars-at-the-end.pem
new file mode 100644 (file)
index 0000000..0b6a3ba
--- /dev/null
@@ -0,0 +1,6 @@
+-----BEGIN CERTIFICATE-----
+MIIEcjCCAyegAwIBAgIUPLgYY73GEwkikNCKRJrcbCR+TbQwDQYJKoZIhvcNAQELBQAwgZUxCzAJBgNVBAYTAkFVMWMwYQYDVQQIDFpUaGUgR3JlYXQgU3RhdGUgb2YgTG9uZy1XaW5kZWQgQ2VydGlmaWNhdGUgRmllbGQgTmFtZXMgV2hlcmVieSB0byBJbmNyZWFzZSB0aGUgT3V0cHV0IFNpemUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMDA0MDcwMDAwNDJaFw0zMDA0MDUwMDAwNDJaMIGVMQswCQYDVQQGEwJBVTFjMGEGA1UECAxaVGhlIEdyZWF0IFN0YXRlIG9mIExvbmctV2luZGVkIENlcnRpZmljYXRlIEZpZWxkIE5hbWVzIFdoZXJlYnkgdG8gSW5jcmVhc2UgdGhlIE91dHB1dCBTaXplMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggFUMA0GCSqGSIb3DQEBAQUAA4IBQQAwggE8AoIBMwLf
+mipKB41NPXrbp/T5eu+fndvZq72N/Tq0vZp2dRoz89NEFC3jYVBjp4pmVwCS9F/fGX1tnVfhb9k/4fqiI/y9lBVzxaHyMG/pt0D2nTS8iaMTM7uBeRvB5rUZlEbU8uvv4GXu3CeP/NnVceXruGbPb4IpjfoUbGLvn5oK35h8a+LNY5f7QRBlAXtUwYrdxVzT+CqQ4wIAuqoIVXgRIweveS1ArbS8hOtsVnu1bUAQVKqORHx8gtbOyiA4heTCEOkwh45YV6KW+uLI1wTeE4E9erlI4RwZ7umbBnQai/hYL//AUfQKQhpGbgfyJrS0UYY7WEP/mcFQh0U2EBTXtAy/e4XPiftViR3+pd+G2TJ/JFofDDzJRrceeo
+9tUnMr0pKtU7oB77lSKgsruKKkhn6lLH8CAwEAAaNTMFEwHQYDVR0OBBYEFIkawSiFUdL6G3jw8qg1WQI8Xi4rMB8GA1UdIwQYMBaAFIkawSiFUdL6G3jw8qg1WQI8Xi4rMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggE0AAHe/+71vykcq9BQ5h2X7MpnkE5n0Yn0Xi24uuCpv59JjABmOdaeT6XBQ5UJN8WfidawgzbJ6WiWgjflaMfRfjsdCJRgvdw0gfXXXrsseJMeMYnw1hQTGuB83BKjXBdL6zb45qGf2Fgjm3aNW2NUVM+Q2QfMjo
+Kx13hTyDh9l5nOhMv/Rkygcx1Row2WbkvrhxvCLxY0VhL7RuPV8K0ogKicv8VJgQriOUVTTkqBP1xUimKSTaNaZ8KAnC7thxxZHxsNa45a6AouPSzyAOPZQgCJW83OIFxvWsdYU1KvP1wmoi1XC9giSQ/5sLPu/eAYTzmY+Xd6Sq8dF8uyodeI2gFu3AzC28PVKeUriIGfxaqEUn+aXx5W+r8JTE6fQ9mBo9YxJBXG+OTIFgHR27q2dJwqK9c=
+-----END CERTIFICATE-----
diff --git a/test/recipes/04-test_pem_data/cert-254-chars-in-the-middle.pem b/test/recipes/04-test_pem_data/cert-254-chars-in-the-middle.pem
new file mode 100644 (file)
index 0000000..cc9076b
--- /dev/null
@@ -0,0 +1,5 @@
+-----BEGIN CERTIFICATE-----
+MIIEcjCCAyegAwIBAgIUPLgYY73GEwkikNCKRJrcbCR+TbQwDQYJKoZIhvcNAQELBQAwgZUxCzAJBgNVBAYTAkFVMWMwYQYDVQQIDFpUaGUgR3JlYXQgU3RhdGUgb2YgTG9uZy1XaW5kZWQgQ2VydGlmaWNhdGUgRmllbGQgTmFtZXMgV2hlcmVieSB0byBJbmNyZWFzZSB0aGUgT
+3V0cHV0IFNpemUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMDA0MDcwMDAwNDJaFw0zMDA0MDUwMDAwNDJaMIGVMQswCQYDVQQGEwJBVTFjMGEGA1UECAxaVGhlIEdyZWF0IFN0YXRlIG9mIExvbmctV2luZGVkIENlcnRpZmljYXRlIEZpZWxkIE5hbWVzIFdoZXJlYnkgdG8gSW5jcmVhc2UgdGhlIE91dHB1dCB
+TaXplMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggFUMA0GCSqGSIb3DQEBAQUAA4IBQQAwggE8AoIBMwLfmipKB41NPXrbp/T5eu+fndvZq72N/Tq0vZp2dRoz89NEFC3jYVBjp4pmVwCS9F/fGX1tnVfhb9k/4fqiI/y9lBVzxaHyMG/pt0D2nTS8iaMTM7uBeRvB5rUZlEbU8uvv4GXu3CeP/NnVceXruGbPb4IpjfoUbGLvn5oK35h8a+LNY5f7QRBlAXtUwYrdxVzT+CqQ4wIAuqoIVXgRIweveS1ArbS8hOtsVnu1bUAQVKqORHx8gtbOyiA4heTCEOkwh45YV6KW+uLI1wTeE4E9erlI4RwZ7umbBnQai/hYL//AUfQKQhpGbgfyJrS0UYY7WEP/mcFQh0U2EBTXtAy/e4XPiftViR3+pd+G2TJ/JFofDDzJRrceeo9tUnMr0pKtU7oB77lSKgsruKKkhn6lLH8CAwEAAaNTMFEwHQYDVR0OBBYEFIkawSiFUdL6G3jw8qg1WQI8Xi4rMB8GA1UdIwQYMBaAFIkawSiFUdL6G3jw8qg1WQI8Xi4rMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggE0AAHe/+71vykcq9BQ5h2X7MpnkE5n0Yn0Xi24uuCpv59JjABmOdaeT6XBQ5UJN8WfidawgzbJ6WiWgjflaMfRfjsdCJRgvdw0gfXXXrsseJMeMYnw1hQTGuB83BKjXBdL6zb45qGf2Fgjm3aNW2NUVM+Q2QfMjoKx13hTyDh9l5nOhMv/Rkygcx1Row2WbkvrhxvCLxY0VhL7RuPV8K0ogKicv8VJgQriOUVTTkqBP1xUimKSTaNaZ8KAnC7thxxZHxsNa45a6AouPSzyAOPZQgCJW83OIFxvWsdYU1KvP1wmoi1XC9giSQ/5sLPu/eAYTzmY+Xd6Sq8dF8uyodeI2gFu3AzC28PVKeUriIGfxaqEUn+aXx5W+r8JTE6fQ9mBo9YxJBXG+OTIFgHR27q2dJwqK9c=
+-----END CERTIFICATE-----
diff --git a/test/recipes/04-test_pem_data/cert-oneline-multiple-of-254.pem b/test/recipes/04-test_pem_data/cert-oneline-multiple-of-254.pem
new file mode 100644 (file)
index 0000000..e0af859
--- /dev/null
@@ -0,0 +1,3 @@
+-----BEGIN CERTIFICATE-----
+MIIEcjCCAyegAwIBAgIUPLgYY73GEwkikNCKRJrcbCR+TbQwDQYJKoZIhvcNAQELBQAwgZUxCzAJBgNVBAYTAkFVMWMwYQYDVQQIDFpUaGUgR3JlYXQgU3RhdGUgb2YgTG9uZy1XaW5kZWQgQ2VydGlmaWNhdGUgRmllbGQgTmFtZXMgV2hlcmVieSB0byBJbmNyZWFzZSB0aGUgT3V0cHV0IFNpemUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMDA0MDcwMDAwNDJaFw0zMDA0MDUwMDAwNDJaMIGVMQswCQYDVQQGEwJBVTFjMGEGA1UECAxaVGhlIEdyZWF0IFN0YXRlIG9mIExvbmctV2luZGVkIENlcnRpZmljYXRlIEZpZWxkIE5hbWVzIFdoZXJlYnkgdG8gSW5jcmVhc2UgdGhlIE91dHB1dCBTaXplMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggFUMA0GCSqGSIb3DQEBAQUAA4IBQQAwggE8AoIBMwLfmipKB41NPXrbp/T5eu+fndvZq72N/Tq0vZp2dRoz89NEFC3jYVBjp4pmVwCS9F/fGX1tnVfhb9k/4fqiI/y9lBVzxaHyMG/pt0D2nTS8iaMTM7uBeRvB5rUZlEbU8uvv4GXu3CeP/NnVceXruGbPb4IpjfoUbGLvn5oK35h8a+LNY5f7QRBlAXtUwYrdxVzT+CqQ4wIAuqoIVXgRIweveS1ArbS8hOtsVnu1bUAQVKqORHx8gtbOyiA4heTCEOkwh45YV6KW+uLI1wTeE4E9erlI4RwZ7umbBnQai/hYL//AUfQKQhpGbgfyJrS0UYY7WEP/mcFQh0U2EBTXtAy/e4XPiftViR3+pd+G2TJ/JFofDDzJRrceeo9tUnMr0pKtU7oB77lSKgsruKKkhn6lLH8CAwEAAaNTMFEwHQYDVR0OBBYEFIkawSiFUdL6G3jw8qg1WQI8Xi4rMB8GA1UdIwQYMBaAFIkawSiFUdL6G3jw8qg1WQI8Xi4rMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggE0AAHe/+71vykcq9BQ5h2X7MpnkE5n0Yn0Xi24uuCpv59JjABmOdaeT6XBQ5UJN8WfidawgzbJ6WiWgjflaMfRfjsdCJRgvdw0gfXXXrsseJMeMYnw1hQTGuB83BKjXBdL6zb45qGf2Fgjm3aNW2NUVM+Q2QfMjoKx13hTyDh9l5nOhMv/Rkygcx1Row2WbkvrhxvCLxY0VhL7RuPV8K0ogKicv8VJgQriOUVTTkqBP1xUimKSTaNaZ8KAnC7thxxZHxsNa45a6AouPSzyAOPZQgCJW83OIFxvWsdYU1KvP1wmoi1XC9giSQ/5sLPu/eAYTzmY+Xd6Sq8dF8uyodeI2gFu3AzC28PVKeUriIGfxaqEUn+aXx5W+r8JTE6fQ9mBo9YxJBXG+OTIFgHR27q2dJwqK9c=
+-----END CERTIFICATE-----