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.

diff --git a/egg/egg-asn1x.c b/egg/egg-asn1x.c
index 743ba2c..50bc133 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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;