Merge remote-tracking branch 'public/pr/2028' into development

* public/pr/2028:
  Update the crypto submodule to a78c958
  Fix ChangeLog entry to correct release version
  Fix typo in x509write test data
  Add ChangeLog entry for unused bits in bitstrings
  Improve docs for named bitstrings and their usage
  Add tests for (named) bitstring to suite_asn1write
  Add new function mbedtls_asn1_write_named_bitstring()

ChangeLog

@@ -7,6 +7,8 @@ Features
      which allows copy-less parsing of DER encoded X.509 CRTs,
      at the cost of additional lifetime constraints on the input
      buffer, but at the benefit of reduced RAM consumption.
+   * Add a new function mbedtls_asn1_write_named_bitstring() to write ASN.1
+     named bitstring in DER as required by RFC 5280 Appendix B.
 
 API Changes
    * Add a new X.509 API call `mbedtls_x509_parse_der_nocopy()`.
@@ -31,6 +33,12 @@ Bugfix
      Fixes #2190.
    * Fix false failure in all.sh when backup files exist in include/mbedtls
      (e.g. config.h.bak). Fixed by Peter Kolbus (Garmin) #2407.
+   * Ensure that unused bits are zero when writing ASN.1 bitstrings when using
+     mbedtls_asn1_write_bitstring().
+   * Fix issue when writing the named bitstrings in KeyUsage and NsCertType
+     extensions in CSRs and CRTs that caused these bitstrings to not be encoded
+     correctly as trailing zeroes were not accounted for as unused bits in the
+     leading content octet. Fixes #1610.
 
 Changes
    * Reduce RAM consumption during session renegotiation by not storing

include/mbedtls/asn1write.h

