Skip to content

Commit a78c958

Browse files
PataterPatater
committed
Merge remote-tracking branch 'tls/pr/2028' into development
2 parents a9d6ba2 + e254f85 commit a78c958

22 files changed

+404
-103
lines changed

‎ChangeLog

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ Changes
5353
underlying OS actually guarantees.
5454
* Fix configuration queries in ssl-opt.h. #2030
5555
* Ensure that ssl-opt.h can be run in OS X. #2029
56+
* Ensure that unused bits are zero when writing ASN.1 bitstrings when using
57+
mbedtls_asn1_write_bitstring().
58+
* Fix issue when writing the named bitstrings in KeyUsage and NsCertType
59+
extensions in CSRs and CRTs that caused these bitstrings to not be encoded
60+
correctly as trailing zeroes were not accounted for as unused bits in the
61+
leading content octet. Fixes #1610.
62+
63+
Features
64+
* Add a new function mbedtls_asn1_write_named_bitstring() to write ASN.1
65+
named bitstring in DER as required by RFC 5280 Appendix B.
5666

5767
= mbed TLS 2.16.0 branch released 2018-12-21
5868

‎include/mbedtls/asn1write.h

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,28 @@ int mbedtls_asn1_write_ia5_string( unsigned char **p, unsigned char *start,
282282
int mbedtls_asn1_write_bitstring( unsigned char **p, unsigned char *start,
283283
const unsigned char *buf, size_t bits );
284284

285+
/**
286+
* \brief This function writes a named bitstring tag
287+
* (#MBEDTLS_ASN1_BIT_STRING) and value in ASN.1 format.
288+
*
289+
* As stated in RFC 5280 Appendix B, trailing zeroes are
290+
* omitted when encoding named bitstrings in DER.
291+
*
292+
* \note This function works backwards within the data buffer.
293+
*
294+
* \param p The reference to the current position pointer.
295+
* \param start The start of the buffer which is used for bounds-checking.
296+
* \param buf The bitstring to write.
297+
* \param bits The total number of bits in the bitstring.
298+
*
299+
* \return The number of bytes written to \p p on success.
300+
* \return A negative error code on failure.
301+
*/
302+
int mbedtls_asn1_write_named_bitstring( unsigned char **p,
303+
unsigned char *start,
304+
const unsigned char *buf,
305+
size_t bits );
306+
285307
/**
286308
* \brief Write an octet string tag (#MBEDTLS_ASN1_OCTET_STRING)
287309
* and value in ASN.1 format.

‎include/mbedtls/x509_csr.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,14 @@ void mbedtls_x509write_csr_set_md_alg( mbedtls_x509write_csr *ctx, mbedtls_md_ty
205205
* \param key_usage key usage flags to set
206206
*
207207
* \return 0 if successful, or MBEDTLS_ERR_X509_ALLOC_FAILED
208+
*
209+
* \note The <code>decipherOnly</code> flag from the Key Usage
210+
* extension is represented by bit 8 (i.e.
211+
* <code>0x8000</code>), which cannot typically be represented
212+
* in an unsigned char. Therefore, the flag
213+
* <code>decipherOnly</code> (i.e.
214+
* #MBEDTLS_X509_KU_DECIPHER_ONLY) cannot be set using this
215+
* function.
208216
*/
209217
int mbedtls_x509write_csr_set_key_usage( mbedtls_x509write_csr *ctx, unsigned char key_usage );
210218

‎library/asn1write.c

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -290,26 +290,75 @@ int mbedtls_asn1_write_ia5_string( unsigned char **p, unsigned char *start,
290290
return( mbedtls_asn1_write_tagged_string(p, start, MBEDTLS_ASN1_IA5_STRING, text, text_len) );
291291
}
292292

293+
int mbedtls_asn1_write_named_bitstring( unsigned char **p,
294+
unsigned char *start,
295+
const unsigned char *buf,
296+
size_t bits )
297+
{
298+
size_t unused_bits, byte_len;
299+
const unsigned char *cur_byte;
300+
unsigned char cur_byte_shifted;
301+
unsigned char bit;
302+
303+
byte_len = ( bits + 7 ) / 8;
304+
unused_bits = ( byte_len * 8 ) - bits;
305+
306+
/*
307+
* Named bitstrings require that trailing 0s are excluded in the encoding
308+
* of the bitstring. Trailing 0s are considered part of the 'unused' bits
309+
* when encoding this value in the first content octet
310+
*/
311+
if( bits != 0 )
312+
{
313+
cur_byte = buf + byte_len - 1;
314+
cur_byte_shifted = *cur_byte >> unused_bits;
315+
316+
for( ; ; )
317+
{
318+
bit = cur_byte_shifted & 0x1;
319+
cur_byte_shifted >>= 1;
320+
321+
if( bit != 0 )
322+
break;
323+
324+
bits--;
325+
if( bits == 0 )
326+
break;
327+
328+
if( bits % 8 == 0 )
329+
cur_byte_shifted = *--cur_byte;
330+
}
331+
}
332+
333+
return( mbedtls_asn1_write_bitstring( p, start, buf, bits ) );
334+
}
335+
293336
int mbedtls_asn1_write_bitstring( unsigned char **p, unsigned char *start,
294337
const unsigned char *buf, size_t bits )
295338
{
296339
int ret;
297-
size_t len = 0, size;
340+
size_t len = 0;
341+
size_t unused_bits, byte_len;
298342

299-
size = ( bits / 8 ) + ( ( bits % 8 ) ? 1 : 0 );
343+
byte_len = ( bits + 7 ) / 8;
344+
unused_bits = ( byte_len * 8 ) - bits;
300345

301-
// Calculate byte length
302-
//
303-
if( *p < start || (size_t)( *p - start ) < size + 1 )
346+
if( *p < start || (size_t)( *p - start ) < byte_len + 1 )
304347
return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL );
305348

306-
len = size + 1;
307-
(*p) -= size;
308-
memcpy( *p, buf, size );
349+
len = byte_len + 1;
309350

310-
// Write unused bits
311-
//
312-
*--(*p) = (unsigned char) (size * 8 - bits);
351+
/* Write the bitstring. Ensure the unused bits are zeroed */
352+
if( byte_len > 0 )
353+
{
354+
byte_len--;
355+
*--( *p ) = buf[byte_len] & ~( ( 0x1 << unused_bits ) - 1 );
356+
( *p ) -= byte_len;
357+
memcpy( *p, buf, byte_len );
358+
}
359+
360+
/* Write unused bits */
361+
*--( *p ) = (unsigned char)unused_bits;
313362

314363
MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) );
315364
MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_BIT_STRING ) );

