x509: fix endianness and input data format for x509write_crt_set_serial_new

Signed-off-by: Valerio Setti <[email protected]>

include/mbedtls/x509_crt.h

@@ -1022,17 +1022,17 @@ int MBEDTLS_DEPRECATED mbedtls_x509write_crt_set_serial(
  * \brief           Set the serial number for a Certificate.
  *
  * \param ctx               CRT context to use
- * \param serial_buff       Input buffer containing the serial number in big
+ * \param serial_buff       A raw array of bytes containing the serial number
- *                          endian format
+ *                          in big endian format
  * \param serial_buff_len   Length of the previous input buffer buffer
  *
  * \return          0 if successful, or
- *                  MBEDTLS_ERR_X509_BAD_INPUT_DATA if the provided input buffer:
+ *                  MBEDTLS_ERR_X509_BAD_INPUT_DATA if the provided input buffer
- *                  - is too big (longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN)
+ *                  is too big (longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN)
- *                  - contains invalid chars
  */
 int mbedtls_x509write_crt_set_serial_new( mbedtls_x509write_cert *ctx,
-                                char* serial_buff, size_t serial_buff_len );
+                                        unsigned char* serial_buff,
+                                        size_t serial_buff_len );
 
 /**
  * \brief           Set the validity period for a Certificate

library/x509write_crt.c

@@ -120,40 +120,23 @@ int mbedtls_x509write_crt_set_serial( mbedtls_x509write_cert *ctx,
     if( ret < 0 )
         return ret;
     
-    /* Reverse the string since "tmp" is in big endian format */
+    /* Copy data to the internal structure skipping leading zeros */
-    for( int i=0; i<MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN; i++ )
+    memcpy(ctx->serial, &tmp[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN - tmp_len],
-        ctx->serial[i] = tmp[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN - 1 - i];
+            tmp_len);
 
     return( 0 );
 }
 #endif // MBEDTLS_BIGNUM_C && !MBEDTLS_DEPRECATED_REMOVED
 
 int mbedtls_x509write_crt_set_serial_new( mbedtls_x509write_cert *ctx,
-                                char* serial_buff, size_t serial_buff_len)
+                                        unsigned char* serial_buff,
+                                        size_t serial_buff_len)
 {
-    int i, j;
-    char c;
-    unsigned char val;
-
     if( serial_buff_len > MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN )
         return MBEDTLS_ERR_X509_BAD_INPUT_DATA;
 
-    /* Store data in little endian format */
+    ctx->serial_len = serial_buff_len;
-    for( i = 0, j = serial_buff_len - 1; j == 0; i++, j-- )
+    memcpy(ctx->serial, serial_buff, serial_buff_len);
-    {
-        c = serial_buff[j];
-        if( c >= 0x30 && c <= 0x39 )
-            val = c - 0x30;
-        else if( c >= 0x41 && c <= 0x46 )
-            val = c - 0x37;
-        else if( c >= 0x61 && c <= 0x66 )
-            val = c - 0x57;
-        else
-            return MBEDTLS_ERR_X509_BAD_INPUT_DATA;
-
-        ctx->serial[i] = val;
-    }
-    ctx->serial_len = i;
 
     return 0;
 }

programs/x509/cert_write.c

@@ -235,6 +235,41 @@ int write_certificate( mbedtls_x509write_cert *crt, const char *output_file,
     return( 0 );
 }
 