@@ -282,6 +282,28 @@ int mbedtls_asn1_write_ia5_string( unsigned char **p, unsigned char *start,
 int mbedtls_asn1_write_bitstring( unsigned char **p, unsigned char *start,
                                   const unsigned char *buf, size_t bits );
 
+/**
+ * \brief           This function writes a named bitstring tag
+ *                  (#MBEDTLS_ASN1_BIT_STRING) and value in ASN.1 format.
+ *
+ *                  As stated in RFC 5280 Appendix B, trailing zeroes are
+ *                  omitted when encoding named bitstrings in DER.
+ *
+ * \note            This function works backwards within the data buffer.
+ *
+ * \param p         The reference to the current position pointer.
+ * \param start     The start of the buffer which is used for bounds-checking.
+ * \param buf       The bitstring to write.
+ * \param bits      The total number of bits in the bitstring.
+ *
+ * \return          The number of bytes written to \p p on success.
+ * \return          A negative error code on failure.
+ */
+int mbedtls_asn1_write_named_bitstring( unsigned char **p,
+                                        unsigned char *start,
+                                        const unsigned char *buf,
+                                        size_t bits );
+
 /**
  * \brief           Write an octet string tag (#MBEDTLS_ASN1_OCTET_STRING)
  *                  and value in ASN.1 format.

include/mbedtls/x509_csr.h

@@ -205,6 +205,14 @@ void mbedtls_x509write_csr_set_md_alg( mbedtls_x509write_csr *ctx, mbedtls_md_ty
  * \param key_usage key usage flags to set
  *
  * \return          0 if successful, or MBEDTLS_ERR_X509_ALLOC_FAILED
+ *
+ * \note            The <code>decipherOnly</code> flag from the Key Usage
+ *                  extension is represented by bit 8 (i.e.
+ *                  <code>0x8000</code>), which cannot typically be represented
+ *                  in an unsigned char. Therefore, the flag
+ *                  <code>decipherOnly</code> (i.e.
+ *                  #MBEDTLS_X509_KU_DECIPHER_ONLY) cannot be set using this
+ *                  function.
  */
 int mbedtls_x509write_csr_set_key_usage( mbedtls_x509write_csr *ctx, unsigned char key_usage );
 

library/asn1write.c

@@ -290,26 +290,75 @@ int mbedtls_asn1_write_ia5_string( unsigned char **p, unsigned char *start,
     return( mbedtls_asn1_write_tagged_string(p, start, MBEDTLS_ASN1_IA5_STRING, text, text_len) );
 }
 
+int mbedtls_asn1_write_named_bitstring( unsigned char **p,
+                                        unsigned char *start,
+                                        const unsigned char *buf,
+                                        size_t bits )
+{
+    size_t unused_bits, byte_len;
+    const unsigned char *cur_byte;
+    unsigned char cur_byte_shifted;
+    unsigned char bit;
+
+    byte_len = ( bits + 7 ) / 8;
+    unused_bits = ( byte_len * 8 ) - bits;
+
+    /*
+     * Named bitstrings require that trailing 0s are excluded in the encoding
+     * of the bitstring. Trailing 0s are considered part of the 'unused' bits
+     * when encoding this value in the first content octet
+     */
+    if( bits != 0 )
+    {
+        cur_byte = buf + byte_len - 1;
+        cur_byte_shifted = *cur_byte >> unused_bits;
+
+        for( ; ; )
+        {
+            bit = cur_byte_shifted & 0x1;
+            cur_byte_shifted >>= 1;
+
+            if( bit != 0 )
+                break;
+
+            bits--;
+            if( bits == 0 )
+                break;
+
+            if( bits % 8 == 0 )
+                cur_byte_shifted = *--cur_byte;
+        }
+    }
+
+    return( mbedtls_asn1_write_bitstring( p, start, buf, bits ) );
+}
+
 int mbedtls_asn1_write_bitstring( unsigned char **p, unsigned char *start,
                           const unsigned char *buf, size_t bits )
 {
     int ret;
-    size_t len = 0, size;
+    size_t len = 0;
+    size_t unused_bits, byte_len;
 
-    size = ( bits / 8 ) + ( ( bits % 8 ) ? 1 : 0 );
+    byte_len = ( bits + 7 ) / 8;
+    unused_bits = ( byte_len * 8 ) - bits;
 
-    // Calculate byte length
-    //
-    if( *p < start || (size_t)( *p - start ) < size + 1 )
+    if( *p < start || (size_t)( *p - start ) < byte_len + 1 )
         return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL );
 
-    len = size + 1;
-    (*p) -= size;
-    memcpy( *p, buf, size );
+    len = byte_len + 1;
 
-    // Write unused bits
-    //
-    *--(*p) = (unsigned char) (size * 8 - bits);
+    /* Write the bitstring. Ensure the unused bits are zeroed */
+    if( byte_len > 0 )
+    {
+        byte_len--;
+        *--( *p ) = buf[byte_len] & ~( ( 0x1 << unused_bits ) - 1 );
+        ( *p ) -= byte_len;
+        memcpy( *p, buf, byte_len );
+    }
+
+    /* Write unused bits */
+    *--( *p ) = (unsigned char)unused_bits;
 
     MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) );
     MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_BIT_STRING ) );

library/x509write_crt.c

@@ -221,23 +221,36 @@ int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert *
 int mbedtls_x509write_crt_set_key_usage( mbedtls_x509write_cert *ctx,
                                          unsigned int key_usage )
 {
-    unsigned char buf[4], ku;
+    unsigned char buf[5], ku[2];
     unsigned char *c;
     int ret;
-
-    /* We currently only support 7 bits, from 0x80 to 0x02 */
-    if( ( key_usage & ~0xfe ) != 0 )
+    const unsigned int allowed_bits = MBEDTLS_X509_KU_DIGITAL_SIGNATURE |
+        MBEDTLS_X509_KU_NON_REPUDIATION   |
+        MBEDTLS_X509_KU_KEY_ENCIPHERMENT  |
+        MBEDTLS_X509_KU_DATA_ENCIPHERMENT |
+        MBEDTLS_X509_KU_KEY_AGREEMENT     |
+        MBEDTLS_X509_KU_KEY_CERT_SIGN     |
+        MBEDTLS_X509_KU_CRL_SIGN          |
+        MBEDTLS_X509_KU_ENCIPHER_ONLY     |
+        MBEDTLS_X509_KU_DECIPHER_ONLY;
+
+    /* Check that nothing other than the allowed flags is set */
+    if( ( key_usage & ~allowed_bits ) != 0 )
         return( MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE );
 
-    c = buf + 4;
-    ku = (unsigned char) key_usage;
+    c = buf + 5;
+    ku[0] = (unsigned char)( key_usage      );
+    ku[1] = (unsigned char)( key_usage >> 8 );
+    ret = mbedtls_asn1_write_named_bitstring( &c, buf, ku, 9 );
 
-    if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ku, 7 ) ) != 4 )
+    if( ret < 0 )
         return( ret );
