rsa: simplify mbedtls_rsa_parse_pubkey() input parameters

In this way mbedtls_rsa_parse_pubkey() and mbedtls_rsa_parse_key()
input parameter list is the same.

Signed-off-by: Valerio Setti <valerio.setti@nordicsemi.no>

library/pkparse.c

@@ -850,7 +850,7 @@ int mbedtls_pk_parse_subpubkey(unsigned char **p, const unsigned char *end,
 
 #if defined(MBEDTLS_RSA_C)
     if (pk_alg == MBEDTLS_PK_RSA) {
-        ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*pk), p, end);
+        ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*pk), *p, (size_t) (end - *p));
     } else
 #endif /* MBEDTLS_RSA_C */
 #if defined(MBEDTLS_PK_HAVE_ECC_KEYS)
@@ -1504,7 +1504,7 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx,
             return ret;
         }
 
-        if ((ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), &p, p + pem.buflen)) != 0) {
+        if ((ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), p, pem.buflen)) != 0) {
             mbedtls_pk_free(ctx);
         }
 
@@ -1551,7 +1551,7 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx,
     }
 
     p = (unsigned char *) key;
-    ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), &p, p + keylen);
+    ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), p, keylen);
     if (ret == 0) {
         return ret;
     }

library/psa_crypto_rsa.c

@@ -73,9 +73,7 @@ psa_status_t mbedtls_psa_rsa_load_representation(
     if (PSA_KEY_TYPE_IS_KEY_PAIR(type)) {
         status = mbedtls_to_psa_error(mbedtls_rsa_parse_key(*p_rsa, data, data_length));
     } else {
-        unsigned char *p = data;
+        status = mbedtls_to_psa_error(mbedtls_rsa_parse_pubkey(*p_rsa, data, data_length));
-        unsigned char *end = (data + data_length);
-        status = mbedtls_to_psa_error(mbedtls_rsa_parse_pubkey(*p_rsa, &p, end));
     }
     if (status != PSA_SUCCESS) {
         goto exit;

library/rsa.c

@@ -46,6 +46,34 @@
 
 #include "mbedtls/platform.h"
 
+/*
+ * Wrapper around mbedtls_asn1_get_mpi() that rejects zero.
+ *
+ * The value zero is:
+ * - never a valid value for an RSA parameter
+ * - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete().
+ *
+ * Since values can't be omitted in PKCS#1, passing a zero value to
+ * rsa_complete() would be incorrect, so reject zero values early.
+ */
+static int asn1_get_nonzero_mpi(unsigned char **p,
+                                const unsigned char *end,
+                                mbedtls_mpi *X)
+{
+    int ret;
+
+    ret = mbedtls_asn1_get_mpi(p, end, X);
+    if (ret != 0) {
+        return ret;
+    }
+
+    if (mbedtls_mpi_cmp_int(X, 0) == 0) {
+        return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
+    }
+
+    return 0;
+}
+
 int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen)
 {
     int ret, version;
@@ -192,9 +220,10 @@ cleanup:
     return ret;
 }
 
-int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p,
+int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen)
-                             const unsigned char *end)
 {
+    unsigned char *p = (unsigned char *) key;
+    unsigned char *end = (unsigned char *) (key + keylen);
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
     size_t len;
 
@@ -205,45 +234,45 @@ int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p,
      *  }
      */
 