‎library/x509write_crt.c

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,23 +221,36 @@ int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert *
221221
int mbedtls_x509write_crt_set_key_usage( mbedtls_x509write_cert *ctx,
222222
unsigned int key_usage )
223223
{
224-
unsigned char buf[4], ku;
224+
unsigned char buf[5], ku[2];
225225
unsigned char *c;
226226
int ret;
227-
228-
/* We currently only support 7 bits, from 0x80 to 0x02 */
229-
if( ( key_usage & ~0xfe ) != 0 )
227+
const unsigned int allowed_bits = MBEDTLS_X509_KU_DIGITAL_SIGNATURE |
228+
MBEDTLS_X509_KU_NON_REPUDIATION |
229+
MBEDTLS_X509_KU_KEY_ENCIPHERMENT |
230+
MBEDTLS_X509_KU_DATA_ENCIPHERMENT |
231+
MBEDTLS_X509_KU_KEY_AGREEMENT |
232+
MBEDTLS_X509_KU_KEY_CERT_SIGN |
233+
MBEDTLS_X509_KU_CRL_SIGN |
234+
MBEDTLS_X509_KU_ENCIPHER_ONLY |
235+
MBEDTLS_X509_KU_DECIPHER_ONLY;
236+
237+
/* Check that nothing other than the allowed flags is set */
238+
if( ( key_usage & ~allowed_bits ) != 0 )
230239
return( MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE );
231240

232-
c = buf + 4;
233-
ku = (unsigned char) key_usage;
241+
c = buf + 5;
242+
ku[0] = (unsigned char)( key_usage );
243+
ku[1] = (unsigned char)( key_usage >> 8 );
244+
ret = mbedtls_asn1_write_named_bitstring( &c, buf, ku, 9 );
234245

235-
if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ku, 7 ) ) != 4 )
246+
if( ret < 0 )
236247
return( ret );
248+
else if( ret < 3 || ret > 5 )
249+
return( MBEDTLS_ERR_X509_INVALID_FORMAT );
237250