+/*
+ * Convert the input "in_buff" string to a raw byte array "out_buff". The amount
+ * of converted data is returned on "written_data".
+ */
+static int parse_serial( const char* in_buff, size_t in_buff_len,
+                    unsigned char* out_buff, size_t out_buff_len,
+                    size_t *written_data )
+{
+    char c;
+    unsigned char val;
+    int i;
+
+    if( out_buff_len < in_buff_len )
+        return( -1 );
+
+    *written_data = 0;
+
+    for( i = 0; i < (int) in_buff_len; i++, (*written_data)++ )
+    {
+        c = in_buff[i];
+        if( c >= 0x30 && c <= 0x39 )
+            val = c - 0x30;
+        else if( c >= 0x41 && c <= 0x46 )
+            val = c - 0x37;
+        else if( c >= 0x61 && c <= 0x66 )
+            val = c - 0x57;
+        else
+            return( -1 );
+
+        out_buff[i] = val;
+    }
+
+    return( 0 );
+}
+
 int main( int argc, char *argv[] )
 {
     int ret = 1;
@@ -252,7 +287,8 @@ int main( int argc, char *argv[] )
     mbedtls_x509_csr csr;
 #endif
     mbedtls_x509write_cert crt;
-    mbedtls_mpi serial;
+    unsigned char serial[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN];
+    size_t serial_len;
     mbedtls_asn1_sequence *ext_key_usage;
     mbedtls_entropy_context entropy;
     mbedtls_ctr_drbg_context ctr_drbg;
@@ -264,7 +300,6 @@ int main( int argc, char *argv[] )
     mbedtls_x509write_crt_init( &crt );
     mbedtls_pk_init( &loaded_issuer_key );
     mbedtls_pk_init( &loaded_subject_key );
-    mbedtls_mpi_init( &serial );
     mbedtls_ctr_drbg_init( &ctr_drbg );
     mbedtls_entropy_init( &entropy );
 #if defined(MBEDTLS_X509_CSR_PARSE_C)
@@ -559,16 +594,6 @@ int main( int argc, char *argv[] )
     mbedtls_printf( "  . Reading serial number..." );
     fflush( stdout );
 
-#if defined(MBEDTLS_BIGNUM_C)
-    if( ( ret = mbedtls_mpi_read_string( &serial, 10, opt.serial ) ) != 0 )
-    {
-        mbedtls_strerror( ret, buf, sizeof(buf) );
-        mbedtls_printf( " failed\n  !  mbedtls_mpi_read_string "
-                        "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf );
-        goto exit;
-    }
-#endif
-
     mbedtls_printf( " ok\n" );
 
     // Parse issuer certificate if present
@@ -723,18 +748,13 @@ int main( int argc, char *argv[] )
     mbedtls_x509write_crt_set_version( &crt, opt.version );
     mbedtls_x509write_crt_set_md_alg( &crt, opt.md );
 
-#if defined(MBEDTLS_BIGNUM_C) && !defined(MBEDTLS_DEPRECATED_REMOVED)
+    if( parse_serial( opt.serial, strlen( opt.serial ),
-    ret = mbedtls_x509write_crt_set_serial( &crt, &serial );
+                    serial, sizeof( serial ), &serial_len ) < 0 )
-    if( ret != 0 )
     {
-        mbedtls_strerror( ret, buf, sizeof(buf) );
+        mbedtls_printf( " failed\n  !  Unable to parse serial\n\n" );
-        mbedtls_printf( " failed\n  !  mbedtls_x509write_crt_set_serial "
-                        "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf );
         goto exit;
     }
-#else
+    ret = mbedtls_x509write_crt_set_serial_new( &crt, serial, serial_len );
-    ret = mbedtls_x509write_crt_set_serial_new( &crt, opt.serial,
-                                                strlen( opt.serial ) );
     if( ret != 0 )
     {
         mbedtls_strerror( ret, buf, sizeof(buf) );
@@ -742,7 +762,6 @@ int main( int argc, char *argv[] )
                         "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf );
         goto exit;
     }
-#endif
 
     ret = mbedtls_x509write_crt_set_validity( &crt, opt.not_before, opt.not_after );
     if( ret != 0 )
@@ -893,7 +912,6 @@ exit:
     mbedtls_x509write_crt_free( &crt );
     mbedtls_pk_free( &loaded_subject_key );
     mbedtls_pk_free( &loaded_issuer_key );
-    mbedtls_mpi_free( &serial );
     mbedtls_ctr_drbg_free( &ctr_drbg );
     mbedtls_entropy_free( &entropy );