Backport: Clean up & improve PK write test functions

tests/data_files/Makefile

@@ -888,6 +888,57 @@ ec_prv.pk8param.pem: ec_prv.pk8param.der
 	$(OPENSSL) pkey -in $< -inform DER -out $@
 all_final += ec_prv.pk8param.pem
 
+################################################################
+#### Convert PEM keys in DER format
+################################################################
+server1.pubkey.der: server1.pubkey
+	$(OPENSSL) pkey -pubin -in $< -out $@ -outform DER
+all_final += server1.pubkey.der
+
+rsa4096_pub.der: rsa4096_pub.pem
+	$(OPENSSL) pkey -pubin -in $< -out $@ -outform DER
+all_final += rsa4096_pub.der
+
+ec_pub.der: ec_pub.pem
+	$(OPENSSL) pkey -pubin -in $< -out $@ -outform DER
+all_final += ec_pub.der
+
+ec_521_pub.der: ec_521_pub.pem
+	$(OPENSSL) pkey -pubin -in $< -out $@ -outform DER
+all_final += ec_521_pub.der
+
+ec_bp512_pub.der: ec_bp512_pub.pem
+	$(OPENSSL) pkey -pubin -in $< -out $@ -outform DER
+all_final += ec_bp512_pub.der
+
+server1.key.der: server1.key
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += server1.key.der
+
+rsa4096_prv.der: rsa4096_prv.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += rsa4096_prv.der
+
+ec_prv.sec1.der: ec_prv.sec1.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += ec_prv.sec1.der
+
+ec_256_long_prv.der: ec_256_long_prv.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += ec_256_long_prv.der
+
+ec_521_prv.der: ec_521_prv.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += ec_521_prv.der
+
+ec_521_short_prv.der: ec_521_short_prv.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += ec_521_short_prv.der
+
+ec_bp512_prv.der: ec_bp512_prv.pem
+	$(OPENSSL) pkey -in $< -out $@ -outform DER
+all_final += ec_bp512_prv.der
+
 ################################################################
 ### Generate CSRs for X.509 write test suite
 ################################################################

tests/suites/test_suite_pkwrite.data

@@ -1,47 +1,95 @@
 Public key write check RSA
-depends_on:MBEDTLS_RSA_C:MBEDTLS_BASE64_C
-pk_write_pubkey_check:"data_files/server1.pubkey"
+depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C
+pk_write_pubkey_check:"data_files/server1.pubkey":TEST_PEM
+
+Public key write check RSA (DER)
+depends_on:MBEDTLS_RSA_C
+pk_write_pubkey_check:"data_files/server1.pubkey.der":TEST_DER
 
 Public key write check RSA 4096
-depends_on:MBEDTLS_RSA_C:MBEDTLS_BASE64_C
-pk_write_pubkey_check:"data_files/rsa4096_pub.pem"
+depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C
+pk_write_pubkey_check:"data_files/rsa4096_pub.pem":TEST_PEM
+
+Public key write check RSA 4096 (DER)
+depends_on:MBEDTLS_RSA_C
+pk_write_pubkey_check:"data_files/rsa4096_pub.der":TEST_DER
 
 Public key write check EC 192 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-pk_write_pubkey_check:"data_files/ec_pub.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_pub.pem":TEST_PEM
+
+Public key write check EC 192 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_pub.der":TEST_DER
 
 Public key write check EC 521 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
-pk_write_pubkey_check:"data_files/ec_521_pub.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_521_pub.pem":TEST_PEM
+
+Public key write check EC 521 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_521_pub.der":TEST_DER
 
 Public key write check EC Brainpool 512 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
-pk_write_pubkey_check:"data_files/ec_bp512_pub.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_bp512_pub.pem":TEST_PEM
+
+Public key write check EC Brainpool 512 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
+pk_write_pubkey_check:"data_files/ec_bp512_pub.der":TEST_DER
 
 Private key write check RSA
-depends_on:MBEDTLS_RSA_C:MBEDTLS_BASE64_C
-pk_write_key_check:"data_files/server1.key"
+depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C
+pk_write_key_check:"data_files/server1.key":TEST_PEM
+
+Private key write check RSA (DER)
+depends_on:MBEDTLS_RSA_C
+pk_write_key_check:"data_files/server1.key.der":TEST_DER
 
 Private key write check RSA 4096