+    else if( ret < 3 || ret > 5 )
+        return( MBEDTLS_ERR_X509_INVALID_FORMAT );
 
     ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_KEY_USAGE,
                                        MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ),
-                                       1, buf, 4 );
+                                       1, c, (size_t)ret );
     if( ret != 0 )
         return( ret );
 
@@ -253,12 +266,13 @@ int mbedtls_x509write_crt_set_ns_cert_type( mbedtls_x509write_cert *ctx,
 
     c = buf + 4;
 
-    if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 )
+    ret = mbedtls_asn1_write_named_bitstring( &c, buf, &ns_cert_type, 8 );
+    if( ret < 3 || ret > 4 )
         return( ret );
 
     ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE,
                                        MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ),
-                                       0, buf, 4 );
+                                       0, c, (size_t)ret );
     if( ret != 0 )
         return( ret );
 

library/x509write_csr.c

@@ -94,12 +94,13 @@ int mbedtls_x509write_csr_set_key_usage( mbedtls_x509write_csr *ctx, unsigned ch
 
     c = buf + 4;
 
-    if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &key_usage, 7 ) ) != 4 )
+    ret = mbedtls_asn1_write_named_bitstring( &c, buf, &key_usage, 8 );
+    if( ret < 3 || ret > 4 )
         return( ret );
 
     ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_KEY_USAGE,
                                        MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ),
-                                       buf, 4 );
+                                       c, (size_t)ret );
     if( ret != 0 )
         return( ret );
 
@@ -115,12 +116,13 @@ int mbedtls_x509write_csr_set_ns_cert_type( mbedtls_x509write_csr *ctx,
 
     c = buf + 4;
 
-    if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 )
+    ret = mbedtls_asn1_write_named_bitstring( &c, buf, &ns_cert_type, 8 );
+    if( ret < 3 || ret > 4 )
         return( ret );
 
     ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE,
                                        MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ),
-                                       buf, 4 );
+                                       c, (size_t)ret );
     if( ret != 0 )
         return( ret );
 

programs/x509/cert_req.c

@@ -65,7 +65,9 @@ int main( void )
 #define DFL_OUTPUT_FILENAME     "cert.req"
 #define DFL_SUBJECT_NAME        "CN=Cert,O=mbed TLS,C=UK"
 #define DFL_KEY_USAGE           0
+#define DFL_FORCE_KEY_USAGE     0
 #define DFL_NS_CERT_TYPE        0
+#define DFL_FORCE_NS_CERT_TYPE  0
 #define DFL_MD_ALG              MBEDTLS_MD_SHA256
 
 #define USAGE \
@@ -85,6 +87,8 @@ int main( void )
     "                          key_agreement\n"         \
     "                          key_cert_sign\n"  \
     "                          crl_sign\n"              \
+    "    force_key_usage=0/1  default: off\n"           \
+    "                          Add KeyUsage even if it is empty\n"  \
     "    ns_cert_type=%%s     default: (empty)\n"       \
     "                        Comma-separated-list of values:\n"     \
     "                          ssl_client\n"            \
@@ -94,6 +98,8 @@ int main( void )
     "                          ssl_ca\n"                \
     "                          email_ca\n"              \
     "                          object_signing_ca\n"     \
+    "    force_ns_cert_type=0/1 default: off\n"         \
+    "                          Add NsCertType even if it is empty\n"    \
     "    md=%%s               default: SHA256\n"       \
     "                          possible values:\n"     \
     "                          MD4, MD5, SHA1\n"       \
@@ -123,7 +129,9 @@ struct options
     const char *output_file;    /* where to store the constructed key file  */
     const char *subject_name;   /* subject name for certificate request */
     unsigned char key_usage;    /* key usage flags                      */
