Change the PKCS7 structure to use SEQUENCE OF for the

authenticated attributes: this is used to retain the
original encoding and not break signatures.

Support for a SET OF which reorders the STACK when
encoding a structure. This will be used with the
PKCS7 code.

4 files changed, 32 insertions, 2 deletions

diff --git a/CHANGES b/CHANGES
index fb55d68918..3efb236f63 100644
--- a/CHANGES
+++ b/CHANGES
@@ -3,6 +3,14 @@
 
  Changes between 0.9.6 and 0.9.7  [xx XXX 2000]
 
+  *) Add a special meaning when SET OF and SEQUENCE OF flags are both
+     set (this was treated exactly the same as SET OF previously). This
+     is used to reorder the STACK representing the structure to match the
+     encoding. This will be used to get round a problem where a PKCS7
+     structure which was signed could not be verified because the STACK
+     order did not reflect the encoded order.
+     [Steve Henson]
+
   *) Reimplement the OCSP ASN1 module using the new code.
      [Steve Henson]
 
diff --git a/crypto/asn1/asn1t.h b/crypto/asn1/asn1t.h
index 430c37124a..ea880d5150 100644
--- a/crypto/asn1/asn1t.h
+++ b/crypto/asn1/asn1t.h
@@ -375,6 +375,14 @@ struct ASN1_ADB_TABLE_st {
 /* Field is a SEQUENCE OF */
 #define ASN1_TFLG_SEQUENCE_OF	(0x2 << 1)
 
+/* Special case: this refers to a SET OF that
+ * will be sorted into DER order when encoded *and*
+ * the corresponding STACK will be modified to match
+ * the new order.
+ */
+#define ASN1_TFLG_SET_ORDER	(0x3 << 1)
+
+/* Mask for SET OF or SEQUENCE OF */
 #define ASN1_TFLG_SK_MASK	(0x3 << 1)
 
 /* These flags mean the tag should be taken from the
diff --git a/crypto/asn1/tasn_enc.c b/crypto/asn1/tasn_enc.c
index f6d33e1776..4b2784987a 100644
--- a/crypto/asn1/tasn_enc.c
+++ b/crypto/asn1/tasn_enc.c
@@ -221,7 +221,11 @@ int ASN1_template_i2d(ASN1_VALUE **pval, unsigned char **out, const ASN1_TEMPLAT
 		int skcontlen, sklen;
 		ASN1_VALUE *skitem;
 		if(!*pval) return 0;
-		isset = flags & ASN1_TFLG_SET_OF;
+		if(flags & ASN1_TFLG_SET_OF) {
+			isset = 1;
+			/* 2 means we reorder */
+			if(flags & ASN1_TFLG_SEQUENCE_OF) isset = 2;
+		} else isset = 0;
 		/* First work out inner tag value */
 		if(flags & ASN1_TFLG_IMPTAG) {
 			sktag = tt->tag;
@@ -284,6 +288,7 @@ int ASN1_template_i2d(ASN1_VALUE **pval, unsigned char **out, const ASN1_TEMPLAT
 typedef	struct {
 	unsigned char *data;
 	int length;
+	ASN1_VALUE *field;
 } DER_ENC;
 
 static int der_cmp(const void *a, const void *b)
@@ -327,6 +332,7 @@ static int asn1_set_seq_out(STACK_OF(ASN1_VALUE) *sk, unsigned char **out, int s
 		skitem = sk_ASN1_VALUE_value(sk, i);
 		tder->data = p;
 		tder->length = ASN1_item_i2d(skitem, &p, item);
+		tder->field = skitem;
 	}
 	/* Now sort them */
 	qsort(derlst, sk_ASN1_VALUE_num(sk), sizeof(*derlst), der_cmp);
@@ -337,6 +343,11 @@ static int asn1_set_seq_out(STACK_OF(ASN1_VALUE) *sk, unsigned char **out, int s
 		p += tder->length;
 	}
 	*out = p;
+	/* If do_sort is 2 then reorder the STACK */
+	if(do_sort == 2) {
+		for(i = 0, tder = derlst; i < sk_ASN1_VALUE_num(sk); i++, tder++)
+			sk_ASN1_VALUE_set(sk, i, tder->field);
+	}
 	OPENSSL_free(derlst);
 	OPENSSL_free(tmpdat);
 	return 1;
diff --git a/crypto/pkcs7/pk7_asn1.c b/crypto/pkcs7/pk7_asn1.c
index 777a8619d6..192890d6c6 100644
--- a/crypto/pkcs7/pk7_asn1.c
+++ b/crypto/pkcs7/pk7_asn1.c
@@ -108,7 +108,10 @@ ASN1_SEQUENCE_cb(PKCS7_SIGNER_INFO, si_cb) = {
 	ASN1_SIMPLE(PKCS7_SIGNER_INFO, version, ASN1_INTEGER),
 	ASN1_SIMPLE(PKCS7_SIGNER_INFO, issuer_and_serial, PKCS7_ISSUER_AND_SERIAL),
 	ASN1_SIMPLE(PKCS7_SIGNER_INFO, digest_alg, X509_ALGOR),
-	ASN1_IMP_SET_OF_OPT(PKCS7_SIGNER_INFO, auth_attr, X509_ATTRIBUTE, 0),
+	/* NB this should be a SET OF but we use a SEQUENCE OF so the original order
+	 * is retained when the structure is reencoded.
+	 */
+	ASN1_IMP_SEQUENCE_OF_OPT(PKCS7_SIGNER_INFO, auth_attr, X509_ATTRIBUTE, 0),
 	ASN1_SIMPLE(PKCS7_SIGNER_INFO, digest_enc_alg, X509_ALGOR),
 	ASN1_SIMPLE(PKCS7_SIGNER_INFO, enc_digest, ASN1_OCTET_STRING),
 	ASN1_IMP_SET_OF_OPT(PKCS7_SIGNER_INFO, unauth_attr, X509_ATTRIBUTE, 1)