From 7512bf7d6398525b74e248c9a19bd17b1b9c600a Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:12:11 +0100 Subject: [PATCH 1/6] Add macros to ASN.1 module to parse ASN.1 tags The macros simply extract the component bits of an ASN.1 tag value --- include/mbedtls/asn1.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index e159e57ea..8d35c4245 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -87,6 +87,22 @@ #define MBEDTLS_ASN1_PRIMITIVE 0x00 #define MBEDTLS_ASN1_CONSTRUCTED 0x20 #define MBEDTLS_ASN1_CONTEXT_SPECIFIC 0x80 + +/* + * Bit masks for each of the components of an ASN.1 tag as specified in + * Information technnology - ASN.1 encoding rules: Specification of Basic + * Encoding Rules (BER), Canonical Encoding Rules (CER), and Distinguished + * encoding rules (DER) Section 8.1.2.2: + * + * Bit 8 7 6 5 1 + * +-------+-----+------------+ + * | Class | P/C | Tag number | + * +-------+-----+------------+ + */ +#define MBEDTLS_ASN1_TAG_CLASS_MASK ( 0x03 << 6 ) +#define MBEDTLS_ASN1_TAG_PC_MASK ( 0x01 << 5 ) +#define MBEDTLS_ASN1_TAG_VALUE_MASK ( 0x1F << 0 ) + /* \} name */ /* \} addtogroup asn1_module */ From 849bc65bbfcc8a06b17169274dbfb163de0d59f1 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:13:12 +0100 Subject: [PATCH 2/6] Fix x509_get_subject_alt_name to drop invalid tag Fix the x509_get_subject_alt_name() function to not accept invalid tags. The problem was that the ASN.1 class for tags consists of two bits. Simply doing bit-wise and of the CONTEXT_SPECIFIC macro with the input tag has the potential of accepting tag values 0x10 (private) which would indicate that the certificate has an incorrect format. --- library/x509_crt.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index c6209fb40..6d08d7795 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -472,9 +472,12 @@ static int x509_get_subject_alt_name( unsigned char **p, if( ( ret = mbedtls_asn1_get_len( p, end, &tag_len ) ) != 0 ) return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + ret ); - if( ( tag & MBEDTLS_ASN1_CONTEXT_SPECIFIC ) != MBEDTLS_ASN1_CONTEXT_SPECIFIC ) + if( ( tag & MBEDTLS_ASN1_TAG_CLASS_MASK ) != + MBEDTLS_ASN1_CONTEXT_SPECIFIC ) + { return( MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG ); + } /* Skip everything but DNS name */ if( tag != ( MBEDTLS_ASN1_CONTEXT_SPECIFIC | 2 ) ) From ceae42659b9d172df9c28a2643a7e6fe7b620cc9 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 25 Aug 2017 17:17:34 +0100 Subject: [PATCH 3/6] Add ChangeLog entry --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index ded60d39f..c81c5d6f5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Bugfix * Fix leap year calculation in x509_date_is_valid() to ensure that invalid dates on leap years with 100 and 400 intervals are handled correctly. Found by Nicholas Wilson. #694 + * Fix X509 CRT parsing that would potentially accept an invalid tag when + parsing the subject alternative names. = mbed TLS 2.6.0 branch released 2017-08-10 From 72705c906c73cc15512c04d317ff0fba6bc4c8e0 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 7 Nov 2017 20:16:19 +0000 Subject: [PATCH 4/6] Add regression test for parsing subjectAltNames --- tests/suites/test_suite_x509parse.data | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index d4cc11a08..374f1c2cc 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1124,6 +1124,10 @@ x509parse_crt:"30173015a0030201038204deadbeef30080604cafed00d0500":"":MBEDTLS_ER X509 Certificate ASN1 (invalid version overflow) x509parse_crt:"301A3018a00602047FFFFFFF8204deadbeef30080604cafed00d0500":"":MBEDTLS_ERR_X509_UNKNOWN_VERSION +X509 Certificate ASN1 (invalid SubjectAltNames tag) +depends_on:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +x509parse_crt:"308203723082025AA003020102020111300D06092A864886F70D0101050500303B310B3009060355040613024E4C3111300F060355040A1308506F6C617253534C3119301706035504031310506F6C617253534C2054657374204341301E170D3132303531303133323334315A170D3232303531313133323334315A303A310B3009060355040613024E4C3111300F060355040A1308506F6C617253534C311830160603550403130F7777772E6578616D706C652E636F6D30820122300D06092A864886F70D01010105000382010F003082010A0282010100B93C4AC5C8A38E9017A49E52AA7175266180E7C7B56D8CFFAAB64126B7BE11AD5C73160C64114804FFD6E13B05DB89BBB39709D51C14DD688739B03D71CBE276D01AD8182D801B54F6E5449AF1CBAF612EDF490D9D09B7EDB1FD3CFD3CFA24CF5DBF7CE453E725B5EA4422E926D3EA20949EE66167BA2E07670B032FA209EDF0338F0BCE10EF67A4C608DAC1EDC23FD74ADD153DF95E1C8160463EB5B33D2FA6DE471CBC92AEEBDF276B1656B7DCECD15557A56EEC7525F5B77BDFABD23A5A91987D97170B130AA76B4A8BC14730FB3AF84104D5C1DFB81DBF7B01A565A2E01E36B7A65CCC305AF8CD6FCDF1196225CA01E3357FFA20F5DCFD69B26A007D17F70203010001A38181307F30090603551D1304023000301D0603551D0E041604147DE49C6BE6F9717D46D2123DAD6B1DFDC2AA784C301F0603551D23041830168014B45AE4A5B3DED252F6B9D5A6950FEB3EBCC7FDFF30320603551D11042B3029C20B6578616D706C652E636F6D820B6578616D706C652E6E6574820D2A2E6578616D706C652E6F7267300D06092A864886F70D010105050003820101004F09CB7AD5EEF5EF620DDC7BA285D68CCA95B46BDA115B92007513B9CA0BCEEAFBC31FE23F7F217479E2E6BCDA06E52F6FF655C67339CF48BC0D2F0CD27A06C34A4CD9485DA0D07389E4D4851D969A0E5799C66F1D21271F8D0529E840AE823968C39707CF3C934C1ADF2FA6A455487F7C8C1AC922DA24CD9239C68AECB08DF5698267CB04EEDE534196C127DC2FFE33FAD30EB8D432A9842853A5F0D189D5A298E71691BB9CC0418E8C58ACFFE3DD2E7AABB0B97176AD0F2733F7A929D3C076C0BF06407C0ED5A47C8AE2326E16AEDA641FB0557CDBDDF1A4BA447CB39958D2346E00EA976C143AF2101E0AA249107601F4F2C818FDCC6346128B091BF194E6":"":MBEDTLS_ERR_X509_INVALID_EXTENSIONS + MBEDTLS_ERR_ASN1_UNEXPECTED_TAG + X509 CRL ASN1 (Incorrect first tag) x509parse_crl:"":"":MBEDTLS_ERR_X509_INVALID_FORMAT From 7786abc16b91897ff185b8141426393f23a5ff7f Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 7 Nov 2017 20:21:56 +0000 Subject: [PATCH 5/6] Define ASN1 bitmask macros in more direct way --- include/mbedtls/asn1.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 8d35c4245..75b7b3dfb 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -99,9 +99,9 @@ * | Class | P/C | Tag number | * +-------+-----+------------+ */ -#define MBEDTLS_ASN1_TAG_CLASS_MASK ( 0x03 << 6 ) -#define MBEDTLS_ASN1_TAG_PC_MASK ( 0x01 << 5 ) -#define MBEDTLS_ASN1_TAG_VALUE_MASK ( 0x1F << 0 ) +#define MBEDTLS_ASN1_TAG_CLASS_MASK 0xC0 +#define MBEDTLS_ASN1_TAG_PC_MASK 0x20 +#define MBEDTLS_ASN1_TAG_VALUE_MASK 0x1F /* \} name */ /* \} addtogroup asn1_module */ From 1ed45ea36b345327d4f6af0344fc0518b22872fb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 8 Mar 2018 18:16:45 +0100 Subject: [PATCH 6/6] Refer to X.690 by number It's easier to identify and find by number than by its very wordy title, especially as there was a typo in the title. --- include/mbedtls/asn1.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/mbedtls/asn1.h b/include/mbedtls/asn1.h index 75b7b3dfb..86b50e6c8 100644 --- a/include/mbedtls/asn1.h +++ b/include/mbedtls/asn1.h @@ -90,9 +90,8 @@ /* * Bit masks for each of the components of an ASN.1 tag as specified in - * Information technnology - ASN.1 encoding rules: Specification of Basic - * Encoding Rules (BER), Canonical Encoding Rules (CER), and Distinguished - * encoding rules (DER) Section 8.1.2.2: + * ITU X.690 (08/2015), section 8.1 "General rules for encoding", + * paragraph 8.1.2.2: * * Bit 8 7 6 5 1 * +-------+-----+------------+