+    int force_key_usage;        /* Force adding the KeyUsage extension  */
     unsigned char ns_cert_type; /* NS cert type                         */
+    int force_ns_cert_type;     /* Force adding NsCertType extension    */
     mbedtls_md_type_t md_alg;   /* Hash algorithm used for signature.   */
 } opt;
 
@@ -190,7 +198,9 @@ int main( int argc, char *argv[] )
     opt.output_file         = DFL_OUTPUT_FILENAME;
     opt.subject_name        = DFL_SUBJECT_NAME;
     opt.key_usage           = DFL_KEY_USAGE;
+    opt.force_key_usage     = DFL_FORCE_KEY_USAGE;
     opt.ns_cert_type        = DFL_NS_CERT_TYPE;
+    opt.force_ns_cert_type  = DFL_FORCE_NS_CERT_TYPE;
     opt.md_alg              = DFL_MD_ALG;
 
     for( i = 1; i < argc; i++ )
@@ -292,6 +302,15 @@ int main( int argc, char *argv[] )
                 q = r;
             }
         }
+        else if( strcmp( p, "force_key_usage" ) == 0 )
+        {
+            switch( atoi( q ) )
+            {
+                case 0: opt.force_key_usage = 0; break;
+                case 1: opt.force_key_usage = 1; break;
+                default: goto usage;
+            }
+        }
         else if( strcmp( p, "ns_cert_type" ) == 0 )
         {
             while( q != NULL )
@@ -319,16 +338,25 @@ int main( int argc, char *argv[] )
                 q = r;
             }
         }
+        else if( strcmp( p, "force_ns_cert_type" ) == 0 )
+        {
+            switch( atoi( q ) )
+            {
+                case 0: opt.force_ns_cert_type = 0; break;
+                case 1: opt.force_ns_cert_type = 1; break;
+                default: goto usage;
+            }
+        }
         else
             goto usage;
     }
 
     mbedtls_x509write_csr_set_md_alg( &req, opt.md_alg );
 
-    if( opt.key_usage )
+    if( opt.key_usage || opt.force_key_usage == 1 )
         mbedtls_x509write_csr_set_key_usage( &req, opt.key_usage );
 