238251
ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_KEY_USAGE,
239252
MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ),
240-
1, buf, 4 );
253+
1, c, (size_t)ret );
241254
if( ret != 0 )
242255
return( ret );
243256

@@ -253,12 +266,13 @@ int mbedtls_x509write_crt_set_ns_cert_type( mbedtls_x509write_cert *ctx,
253266

254267
c = buf + 4;
255268

256-
if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 )
269+
ret = mbedtls_asn1_write_named_bitstring( &c, buf, &ns_cert_type, 8 );
270+
if( ret < 3 || ret > 4 )
257271
return( ret );
258272

259273
ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE,
260274
MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ),
261-
0, buf, 4 );
275+
0, c, (size_t)ret );
262276
if( ret != 0 )
263277
return( ret );
264278

‎library/x509write_csr.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,13 @@ int mbedtls_x509write_csr_set_key_usage( mbedtls_x509write_csr *ctx, unsigned ch
9494

9595
c = buf + 4;
9696

97-
if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &key_usage, 7 ) ) != 4 )
97+
ret = mbedtls_asn1_write_named_bitstring( &c, buf, &key_usage, 8 );
98+
if( ret < 3 || ret > 4 )
9899
return( ret );
99100

100101
ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_KEY_USAGE,
101102
MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ),
102-
buf, 4 );
103+
c, (size_t)ret );
103104
if( ret != 0 )
104105
return( ret );
105106

@@ -115,12 +116,13 @@ int mbedtls_x509write_csr_set_ns_cert_type( mbedtls_x509write_csr *ctx,
115116

116117
c = buf + 4;
117118

118-
if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 )
119+
ret = mbedtls_asn1_write_named_bitstring( &c, buf, &ns_cert_type, 8 );
120+
if( ret < 3 || ret > 4 )
119121
return( ret );
120122

121123
ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE,
122124
MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ),
123-
buf, 4 );
125+
c, (size_t)ret );
124126
if( ret != 0 )
125127
return( ret );
126128

‎programs/x509/cert_req.c

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ int main( void )
6565
#define DFL_OUTPUT_FILENAME "cert.req"
6666
#define DFL_SUBJECT_NAME "CN=Cert,O=mbed TLS,C=UK"
6767
#define DFL_KEY_USAGE 0
68+
#define DFL_FORCE_KEY_USAGE 0
6869
#define DFL_NS_CERT_TYPE 0
70+
#define DFL_FORCE_NS_CERT_TYPE 0
6971
#define DFL_MD_ALG MBEDTLS_MD_SHA256
7072

7173
#define USAGE \
@@ -85,6 +87,8 @@ int main( void )
8587
" key_agreement\n" \
8688
" key_cert_sign\n" \
8789
" crl_sign\n" \
90+
" force_key_usage=0/1 default: off\n" \
91+
" Add KeyUsage even if it is empty\n" \
8892
" ns_cert_type=%%s default: (empty)\n" \
8993
" Comma-separated-list of values:\n" \
9094
" ssl_client\n" \
@@ -94,6 +98,8 @@ int main( void )
9498
" ssl_ca\n" \
9599
" email_ca\n" \
96100
" object_signing_ca\n" \
101+
" force_ns_cert_type=0/1 default: off\n" \
102+
" Add NsCertType even if it is empty\n" \
97103
" md=%%s default: SHA256\n" \
98104
" possible values:\n" \
99105
" MD4, MD5, SHA1\n" \
@@ -123,7 +129,9 @@ struct options
123129
const char *output_file; /* where to store the constructed key file */
124130
const char *subject_name; /* subject name for certificate request */
125131
unsigned char key_usage; /* key usage flags */
132+
int force_key_usage; /* Force adding the KeyUsage extension */
126133
unsigned char ns_cert_type; /* NS cert type */
134+
int force_ns_cert_type; /* Force adding NsCertType extension */
127135
mbedtls_md_type_t md_alg; /* Hash algorithm used for signature. */
128136
} opt;
129137