-    if ((ret = mbedtls_asn1_get_tag(p, end, &len,
+    if ((ret = mbedtls_asn1_get_tag(&p, end, &len,
                                     MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0) {
         return ret;
     }
 
-    if (*p + len != end) {
+    if (p + len != end) {
         return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH;
     }
 
     /* Import N */
-    if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) {
+    if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) {
         return ret;
     }
 
-    if ((ret = mbedtls_rsa_import_raw(rsa, *p, len, NULL, 0, NULL, 0,
+    if ((ret = mbedtls_rsa_import_raw(rsa, p, len, NULL, 0, NULL, 0,
                                       NULL, 0, NULL, 0)) != 0) {
         return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
     }
 
-    *p += len;
+    p += len;
 
     /* Import E */
-    if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) {
+    if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) {
         return ret;
     }
 
     if ((ret = mbedtls_rsa_import_raw(rsa, NULL, 0, NULL, 0, NULL, 0,
-                                      NULL, 0, *p, len)) != 0) {
+                                      NULL, 0, p, len)) != 0) {
         return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
     }
 
-    *p += len;
+    p += len;
 
     if (mbedtls_rsa_complete(rsa) != 0 ||
         mbedtls_rsa_check_pubkey(rsa) != 0) {
         return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
     }
 
-    if (*p != end) {
+    if (p != end) {
         return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH;
     }
 
@@ -992,34 +1021,6 @@ size_t mbedtls_rsa_get_len(const mbedtls_rsa_context *ctx)
     return ctx->len;
 }
 
-/*
- * Wrapper around mbedtls_asn1_get_mpi() that rejects zero.
- *
- * The value zero is:
- * - never a valid value for an RSA parameter
- * - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete().
- *
- * Since values can't be omitted in PKCS#1, passing a zero value to
- * rsa_complete() would be incorrect, so reject zero values early.
- */
-static int asn1_get_nonzero_mpi(unsigned char **p,
-                                const unsigned char *end,
-                                mbedtls_mpi *X)
-{
-    int ret;
-
-    ret = mbedtls_asn1_get_mpi(p, end, X);
-    if (ret != 0) {
-        return ret;
-    }
-
-    if (mbedtls_mpi_cmp_int(X, 0) == 0) {
-        return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
-    }
-
-    return 0;
-}
-
 #if defined(MBEDTLS_GENPRIME)
 
 /*

library/rsa_internal.h

@@ -34,18 +34,15 @@ int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, si
  * \brief           Parse a PKCS#1 (ASN.1) encoded public RSA key.
  *
  * \param rsa       The RSA context where parsed data will be stored.
- * \param p         Beginning of the buffer containing the key to be parsed.
+ * \param key       The buffer that contains the key.
- *                  On successful return, the referenced pointer will be
+ * \param keylen    The length of the key buffer in bytes.
- *                  updated in order to point to the end of the parsed data.
- * \param end       End of the buffer containing the key to be parsed.
  *
  * \return          0 on success.
  * \return          MBEDTLS_ERR_ASN1_xxx in case of ASN.1 parsing errors.
  * \return          MBEDTLS_ERR_RSA_BAD_INPUT_DATA in case of importing or
  *                  priv/pub validation errors.
  */
-int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p,
+int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen);
-                             const unsigned char *end);
 
 /**
  * \brief           Write a PKCS#1 (ASN.1) encoded private RSA key.

tests/suites/test_suite_rsa.function

@@ -1376,13 +1376,11 @@ exit:
 void rsa_parse_pkcs1_key(int is_public, data_t *input, int exp_ret_val)
 {
     mbedtls_rsa_context rsa_ctx;
-    unsigned char *start = input->x;
-    unsigned char *end = input->x + input->len;
 
     mbedtls_rsa_init(&rsa_ctx);
 
     if (is_public) {
-        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &start, end), exp_ret_val);
+        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), exp_ret_val);
     } else {
         TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), exp_ret_val);
     }
@@ -1396,8 +1394,6 @@ exit:
 void rsa_parse_write_pkcs1_key(int is_public, data_t *input)
 {
     mbedtls_rsa_context rsa_ctx;
-    unsigned char *input_start = input->x;
-    unsigned char *input_end = input->x + input->len;
     unsigned char *output_buf = NULL;
     unsigned char *output_start;
     unsigned char *output_end;
@@ -1410,8 +1406,7 @@ void rsa_parse_write_pkcs1_key(int is_public, data_t *input)
 
     /* Parse the key and write it back to output_buf. */
     if (is_public) {
-        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &input_start, input_end), 0);
+        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), 0);
-        TEST_ASSERT(input_start == input_end);
         TEST_EQUAL(mbedtls_rsa_write_pubkey(&rsa_ctx, output_start, &output_end), input->len);
     } else {
         TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), 0);
@@ -1434,7 +1429,7 @@ exit:
 void rsa_key_write_incremental(int is_public, data_t *input)
 {
     mbedtls_rsa_context rsa_ctx;
-    unsigned char *buf = NULL, *start, *end;
+    unsigned char *buf = NULL, *end;
     size_t i;
 
     mbedtls_rsa_init(&rsa_ctx);
@@ -1442,9 +1437,7 @@ void rsa_key_write_incremental(int is_public, data_t *input)
     /* This is supposed to succeed as the real target of this test are the
      * write attempt below. */
     if (is_public) {
-        start = input->x;
+        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), 0);
-        end = input->x + input->len;
-        TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &start, end), 0);
     } else {
         TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), 0);
     }