From e3d4fe2be3bda0d9acc985a6cd67efe249d8326c Mon Sep 17 00:00:00 2001 From: Andreas Steffen Date: Fri, 5 May 2017 09:01:08 +0200 Subject: [PATCH] asn1-parser: Fix CHOICE parsing Also fixes the application in the x509 plugin and the parsing of nameConstraints, which doesn't require a loop. Fixes: CVE-2017-9023 --- src/libstrongswan/asn1/asn1_parser.c | 70 +++++++++++++++--- src/libstrongswan/asn1/asn1_parser.h | 27 +++---- src/libstrongswan/plugins/x509/x509_cert.c | 115 +++++++++++++++-------------- 3 files changed, 135 insertions(+), 77 deletions(-) diff --git a/src/libstrongswan/asn1/asn1_parser.c b/src/libstrongswan/asn1/asn1_parser.c index e7b7a428d9a2..4d5f799b73a9 100644 --- a/src/libstrongswan/asn1/asn1_parser.c +++ b/src/libstrongswan/asn1/asn1_parser.c @@ -1,8 +1,7 @@ /* * Copyright (C) 2006 Martin Will - * Copyright (C) 2000-2008 Andreas Steffen - * - * Hochschule fuer Technik Rapperswil + * Copyright (C) 2000-2017 Andreas Steffen + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -76,12 +75,18 @@ struct private_asn1_parser_t { * Current parsing pointer for each level */ chunk_t blobs[ASN1_MAX_LEVEL + 2]; + + /** + * Parsing a CHOICE on the current level ? + */ + bool choice[ASN1_MAX_LEVEL + 2]; + }; METHOD(asn1_parser_t, iterate, bool, private_asn1_parser_t *this, int *objectID, chunk_t *object) { - chunk_t *blob, *blob1; + chunk_t *blob, *blob1, blob_ori; u_char *start_ptr; u_int level; asn1Object_t obj; @@ -97,7 +102,7 @@ METHOD(asn1_parser_t, iterate, bool, return FALSE; } - if (obj.flags & ASN1_END) /* end of loop or option found */ + if (obj.flags & ASN1_END) /* end of loop or choice or option found */ { if (this->loopAddr[obj.level] && this->blobs[obj.level+1].len > 0) { @@ -106,13 +111,42 @@ METHOD(asn1_parser_t, iterate, bool, } else { - this->loopAddr[obj.level] = 0; /* exit loop or option*/ + this->loopAddr[obj.level] = 0; /* exit loop */ + + if (obj.flags & ASN1_CHOICE) /* end of choices */ + { + if (this->choice[obj.level+1]) + { + DBG1(DBG_LIB, "L%d - %s: incorrect choice encoding", + this->level0 + obj.level, obj.name); + this->success = FALSE; + goto end; + } + } + + if (obj.flags & ASN1_CH) /* end of choice */ + { + /* parsed a valid choice */ + this->choice[obj.level] = FALSE; + + /* advance to end of choices */ + do + { + this->line++; + } + while (!((this->objects[this->line].flags & ASN1_END) && + (this->objects[this->line].flags & ASN1_CHOICE) && + (this->objects[this->line].level == obj.level-1))); + this->line--; + } + goto end; } } level = this->level0 + obj.level; blob = this->blobs + obj.level; + blob_ori = *blob; blob1 = blob + 1; start_ptr = blob->ptr; @@ -129,7 +163,6 @@ METHOD(asn1_parser_t, iterate, bool, } /* handle ASN.1 options */ - if ((obj.flags & ASN1_OPT) && (blob->len == 0 || *start_ptr != obj.type)) { @@ -144,7 +177,6 @@ METHOD(asn1_parser_t, iterate, bool, } /* an ASN.1 object must possess at least a tag and length field */ - if (blob->len < 2) { DBG1(DBG_LIB, "L%d - %s: ASN.1 object smaller than 2 octets", @@ -167,8 +199,16 @@ METHOD(asn1_parser_t, iterate, bool, blob->ptr += blob1->len; blob->len -= blob1->len; - /* return raw ASN.1 object without prior type checking */ + /* handle ASN.1 choice without explicit context encoding */ + if ((obj.flags & ASN1_CHOICE) && obj.type == ASN1_EOC) + { + DBG2(DBG_LIB, "L%d - %s:", level, obj.name); + this->choice[obj.level+1] = TRUE; + *blob1 = blob_ori; + goto end; + } + /* return raw ASN.1 object without prior type checking */ if (obj.flags & ASN1_RAW) { DBG2(DBG_LIB, "L%d - %s:", level, obj.name); @@ -209,6 +249,18 @@ METHOD(asn1_parser_t, iterate, bool, } } + /* In case of a "CHOICE" start to scan for exactly one valid choice */ + if (obj.flags & ASN1_CHOICE) + { + if (blob1->len == 0) + { + DBG1(DBG_LIB, "L%d - %s: contains no choice", level, obj.name); + this->success = FALSE; + goto end; + } + this->choice[obj.level+1] = TRUE; + } + if (obj.flags & ASN1_OBJ) { object->ptr = start_ptr; diff --git a/src/libstrongswan/asn1/asn1_parser.h b/src/libstrongswan/asn1/asn1_parser.h index 0edc22c2378c..2ee1e892fc16 100644 --- a/src/libstrongswan/asn1/asn1_parser.h +++ b/src/libstrongswan/asn1/asn1_parser.h @@ -1,8 +1,7 @@ /* * Copyright (C) 2006 Martin Will - * Copyright (C) 2000-2008 Andreas Steffen - * - * Hochschule fuer Technik Rapperswil + * Copyright (C) 2000-2017 Andreas Steffen + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -32,15 +31,17 @@ /** * Definition of ASN.1 flags */ -#define ASN1_NONE 0x00 -#define ASN1_DEF 0x01 -#define ASN1_OPT 0x02 -#define ASN1_LOOP 0x04 -#define ASN1_END 0x08 -#define ASN1_OBJ 0x10 -#define ASN1_BODY 0x20 -#define ASN1_RAW 0x40 -#define ASN1_EXIT 0x80 +#define ASN1_NONE 0x0000 +#define ASN1_DEF 0x0001 +#define ASN1_OPT 0x0002 +#define ASN1_LOOP 0x0004 +#define ASN1_CHOICE 0x0008 +#define ASN1_CH 0x0010 +#define ASN1_END 0x0020 +#define ASN1_OBJ 0x0040 +#define ASN1_BODY 0x0080 +#define ASN1_RAW 0x0100 +#define ASN1_EXIT 0x0200 typedef struct asn1Object_t asn1Object_t; @@ -51,7 +52,7 @@ struct asn1Object_t{ u_int level; const u_char *name; asn1_t type; - u_char flags; + uint16_t flags; }; typedef struct asn1_parser_t asn1_parser_t; diff --git a/src/libstrongswan/plugins/x509/x509_cert.c b/src/libstrongswan/plugins/x509/x509_cert.c index b3d90c5f61ef..f9573e953cbf 100644 --- a/src/libstrongswan/plugins/x509/x509_cert.c +++ b/src/libstrongswan/plugins/x509/x509_cert.c @@ -2,10 +2,10 @@ * Copyright (C) 2000 Andreas Hess, Patric Lichtsteiner, Roger Wegmann * Copyright (C) 2001 Marco Bertossa, Andreas Schleiss * Copyright (C) 2002 Mario Strasser - * Copyright (C) 2000-2006 Andreas Steffen + * Copyright (C) 2000-2017 Andreas Steffen * Copyright (C) 2006-2009 Martin Willi * Copyright (C) 2008 Tobias Brunner - * Hochschule fuer Technik Rapperswil + * HSR Hochschule fuer Technik Rapperswil * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License as published by the @@ -789,20 +789,20 @@ static bool parse_extendedKeyUsage(chunk_t blob, int level0, * ASN.1 definition of crlDistributionPoints */ static const asn1Object_t crlDistributionPointsObjects[] = { - { 0, "crlDistributionPoints", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ - { 1, "DistributionPoint", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ - { 2, "distributionPoint", ASN1_CONTEXT_C_0, ASN1_OPT|ASN1_LOOP }, /* 2 */ - { 3, "fullName", ASN1_CONTEXT_C_0, ASN1_OPT|ASN1_OBJ }, /* 3 */ - { 3, "end choice", ASN1_EOC, ASN1_END }, /* 4 */ - { 3, "nameRelToCRLIssuer",ASN1_CONTEXT_C_1, ASN1_OPT|ASN1_BODY }, /* 5 */ - { 3, "end choice", ASN1_EOC, ASN1_END }, /* 6 */ - { 2, "end opt", ASN1_EOC, ASN1_END }, /* 7 */ - { 2, "reasons", ASN1_CONTEXT_C_1, ASN1_OPT|ASN1_BODY }, /* 8 */ - { 2, "end opt", ASN1_EOC, ASN1_END }, /* 9 */ - { 2, "crlIssuer", ASN1_CONTEXT_C_2, ASN1_OPT|ASN1_OBJ }, /* 10 */ - { 2, "end opt", ASN1_EOC, ASN1_END }, /* 11 */ - { 0, "end loop", ASN1_EOC, ASN1_END }, /* 12 */ - { 0, "exit", ASN1_EOC, ASN1_EXIT } + { 0, "crlDistributionPoints", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ + { 1, "DistributionPoint", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ + { 2, "distributionPoint", ASN1_CONTEXT_C_0, ASN1_OPT|ASN1_CHOICE }, /* 2 */ + { 3, "fullName", ASN1_CONTEXT_C_0, ASN1_OPT|ASN1_OBJ }, /* 3 */ + { 3, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 4 */ + { 3, "nameRelToCRLIssuer",ASN1_CONTEXT_C_1, ASN1_OPT|ASN1_BODY }, /* 5 */ + { 3, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 6 */ + { 2, "end opt/choices", ASN1_EOC, ASN1_END|ASN1_CHOICE }, /* 7 */ + { 2, "reasons", ASN1_CONTEXT_C_1, ASN1_OPT|ASN1_BODY }, /* 8 */ + { 2, "end opt", ASN1_EOC, ASN1_END }, /* 9 */ + { 2, "crlIssuer", ASN1_CONTEXT_C_2, ASN1_OPT|ASN1_OBJ }, /* 10 */ + { 2, "end opt", ASN1_EOC, ASN1_END }, /* 11 */ + { 0, "end loop", ASN1_EOC, ASN1_END }, /* 12 */ + { 0, "exit", ASN1_EOC, ASN1_EXIT } }; #define CRL_DIST_POINTS 1 #define CRL_DIST_POINTS_FULLNAME 3 @@ -910,14 +910,13 @@ end: * ASN.1 definition of nameConstraints */ static const asn1Object_t nameConstraintsObjects[] = { - { 0, "nameConstraints", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ + { 0, "nameConstraints", ASN1_SEQUENCE, ASN1_NONE }, /* 0 */ { 1, "permittedSubtrees", ASN1_CONTEXT_C_0, ASN1_OPT|ASN1_LOOP }, /* 1 */ { 2, "generalSubtree", ASN1_SEQUENCE, ASN1_BODY }, /* 2 */ { 1, "end loop", ASN1_EOC, ASN1_END }, /* 3 */ { 1, "excludedSubtrees", ASN1_CONTEXT_C_1, ASN1_OPT|ASN1_LOOP }, /* 4 */ { 2, "generalSubtree", ASN1_SEQUENCE, ASN1_BODY }, /* 5 */ { 1, "end loop", ASN1_EOC, ASN1_END }, /* 6 */ - { 0, "end loop", ASN1_EOC, ASN1_END }, /* 7 */ { 0, "exit", ASN1_EOC, ASN1_EXIT } }; #define NAME_CONSTRAINT_PERMITTED 2 @@ -974,25 +973,27 @@ end: * ASN.1 definition of a certificatePolicies extension */ static const asn1Object_t certificatePoliciesObject[] = { - { 0, "certificatePolicies", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ - { 1, "policyInformation", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ - { 2, "policyId", ASN1_OID, ASN1_BODY }, /* 2 */ - { 2, "qualifier", ASN1_SEQUENCE, ASN1_OPT|ASN1_BODY }, /* 3 */ - { 3, "qualifierInfo", ASN1_SEQUENCE, ASN1_NONE }, /* 4 */ - { 4, "qualifierId", ASN1_OID, ASN1_BODY }, /* 5 */ - { 4, "cPSuri", ASN1_IA5STRING, ASN1_OPT|ASN1_BODY }, /* 6 */ - { 4, "end choice", ASN1_EOC, ASN1_END }, /* 7 */ - { 4, "userNotice", ASN1_SEQUENCE, ASN1_OPT|ASN1_NONE }, /* 8 */ - { 5, "explicitText", ASN1_EOC, ASN1_RAW }, /* 9 */ - { 4, "end choice", ASN1_EOC, ASN1_END }, /* 10 */ - { 2, "end opt", ASN1_EOC, ASN1_END }, /* 12 */ - { 0, "end loop", ASN1_EOC, ASN1_END }, /* 13 */ - { 0, "exit", ASN1_EOC, ASN1_EXIT } + { 0, "certificatePolicies", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ + { 1, "policyInformation", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ + { 2, "policyId", ASN1_OID, ASN1_BODY }, /* 2 */ + { 2, "qualifiers", ASN1_SEQUENCE, ASN1_OPT|ASN1_LOOP }, /* 3 */ + { 3, "qualifierInfo", ASN1_SEQUENCE, ASN1_NONE }, /* 4 */ + { 4, "qualifierId", ASN1_OID, ASN1_BODY }, /* 5 */ + { 4, "qualifier", ASN1_EOC, ASN1_CHOICE }, /* 6 */ + { 5, "cPSuri", ASN1_IA5STRING, ASN1_OPT|ASN1_BODY }, /* 7 */ + { 5, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 8 */ + { 5, "userNotice", ASN1_SEQUENCE, ASN1_OPT|ASN1_BODY }, /* 9 */ + { 6, "explicitText", ASN1_EOC, ASN1_RAW }, /* 10 */ + { 5, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 11 */ + { 4, "end choices", ASN1_EOC, ASN1_END|ASN1_CHOICE }, /* 12 */ + { 2, "end opt/loop", ASN1_EOC, ASN1_END }, /* 13 */ + { 0, "end loop", ASN1_EOC, ASN1_END }, /* 14 */ + { 0, "exit", ASN1_EOC, ASN1_EXIT } }; -#define CERT_POLICY_ID 2 -#define CERT_POLICY_QUALIFIER_ID 5 -#define CERT_POLICY_CPS_URI 6 -#define CERT_POLICY_EXPLICIT_TEXT 9 +#define CERT_POLICY_ID 2 +#define CERT_POLICY_QUALIFIER_ID 5 +#define CERT_POLICY_CPS_URI 7 +#define CERT_POLICY_EXPLICIT_TEXT 10 /** * Parse certificatePolicies @@ -1157,27 +1158,31 @@ static bool parse_policyConstraints(chunk_t blob, int level0, * ASN.1 definition of ipAddrBlocks according to RFC 3779 */ static const asn1Object_t ipAddrBlocksObjects[] = { - { 0, "ipAddrBlocks", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ - { 1, "ipAddressFamily", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ - { 2, "addressFamily", ASN1_OCTET_STRING, ASN1_BODY }, /* 2 */ - { 2, "inherit", ASN1_NULL, ASN1_OPT|ASN1_NONE }, /* 3 */ - { 2, "end choice", ASN1_EOC, ASN1_END }, /* 4 */ - { 2, "addressesOrRanges", ASN1_SEQUENCE, ASN1_OPT|ASN1_LOOP }, /* 5 */ - { 3, "addressPrefix", ASN1_BIT_STRING, ASN1_OPT|ASN1_BODY }, /* 6 */ - { 3, "end choice", ASN1_EOC, ASN1_END }, /* 7 */ - { 3, "addressRange", ASN1_SEQUENCE, ASN1_OPT|ASN1_NONE }, /* 8 */ - { 4, "min", ASN1_BIT_STRING, ASN1_BODY }, /* 9 */ - { 4, "max", ASN1_BIT_STRING, ASN1_BODY }, /* 10 */ - { 3, "end choice", ASN1_EOC, ASN1_END }, /* 11 */ - { 2, "end choice/loop", ASN1_EOC, ASN1_END }, /* 12 */ - { 0, "end loop", ASN1_EOC, ASN1_END }, /* 13 */ - { 0, "exit", ASN1_EOC, ASN1_EXIT } + { 0, "ipAddrBlocks", ASN1_SEQUENCE, ASN1_LOOP }, /* 0 */ + { 1, "ipAddressFamily", ASN1_SEQUENCE, ASN1_NONE }, /* 1 */ + { 2, "addressFamily", ASN1_OCTET_STRING, ASN1_BODY }, /* 2 */ + { 2, "ipAddressChoice", ASN1_EOC, ASN1_CHOICE }, /* 3 */ + { 3, "inherit", ASN1_NULL, ASN1_OPT }, /* 4 */ + { 3, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 5 */ + { 3, "addressesOrRanges", ASN1_SEQUENCE, ASN1_OPT|ASN1_LOOP }, /* 6 */ + { 4, "addressOrRange", ASN1_EOC, ASN1_CHOICE }, /* 7 */ + { 5, "addressPrefix", ASN1_BIT_STRING, ASN1_OPT|ASN1_BODY }, /* 8 */ + { 5, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 9 */ + { 5, "addressRange", ASN1_SEQUENCE, ASN1_OPT }, /* 10 */ + { 6, "min", ASN1_BIT_STRING, ASN1_BODY }, /* 11 */ + { 6, "max", ASN1_BIT_STRING, ASN1_BODY }, /* 12 */ + { 5, "end choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 13 */ + { 4, "end choices", ASN1_EOC, ASN1_END|ASN1_CHOICE }, /* 14 */ + { 3, "end loop/choice", ASN1_EOC, ASN1_END|ASN1_CH }, /* 15 */ + { 2, "end choices", ASN1_EOC, ASN1_END|ASN1_CHOICE }, /* 16 */ + { 0, "end loop", ASN1_EOC, ASN1_END }, /* 17 */ + { 0, "exit", ASN1_EOC, ASN1_EXIT } }; #define IP_ADDR_BLOCKS_FAMILY 2 -#define IP_ADDR_BLOCKS_INHERIT 3 -#define IP_ADDR_BLOCKS_PREFIX 6 -#define IP_ADDR_BLOCKS_MIN 9 -#define IP_ADDR_BLOCKS_MAX 10 +#define IP_ADDR_BLOCKS_INHERIT 4 +#define IP_ADDR_BLOCKS_PREFIX 8 +#define IP_ADDR_BLOCKS_MIN 11 +#define IP_ADDR_BLOCKS_MAX 12 static bool check_address_object(ts_type_t ts_type, chunk_t object) { -- 1.9.1