@@ -190,7 +198,9 @@ int main( int argc, char *argv[] )
190198
opt.output_file = DFL_OUTPUT_FILENAME;
191199
opt.subject_name = DFL_SUBJECT_NAME;
192200
opt.key_usage = DFL_KEY_USAGE;
201+
opt.force_key_usage = DFL_FORCE_KEY_USAGE;
193202
opt.ns_cert_type = DFL_NS_CERT_TYPE;
203+
opt.force_ns_cert_type = DFL_FORCE_NS_CERT_TYPE;
194204
opt.md_alg = DFL_MD_ALG;
195205

196206
for( i = 1; i < argc; i++ )
@@ -292,6 +302,15 @@ int main( int argc, char *argv[] )
292302
q = r;
293303
}
294304
}
305+
else if( strcmp( p, "force_key_usage" ) == 0 )
306+
{
307+
switch( atoi( q ) )
308+
{
309+
case 0: opt.force_key_usage = 0; break;
310+
case 1: opt.force_key_usage = 1; break;
311+
default: goto usage;
312+
}
313+
}
295314
else if( strcmp( p, "ns_cert_type" ) == 0 )
296315
{
297316
while( q != NULL )
@@ -319,16 +338,25 @@ int main( int argc, char *argv[] )
319338
q = r;
320339
}
321340
}
341+
else if( strcmp( p, "force_ns_cert_type" ) == 0 )
342+
{
343+
switch( atoi( q ) )
344+
{
345+
case 0: opt.force_ns_cert_type = 0; break;
346+
case 1: opt.force_ns_cert_type = 1; break;
347+
default: goto usage;
348+
}
349+
}
322350
else
323351
goto usage;
324352
}
325353

326354
mbedtls_x509write_csr_set_md_alg( &req, opt.md_alg );
327355

328-
if( opt.key_usage )
356+
if( opt.key_usage || opt.force_key_usage == 1 )
329357
mbedtls_x509write_csr_set_key_usage( &req, opt.key_usage );
330358

331-
if( opt.ns_cert_type )
359+
if( opt.ns_cert_type || opt.force_ns_cert_type == 1 )
332360
mbedtls_x509write_csr_set_ns_cert_type( &req, opt.ns_cert_type );
333361

