Merge branch 'iotssl-629-der-trailing-bytes'
Fixes bug in mbedtls_x509_crt_parse that caused trailing extra data in the
buffer following DER certificates to be included in the raw representation.
diff --git a/ChangeLog b/ChangeLog
index 5539181..007f604 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -14,6 +14,8 @@
* Fix an issue that caused valid certificates to be rejected whenever an
expired or not yet valid certificate was parsed before a valid certificate
in the trusted certificate list.
+ * Fix bug in mbedtls_x509_crt_parse that caused trailing extra data in the
+ buffer after DER certificates to be included in the raw representation.
Changes
* On ARM platforms, when compiling with -O0 with GCC, Clang or armcc5,
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 334b8ef..c3adf7c 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -677,14 +677,9 @@
if( crt == NULL || buf == NULL )
return( MBEDTLS_ERR_X509_BAD_INPUT_DATA );
- p = mbedtls_calloc( 1, len = buflen );
- if( p == NULL )
- return( MBEDTLS_ERR_X509_ALLOC_FAILED );
-
- memcpy( p, buf, buflen );
-
- crt->raw.p = p;
- crt->raw.len = len;
+ // Use the original buffer until we figure out actual length
+ p = (unsigned char*) buf;
+ len = buflen;
end = p + len;
/*
@@ -708,6 +703,18 @@
}
crt_end = p + len;
+ // Create and populate a new buffer for the raw field
+ crt->raw.len = crt_end - buf;
+ crt->raw.p = p = mbedtls_calloc( 1, crt->raw.len );
+ if( p == NULL )
+ return( MBEDTLS_ERR_X509_ALLOC_FAILED );
+
+ memcpy( p, buf, crt->raw.len );
+
+ // Direct pointers to the new buffer
+ p += crt->raw.len - len;
+ end = crt_end = p + len;
+
/*
* TBSCertificate ::= SEQUENCE {
*/
diff --git a/tests/data_files/server5-der0.crt b/tests/data_files/server5-der0.crt
new file mode 100644
index 0000000..08d8dd3
--- /dev/null
+++ b/tests/data_files/server5-der0.crt
Binary files differ
diff --git a/tests/data_files/server5-der1a.crt b/tests/data_files/server5-der1a.crt
new file mode 100644
index 0000000..015017b
--- /dev/null
+++ b/tests/data_files/server5-der1a.crt
Binary files differ
diff --git a/tests/data_files/server5-der1b.crt b/tests/data_files/server5-der1b.crt
new file mode 100644
index 0000000..6340d9e
--- /dev/null
+++ b/tests/data_files/server5-der1b.crt
Binary files differ
diff --git a/tests/data_files/server5-der2.crt b/tests/data_files/server5-der2.crt
new file mode 100644
index 0000000..c6e320a
--- /dev/null
+++ b/tests/data_files/server5-der2.crt
Binary files differ
diff --git a/tests/data_files/server5-der4.crt b/tests/data_files/server5-der4.crt
new file mode 100644
index 0000000..4af05cc
--- /dev/null
+++ b/tests/data_files/server5-der4.crt
Binary files differ
diff --git a/tests/data_files/server5-der8.crt b/tests/data_files/server5-der8.crt
new file mode 100644
index 0000000..65be7dc
--- /dev/null
+++ b/tests/data_files/server5-der8.crt
Binary files differ
diff --git a/tests/data_files/server5-der9.crt b/tests/data_files/server5-der9.crt
new file mode 100644
index 0000000..4947f1f
--- /dev/null
+++ b/tests/data_files/server5-der9.crt
Binary files differ
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 8792b21..c08af7b 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1559,6 +1559,64 @@
-S "received TLS_EMPTY_RENEGOTIATION_INFO\|found renegotiation extension" \
-S "server hello, secure renegotiation extension"
+# Tests for silently dropping trailing extra bytes in .der certificates
+
+requires_gnutls
+run_test "DER format: no trailing bytes" \
+ "$P_SRV crt_file=data_files/server5-der0.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with a trailing zero byte" \
+ "$P_SRV crt_file=data_files/server5-der1a.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with a trailing random byte" \
+ "$P_SRV crt_file=data_files/server5-der1b.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with 2 trailing random bytes" \
+ "$P_SRV crt_file=data_files/server5-der2.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with 4 trailing random bytes" \
+ "$P_SRV crt_file=data_files/server5-der4.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with 8 trailing random bytes" \
+ "$P_SRV crt_file=data_files/server5-der8.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
+requires_gnutls
+run_test "DER format: with 9 trailing random bytes" \
+ "$P_SRV crt_file=data_files/server5-der9.crt \
+ key_file=data_files/server5.key" \
+ "$G_CLI " \
+ 0 \
+ -c "Handshake was completed" \
+
# Tests for auth_mode
run_test "Authentication: server badcert, client required" \
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index b21a640..5c68872 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -771,7 +771,7 @@
x509parse_crt:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT
X509 Certificate ASN1 (Correct first tag, data length does not match)
-x509parse_crt:"300000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG
+x509parse_crt:"300000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA
X509 Certificate ASN1 (Correct first tag, no more data)
x509parse_crt:"3000":"":MBEDTLS_ERR_X509_INVALID_FORMAT + MBEDTLS_ERR_ASN1_OUT_OF_DATA