-depends_on:MBEDTLS_RSA_C:MBEDTLS_BASE64_C
-pk_write_key_check:"data_files/rsa4096_prv.pem"
+depends_on:MBEDTLS_RSA_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C
+pk_write_key_check:"data_files/rsa4096_prv.pem":TEST_PEM
+
+Private key write check RSA 4096 (DER)
+depends_on:MBEDTLS_RSA_C
+pk_write_key_check:"data_files/rsa4096_prv.der":TEST_DER
 
 Private key write check EC 192 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
-pk_write_key_check:"data_files/ec_prv.sec1.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
+pk_write_key_check:"data_files/ec_prv.sec1.pem":TEST_PEM
+
+Private key write check EC 192 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
+pk_write_key_check:"data_files/ec_prv.sec1.der":TEST_DER
 
 Private key write check EC 256 bits (top bit set)
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
-pk_write_key_check:"data_files/ec_256_long_prv.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_write_key_check:"data_files/ec_256_long_prv.pem":TEST_PEM
+
+Private key write check EC 256 bits (top bit set) (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_write_key_check:"data_files/ec_256_long_prv.der":TEST_DER
 
 Private key write check EC 521 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
-pk_write_key_check:"data_files/ec_521_prv.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_key_check:"data_files/ec_521_prv.pem":TEST_PEM
+
+Private key write check EC 521 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_key_check:"data_files/ec_521_prv.der":TEST_DER
 
 Private key write check EC 521 bits (top byte is 0)
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
-pk_write_key_check:"data_files/ec_521_short_prv.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_key_check:"data_files/ec_521_short_prv.pem":TEST_PEM
+
+Private key write check EC 521 bits (top byte is 0) (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP521R1_ENABLED
+pk_write_key_check:"data_files/ec_521_short_prv.der":TEST_DER
 
 Private key write check EC Brainpool 512 bits
-depends_on:MBEDTLS_ECP_C:MBEDTLS_BASE64_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
-pk_write_key_check:"data_files/ec_bp512_prv.pem"
+depends_on:MBEDTLS_ECP_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_PEM_WRITE_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
+pk_write_key_check:"data_files/ec_bp512_prv.pem":TEST_PEM
+
+Private key write check EC Brainpool 512 bits (DER)
+depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_BP512R1_ENABLED
+pk_write_key_check:"data_files/ec_bp512_prv.der":TEST_DER

tests/suites/test_suite_pkwrite.function

@@ -2,87 +2,89 @@
 #include "mbedtls/pk.h"
 #include "mbedtls/pem.h"
 #include "mbedtls/oid.h"
-/* END_HEADER */
 
-/* BEGIN_DEPENDENCIES
- * depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_BIGNUM_C:MBEDTLS_FS_IO
- * END_DEPENDENCIES
- */
+typedef enum {
+    TEST_PEM,
+    TEST_DER
+} pkwrite_file_format_t;
 
-/* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */
-void pk_write_pubkey_check(char *key_file)
+static void pk_write_check_common(char *key_file, int is_public_key, int is_der)
 {
     mbedtls_pk_context key;
-    unsigned char buf[5000];
-    unsigned char check_buf[5000];
+    unsigned char *buf = NULL;
+    unsigned char *check_buf = NULL;
+    unsigned char *start_buf;
+    size_t buf_len, check_buf_len;
     int ret;
-    FILE *f;
-    size_t ilen, pem_len, buf_index;
 
-    memset(buf, 0, sizeof(buf));
-    memset(check_buf, 0, sizeof(check_buf));
+    /* Note: if mbedtls_pk_load_file() successfully reads the file, then
+       it also allocates check_buf, which should be freed on exit */
+    TEST_EQUAL(mbedtls_pk_load_file(key_file, &check_buf, &check_buf_len), 0);
+    TEST_ASSERT(check_buf_len > 0);
 
-    mbedtls_pk_init(&key);
-    TEST_ASSERT(mbedtls_pk_parse_public_keyfile(&key, key_file) == 0);
-
-    ret = mbedtls_pk_write_pubkey_pem(&key, buf, sizeof(buf));
-    TEST_ASSERT(ret == 0);
+    ASSERT_ALLOC(buf, check_buf_len);
 
-    pem_len = strlen((char *) buf);
-
-    // check that the rest of the buffer remains clear
-    for (buf_index = pem_len; buf_index < sizeof(buf); ++buf_index) {
-        TEST_ASSERT(buf[buf_index] == 0);
+    mbedtls_pk_init(&key);
+    if (is_public_key) {
+        TEST_EQUAL(mbedtls_pk_parse_public_keyfile(&key, key_file), 0);
+        if (is_der) {
+            ret = mbedtls_pk_write_pubkey_der(&key, buf, check_buf_len);
+        } else {
+#if defined(MBEDTLS_PEM_WRITE_C)
+            ret = mbedtls_pk_write_pubkey_pem(&key, buf, check_buf_len);
+#else
+            ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
+#endif
+        }
+    } else {
+        TEST_EQUAL(mbedtls_pk_parse_keyfile(&key, key_file, NULL), 0);
+        if (is_der) {
+            ret = mbedtls_pk_write_key_der(&key, buf, check_buf_len);
+        } else {
+#if defined(MBEDTLS_PEM_WRITE_C)
+            ret = mbedtls_pk_write_key_pem(&key, buf, check_buf_len);
+#else
+            ret = MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE;
+#endif
+        }
     }
 
-    f = fopen(key_file, "r");
-    TEST_ASSERT(f != NULL);
-    ilen = fread(check_buf, 1, sizeof(check_buf), f);
-    fclose(f);
+    if (is_der) {
+        TEST_LE_U(1, ret);
+        buf_len = ret;
+        start_buf = buf + check_buf_len - buf_len;
+    } else {
+        TEST_EQUAL(ret, 0);
+        buf_len = strlen((char *) buf) + 1; /* +1 takes the string terminator into account */
+        start_buf = buf;
+    }
 
-    TEST_ASSERT(ilen == pem_len);
-    TEST_ASSERT(memcmp((char *) buf, (char *) check_buf, ilen) == 0);
+    ASSERT_COMPARE(start_buf, buf_len, check_buf, check_buf_len);
 
 exit:
+    mbedtls_free(buf);
+    mbedtls_free(check_buf);
     mbedtls_pk_free(&key);
 }
-/* END_CASE */
-
-/* BEGIN_CASE depends_on:MBEDTLS_PEM_WRITE_C */
-void pk_write_key_check(char *key_file)
-{
-    mbedtls_pk_context key;
-    unsigned char buf[5000];
-    unsigned char check_buf[5000];
-    int ret;
-    FILE *f;
-    size_t ilen, pem_len, buf_index;
-
-    memset(buf, 0, sizeof(buf));
-    memset(check_buf, 0, sizeof(check_buf));
-
-    mbedtls_pk_init(&key);
-    TEST_ASSERT(mbedtls_pk_parse_keyfile(&key, key_file, NULL) == 0);
-
-    ret = mbedtls_pk_write_key_pem(&key, buf, sizeof(buf));
-    TEST_ASSERT(ret == 0);
-
-    pem_len = strlen((char *) buf);
-
-    // check that the rest of the buffer remains clear
-    for (buf_index = pem_len; buf_index < sizeof(buf); ++buf_index) {
-        TEST_ASSERT(buf[buf_index] == 0);
-    }
+/* END_HEADER */
 
-    f = fopen(key_file, "r");
-    TEST_ASSERT(f != NULL);
-    ilen = fread(check_buf, 1, sizeof(check_buf), f);
-    fclose(f);
+/* BEGIN_DEPENDENCIES
+ * depends_on:MBEDTLS_PK_PARSE_C:MBEDTLS_PK_WRITE_C:MBEDTLS_BIGNUM_C:MBEDTLS_FS_IO
+ * END_DEPENDENCIES
+ */
 
-    TEST_ASSERT(ilen == strlen((char *) buf));
-    TEST_ASSERT(memcmp((char *) buf, (char *) check_buf, ilen) == 0);
+/* BEGIN_CASE */
+void pk_write_pubkey_check(char *key_file, int is_der)
+{
+    pk_write_check_common(key_file, 1, is_der);
+    goto exit; /* make the compiler happy */
+}
+/* END_CASE */
 
-exit:
-    mbedtls_pk_free(&key);
+/* BEGIN_CASE */
+void pk_write_key_check(char *key_file, int is_der)
+{
+    pk_write_check_common(key_file, 0, is_der);
+    goto exit; /* make the compiler happy */
 }
 /* END_CASE */