334362
/*

‎tests/data_files/Makefile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,14 @@ server1.req.ku-ct: server1.key
785785
$(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
786786
all_final += server1.req.ku-ct
787787

788+
server1.req.key_usage_empty: server1.key
789+
$(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 force_key_usage=1
790+
all_final += server1.req.key_usage_empty
791+
792+
server1.req.cert_type_empty: server1.key
793+
$(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 force_ns_cert_type=1
794+
all_final += server1.req.cert_type_empty
795+
788796
# server2*
789797

790798
server2.req.sha256: server2.key

‎tests/data_files/server1.cert_type.crt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,10 @@ lZvc/kFeF6babFtpzAK6FCwWJJxK3M3Q91Jnc/EtoCP9fvQxyi1wyokLBNsupk9w
1111
bp7OvViJ4lNZnm5akmXiiD8MlBmj3eXonZUT7Snbq3AS3FrKaxerUoJUsQIDAQAB
1212
o2AwXjAJBgNVHRMEAjAAMB0GA1UdDgQWBBQfdNY/KcF0dEU7BRIsPai9Q1kCpjAf
1313
BgNVHSMEGDAWgBS0WuSls97SUva51aaVD+s+vMf9/zARBglghkgBhvhCAQEEBAMC
14-
AEAwDQYJKoZIhvcNAQEFBQADggEBAEQOk5Ejgu/GsxvMo+RknXcta5Qr6MiNo1EM
15-
G5Xrf++aaf4Mi38p5ZxWDxQDyBmutSnuJgzO+Dxe5w/RNojFa4ri4g5Zk8zwfIcQ
16-
8jR6a9DJtxarxDj/UqEzaiBa5MpxsbQqbmou7X7YW9LHDzmCgzbaabyWCuGYxvmh
17-
lDbcISST73G+vJEeExcBHyom/GV9TNcFAGa66YV/FtABg2tiy9znmUeMnZeYkC9S
18-
05m6UstAU6pMdwiTpjZjovsTlAcmC76XmE/GpREhRvtGCKTb2pUi3agqsrapABmF
19-
EGZT9cpwkrl3cxh+jxAMEuhJLdEScDWHVsiNS5y9yxitWC4NqR4=
14+
BkAwDQYJKoZIhvcNAQEFBQADggEBAK1WXZYd6k7/zE2NcszT6rxNaSixPZrDYzRt
15+
Iz5rpH33IHkCdR956/ExCcDMqGNVtKtBdr8kw3+jzyPQhwyHVPNv4C/cgt0C89Pf
16+
qZLQGuEPVp1X4tzEY2Kno9c1tllLVzJdvz1mRhSb9z5CWQKNMT+8MMl3k+0NZ4LT
17+
NEx4gTZxYEsAGEuO/Yij9ctxp4RdSP585FXgiMC00ieMe/aJxlOIgpIhuWdu0KPP
18+
G5guYd4hQ9ZrGVOGdjv2cZbh4DuQOsCwU9in/e1RKFV6eMmyOdvLJ4jkTauwkGJG
19+
lCclZZQwzGawOiMl2OYPUia5bkaEsdE/0QW/lf36lco8CNjpUfY=
2020
-----END CERTIFICATE-----

‎tests/data_files/server1.cert_type_noauthid.crt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ CrFTxjB+FTms+Vruf5KepgVb5xOXhbUjktnUJAbVCSWJdQfdphqPPwkZvq1lLGTr
1010
lZvc/kFeF6babFtpzAK6FCwWJJxK3M3Q91Jnc/EtoCP9fvQxyi1wyokLBNsupk9w
1111
bp7OvViJ4lNZnm5akmXiiD8MlBmj3eXonZUT7Snbq3AS3FrKaxerUoJUsQIDAQAB
1212
oz8wPTAJBgNVHRMEAjAAMB0GA1UdDgQWBBQfdNY/KcF0dEU7BRIsPai9Q1kCpjAR
13-
BglghkgBhvhCAQEEBAMCAEAwDQYJKoZIhvcNAQEFBQADggEBAJc3yZUS9X3/lb63
14-
Nlt8rtXC45wbWZUoOK8N55IzEJC7FrttAStq24kq9QV0qiox8m1WLA+6xVaeZaXu
15-
h2z3WlUlyCNaKqHEpuSYu/XQ0td6j3jCMj3VDSZGHnKgliQ9fkkt+waPVCAZldwj
16-
rHsZibl2Dqzb3KttKqD1VyEVOUJ+saXRDJLFdK1M9nwdWMfOg/XE0WbqfVzw9COs
17-
08dJ6KL7SOvXtiOVQLNv7XN/2j+wF6+IoLDdLCDByj5VtK2q2vyVk5tpDJI1S696
18-
dP8Zi7VbBTS9OlVC+Gw3CntDKZA8e215MNG6iBuEM2mgi4i0umo7mN8FoA1zusnE
19-
8mCO55Q=
13+
BglghkgBhvhCAQEEBAMCBkAwDQYJKoZIhvcNAQEFBQADggEBAGl6bYCGKvDCvfSU
14+
PTyaiFPNGXV98AnIG0Hu4EJjs1owBU/Yf8UdFbWJtOymR80SbzmeQ6rEIoY1oXDA
15+
o9Y8yRgW8t25Wmq/0DCu/5P0/L6asstLTxLG4qajClVwqDLEqZNixmq8QorAOtK1
16+
JngFA+A5jzc70Ikl9+Hbx/2SEMrCpo0QLSco7KDK7XpNOHbkRz2AqSm0se4jDMP1
17+
Cwd2UtcpctIZEbECZo6S9WrVMqIhRF1Y5FeauBA2ORvGIHohaYJ9VzYWYXIp7N8d
18+
QXGv+M7ffpZiERcRr8lxtboPnTXKlv1mLCEX7g+KuiJQUm4OGfTCd5VHzWM7O5Id
19+
b+IvZD0=
2020
-----END CERTIFICATE-----

0 commit comments

Comments
 (0)