-    if( opt.ns_cert_type )
+    if( opt.ns_cert_type || opt.force_ns_cert_type == 1 )
         mbedtls_x509write_csr_set_ns_cert_type( &req, opt.ns_cert_type );
 
     /*

tests/data_files/Makefile

@@ -785,6 +785,14 @@ server1.req.ku-ct: server1.key
 	$(MBEDTLS_CERT_REQ) output_file=$@ filename=$< key_usage=digital_signature,non_repudiation,key_encipherment ns_cert_type=ssl_server subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1
 all_final += server1.req.ku-ct
 
+server1.req.key_usage_empty: server1.key
+	$(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 force_key_usage=1
+all_final += server1.req.key_usage_empty
+
+server1.req.cert_type_empty: server1.key
+	$(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 force_ns_cert_type=1
+all_final += server1.req.cert_type_empty
+
 # server2*
 
 server2.req.sha256: server2.key

tests/data_files/server1.cert_type.crt

@@ -11,10 +11,10 @@ lZvc/kFeF6babFtpzAK6FCwWJJxK3M3Q91Jnc/EtoCP9fvQxyi1wyokLBNsupk9w
 bp7OvViJ4lNZnm5akmXiiD8MlBmj3eXonZUT7Snbq3AS3FrKaxerUoJUsQIDAQAB
 o2AwXjAJBgNVHRMEAjAAMB0GA1UdDgQWBBQfdNY/KcF0dEU7BRIsPai9Q1kCpjAf
 BgNVHSMEGDAWgBS0WuSls97SUva51aaVD+s+vMf9/zARBglghkgBhvhCAQEEBAMC
-AEAwDQYJKoZIhvcNAQEFBQADggEBAEQOk5Ejgu/GsxvMo+RknXcta5Qr6MiNo1EM
-G5Xrf++aaf4Mi38p5ZxWDxQDyBmutSnuJgzO+Dxe5w/RNojFa4ri4g5Zk8zwfIcQ
-8jR6a9DJtxarxDj/UqEzaiBa5MpxsbQqbmou7X7YW9LHDzmCgzbaabyWCuGYxvmh
-lDbcISST73G+vJEeExcBHyom/GV9TNcFAGa66YV/FtABg2tiy9znmUeMnZeYkC9S
-05m6UstAU6pMdwiTpjZjovsTlAcmC76XmE/GpREhRvtGCKTb2pUi3agqsrapABmF
-EGZT9cpwkrl3cxh+jxAMEuhJLdEScDWHVsiNS5y9yxitWC4NqR4=
+BkAwDQYJKoZIhvcNAQEFBQADggEBAK1WXZYd6k7/zE2NcszT6rxNaSixPZrDYzRt
+Iz5rpH33IHkCdR956/ExCcDMqGNVtKtBdr8kw3+jzyPQhwyHVPNv4C/cgt0C89Pf
+qZLQGuEPVp1X4tzEY2Kno9c1tllLVzJdvz1mRhSb9z5CWQKNMT+8MMl3k+0NZ4LT
+NEx4gTZxYEsAGEuO/Yij9ctxp4RdSP585FXgiMC00ieMe/aJxlOIgpIhuWdu0KPP
+G5guYd4hQ9ZrGVOGdjv2cZbh4DuQOsCwU9in/e1RKFV6eMmyOdvLJ4jkTauwkGJG
+lCclZZQwzGawOiMl2OYPUia5bkaEsdE/0QW/lf36lco8CNjpUfY=
 -----END CERTIFICATE-----

tests/data_files/server1.cert_type_noauthid.crt

@@ -10,11 +10,11 @@ CrFTxjB+FTms+Vruf5KepgVb5xOXhbUjktnUJAbVCSWJdQfdphqPPwkZvq1lLGTr
 lZvc/kFeF6babFtpzAK6FCwWJJxK3M3Q91Jnc/EtoCP9fvQxyi1wyokLBNsupk9w
 bp7OvViJ4lNZnm5akmXiiD8MlBmj3eXonZUT7Snbq3AS3FrKaxerUoJUsQIDAQAB
 oz8wPTAJBgNVHRMEAjAAMB0GA1UdDgQWBBQfdNY/KcF0dEU7BRIsPai9Q1kCpjAR
-BglghkgBhvhCAQEEBAMCAEAwDQYJKoZIhvcNAQEFBQADggEBAJc3yZUS9X3/lb63
-Nlt8rtXC45wbWZUoOK8N55IzEJC7FrttAStq24kq9QV0qiox8m1WLA+6xVaeZaXu
-h2z3WlUlyCNaKqHEpuSYu/XQ0td6j3jCMj3VDSZGHnKgliQ9fkkt+waPVCAZldwj
-rHsZibl2Dqzb3KttKqD1VyEVOUJ+saXRDJLFdK1M9nwdWMfOg/XE0WbqfVzw9COs
-08dJ6KL7SOvXtiOVQLNv7XN/2j+wF6+IoLDdLCDByj5VtK2q2vyVk5tpDJI1S696
-dP8Zi7VbBTS9OlVC+Gw3CntDKZA8e215MNG6iBuEM2mgi4i0umo7mN8FoA1zusnE
-8mCO55Q=
+BglghkgBhvhCAQEEBAMCBkAwDQYJKoZIhvcNAQEFBQADggEBAGl6bYCGKvDCvfSU
+PTyaiFPNGXV98AnIG0Hu4EJjs1owBU/Yf8UdFbWJtOymR80SbzmeQ6rEIoY1oXDA
+o9Y8yRgW8t25Wmq/0DCu/5P0/L6asstLTxLG4qajClVwqDLEqZNixmq8QorAOtK1
+JngFA+A5jzc70Ikl9+Hbx/2SEMrCpo0QLSco7KDK7XpNOHbkRz2AqSm0se4jDMP1
+Cwd2UtcpctIZEbECZo6S9WrVMqIhRF1Y5FeauBA2ORvGIHohaYJ9VzYWYXIp7N8d
+QXGv+M7ffpZiERcRr8lxtboPnTXKlv1mLCEX7g+KuiJQUm4OGfTCd5VHzWM7O5Id
+b+IvZD0=
 -----END CERTIFICATE-----