From 0b1557f7da4f6688abb96c50f374435e73962967 Mon Sep 17 00:00:00 2001 From: Stef Walter Date: Mon, 12 Sep 2011 12:21:05 +0200 Subject: [PATCH] gcr: Accept slightly invalid PKCS#12 files * In particular when the order of a SET OF is incorrect as is generated by certain implementations. * Revert cbecc802e8cf5803aac9fbd3c546b539773220b2 since this fix was wrong. * Add egg_asn1x_decode_no_validate() so that callers can validate on their own and specify validation options. --- egg/egg-asn1x.c | 70 +++++++++++++++++++++++++++++++++++++------------------- egg/egg-asn1x.h | 7 +++++- gcr/gcr-parser.c | 58 ++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 103 insertions(+), 32 deletions(-) diff --git a/egg/egg-asn1x.c b/egg/egg-asn1x.c index 743ba2c..50bc133 100644 --- a/egg/egg-asn1x.c +++ b/egg/egg-asn1x.c @@ -158,7 +158,7 @@ struct _Abits { /* Forward Declarations */ static gboolean anode_decode_anything (GNode*, Atlv*); static gboolean anode_decode_anything_for_flags (GNode *, Atlv*, gint); -static gboolean anode_validate_anything (GNode*); +static gboolean anode_validate_anything (GNode*, gboolean); static gboolean anode_encode_prepare (GNode*, gboolean want); static gint @@ -354,13 +354,13 @@ anode_opts_lookup (GNode *node, gint type, const gchar *name) static gint compare_tlvs (Atlv *tlva, Atlv *tlvb) { - gint la = tlva->len; - gint lb = tlvb->len; + gint la = tlva->off + tlva->len; + gint lb = tlvb->off + tlvb->len; gint res; g_assert (tlva->buf); g_assert (tlvb->buf); - res = memcmp (tlva->buf + tlva->off, tlvb->buf + tlvb->off, MIN (la, lb)); + res = memcmp (tlva->buf, tlvb->buf, MIN (la, lb)); if (la == lb || res != 0) return res; return la < lb ? -1 : 1; @@ -1139,14 +1139,12 @@ anode_decode_anything (GNode *node, Atlv *tlv) } gboolean -egg_asn1x_decode (GNode *asn, gconstpointer data, gsize n_data) +egg_asn1x_decode_no_validate (GNode *asn, + gconstpointer data, + gsize n_data) { Atlv tlv; - g_return_val_if_fail (asn, FALSE); - g_return_val_if_fail (data, FALSE); - g_return_val_if_fail (n_data, FALSE); - egg_asn1x_clear (asn); if (!anode_decode_tlv_for_data (data, (const guchar*)data + n_data, &tlv)) @@ -1158,7 +1156,25 @@ egg_asn1x_decode (GNode *asn, gconstpointer data, gsize n_data) if (tlv.end - tlv.buf != n_data) return FALSE; - return egg_asn1x_validate (asn); + return TRUE; +} + +gboolean +egg_asn1x_decode (GNode *asn, + gconstpointer data, + gsize n_data) +{ + gboolean ret; + + g_return_val_if_fail (asn, FALSE); + g_return_val_if_fail (data, FALSE); + g_return_val_if_fail (n_data, FALSE); + + ret = egg_asn1x_decode_no_validate (asn, data, n_data); + if (!ret) + return ret; + + return egg_asn1x_validate (asn, TRUE); } /* ----------------------------------------------------------------------------------- @@ -1705,7 +1721,7 @@ egg_asn1x_encode (GNode *asn, EggAllocator allocator, gsize *n_data) return NULL; if (anode_encode_build (asn, data, length) && - anode_validate_anything (asn)) { + anode_validate_anything (asn, TRUE)) { anode_encode_commit (asn); *n_data = length; return data; @@ -3334,7 +3350,8 @@ anode_validate_time (GNode *node, Atlv *tlv) } static gboolean -anode_validate_choice (GNode *node) +anode_validate_choice (GNode *node, + gboolean strict) { GNode *child, *choice; Anode *an; @@ -3344,7 +3361,7 @@ anode_validate_choice (GNode *node) if (!choice) return anode_failure (node, "one choice must be set"); - if (!anode_validate_anything (choice)) + if (!anode_validate_anything (choice, strict)) return FALSE; for (child = node->children; child; child = child->next) { @@ -3359,7 +3376,8 @@ anode_validate_choice (GNode *node) } static gboolean -anode_validate_sequence_or_set (GNode *node) +anode_validate_sequence_or_set (GNode *node, + gboolean strict) { GNode *child; gulong tag = 0; @@ -3371,7 +3389,7 @@ anode_validate_sequence_or_set (GNode *node) /* All of the children must validate properly */ for (child = node->children; child; child = child->next) { - if (!anode_validate_anything (child)) + if (!anode_validate_anything (child, strict)) return FALSE; /* Tags must be in ascending order */ @@ -3388,7 +3406,8 @@ anode_validate_sequence_or_set (GNode *node) } static gboolean -anode_validate_sequence_or_set_of (GNode *node) +anode_validate_sequence_or_set_of (GNode *node, + gboolean strict) { GNode *child; Atlv *tlv, *ptlv; @@ -3406,7 +3425,7 @@ anode_validate_sequence_or_set_of (GNode *node) for (child = node->children; child; child = child->next) { tlv = anode_get_tlv_data (child); if (tlv) { - if (!anode_validate_anything (child)) + if (!anode_validate_anything (child, strict)) return FALSE; /* Tag must have same tag as top */ @@ -3416,7 +3435,8 @@ anode_validate_sequence_or_set_of (GNode *node) return anode_failure (node, "invalid mismatched content"); /* Set of must be in ascending order */ - if (type == TYPE_SET_OF && ptlv && compare_tlvs (ptlv, tlv) > 0) + if (strict && type == TYPE_SET_OF && + ptlv != NULL && compare_tlvs (ptlv, tlv) > 0) return anode_failure (node, "content must be in ascending order"); ptlv = tlv; ++count; @@ -3427,7 +3447,8 @@ anode_validate_sequence_or_set_of (GNode *node) } static gboolean -anode_validate_anything (GNode *node) +anode_validate_anything (GNode *node, + gboolean strict) { Atlv *tlv; gint type; @@ -3471,16 +3492,16 @@ anode_validate_anything (GNode *node) case TYPE_ANY: return TRUE; case TYPE_CHOICE: - return anode_validate_choice (node); + return anode_validate_choice (node, strict); /* Structured types */ case TYPE_SEQUENCE: case TYPE_SET: - return anode_validate_sequence_or_set (node); + return anode_validate_sequence_or_set (node, strict); case TYPE_SEQUENCE_OF: case TYPE_SET_OF: - return anode_validate_sequence_or_set_of (node); + return anode_validate_sequence_or_set_of (node, strict); default: g_return_val_if_reached (FALSE); @@ -3488,10 +3509,11 @@ anode_validate_anything (GNode *node) } gboolean -egg_asn1x_validate (GNode *asn) +egg_asn1x_validate (GNode *asn, + gboolean strict) { g_return_val_if_fail (asn, FALSE); - return anode_validate_anything (asn); + return anode_validate_anything (asn, strict); } /* ----------------------------------------------------------------------------------- diff --git a/egg/egg-asn1x.h b/egg/egg-asn1x.h index 86b76c5..b6c8427 100644 --- a/egg/egg-asn1x.h +++ b/egg/egg-asn1x.h @@ -54,7 +54,12 @@ gboolean egg_asn1x_decode (GNode *asn, gconstpointer data, gsize n_data); -gboolean egg_asn1x_validate (GNode *asn); +gboolean egg_asn1x_decode_no_validate (GNode *asn, + gconstpointer data, + gsize n_data); + +gboolean egg_asn1x_validate (GNode *asn, + gboolean strict); gpointer egg_asn1x_encode (GNode *asn, EggAllocator allocator, diff --git a/gcr/gcr-parser.c b/gcr/gcr-parser.c index ff96789..496c51d 100644 --- a/gcr/gcr-parser.c +++ b/gcr/gcr-parser.c @@ -815,6 +815,38 @@ done: * PKCS12 */ +static GNode * +decode_pkcs12_asn1_accepting_invalid_crap (const ASN1_ARRAY_TYPE *defs, + const gchar *identifier, + gconstpointer data, + gsize n_data) +{ + GNode *asn; + + /* + * Because PKCS#12 files, the bags specifically, are notorious for + * being crappily constructed and are often break rules such as DER + * sorting order etc.. we parse the DER in a non-strict fashion. + * + * The rules in DER are designed for X.509 certificates, so there is + * only one way to represent a given certificate (although they fail + * at that as well). But with PKCS#12 we don't have such high + * requirements, and we can slack off on our validation. + */ + + asn = egg_asn1x_create (defs, identifier); + g_return_val_if_fail (asn != NULL, NULL); + + /* Passing FALSE as the strictness argument */ + if (!egg_asn1x_decode_no_validate (asn, data, n_data) || + !egg_asn1x_validate (asn, FALSE)) { + egg_asn1x_destroy (asn); + asn = NULL; + } + + return asn; +} + static gint handle_pkcs12_cert_bag (GcrParser *self, const guchar *data, gsize n_data) { @@ -826,7 +858,9 @@ handle_pkcs12_cert_bag (GcrParser *self, const guchar *data, gsize n_data) gint ret; ret = GCR_ERROR_UNRECOGNIZED; - asn = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-12-CertBag", data, n_data); + asn = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, + "pkcs-12-CertBag", + data, n_data); if (!asn) goto done; @@ -902,7 +936,9 @@ handle_pkcs12_bag (GcrParser *self, const guchar *data, gsize n_data) ret = GCR_ERROR_UNRECOGNIZED; - asn = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-12-SafeContents", data, n_data); + asn = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, + "pkcs-12-SafeContents", + data, n_data); if (!asn) goto done; @@ -981,7 +1017,9 @@ handle_pkcs12_encrypted_bag (GcrParser *self, const guchar *data, gsize n_data) ret = GCR_ERROR_UNRECOGNIZED; - asn = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-7-EncryptedData", data, n_data); + asn = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, + "pkcs-7-EncryptedData", + data, n_data); if (!asn) goto done; @@ -1068,7 +1106,9 @@ handle_pkcs12_safe (GcrParser *self, const guchar *data, gsize n_data) ret = GCR_ERROR_UNRECOGNIZED; - asn = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-12-AuthenticatedSafe", data, n_data); + asn = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, + "pkcs-12-AuthenticatedSafe", + data, n_data); if (!asn) goto done; @@ -1097,7 +1137,9 @@ handle_pkcs12_safe (GcrParser *self, const guchar *data, gsize n_data) if (oid == GCR_OID_PKCS7_DATA) { egg_asn1x_destroy (asn_content); - asn_content = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-7-Data", bag, n_bag); + asn_content = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, + "pkcs-7-Data", + bag, n_bag); if (!asn_content) goto done; @@ -1236,7 +1278,8 @@ parse_der_pkcs12 (GcrParser *self, const guchar *data, gsize n_data) ret = GCR_ERROR_UNRECOGNIZED; - asn = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-12-PFX", data, n_data); + asn = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, "pkcs-12-PFX", + data, n_data); if (!asn) goto done; @@ -1256,7 +1299,8 @@ parse_der_pkcs12 (GcrParser *self, const guchar *data, gsize n_data) if (!element) goto done; - asn_content = egg_asn1x_create_and_decode (pkix_asn1_tab, "pkcs-7-Data", element, n_element); + asn_content = decode_pkcs12_asn1_accepting_invalid_crap (pkix_asn1_tab, "pkcs-7-Data", + element, n_element); if (!asn_content) goto done; -- 2.7.4