Fix the PKCS#7 stuff: signature verify could fail if attributes reordered, the

detached data encoding was wrong and free up public keys.

diff --git a/CHANGES b/CHANGES
index 5346d4001e941024013b28a6c3d517d46c5865a2..81bd07e5601cd93a9fbd2b7a18176d7856077d59 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -5,6 +5,12 @@
 
  Changes between 0.9.1c and 0.9.2
 
+  *) Add a bunch of fixes to the PKCS#7 stuff. It used to sometimes reorder
+     signed attributes when verifying signatures (this would break them), 
+     the detached data encoding was wrong and public keys obtained using
+     X509_get_pubkey() weren't freed.
+     [Steve Henson]
+
   *) Add text documentation for the BUFFER functions. Also added a work around
      to a Win95 console bug. This was triggered by the password read stuff: the
      last character typed gets carried over to the next fread(). If you were 

diff --git a/crypto/pkcs7/example.c b/crypto/pkcs7/example.c
index 9309e1d5efed7baaed514a04c30f96edf6f0fd87..7dd81e30230215e44e964f82d2ae23c81ea1409f 100644 (file)
--- a/crypto/pkcs7/example.c
+++ b/crypto/pkcs7/example.c
@@ -135,7 +135,7 @@ char **str2;
                        OBJ_create("1.9.9999","OID_example","Our example OID");
        /* To retrieve */
        so=PKCS7_get_signed_attribute(si,signed_seq2string_nid);
-       if (so->type == V_ASN1_SEQUENCE)
+       if (so && (so->type == V_ASN1_SEQUENCE))
                {
                ASN1_CTX c;
                ASN1_STRING *s;

diff --git a/crypto/pkcs7/pk7_doit.c b/crypto/pkcs7/pk7_doit.c
index 071ff099377c284454d99fbd3d6b1fead84cba3d..2e27f7d3daad41a62ef9fd14d58ae106cb77e8d9 100644 (file)
--- a/crypto/pkcs7/pk7_doit.c
+++ b/crypto/pkcs7/pk7_doit.c
@@ -185,6 +185,7 @@ BIO *bio;
                                }
                        pkey=X509_get_pubkey(ri->cert);
                        jj=EVP_PKEY_size(pkey);
+                       EVP_PKEY_free(pkey);
                        if (max < jj) max=jj;
                        }
                if ((tmp=(unsigned char *)Malloc(max)) == NULL)
@@ -197,6 +198,7 @@ BIO *bio;
                        ri=(PKCS7_RECIP_INFO *)sk_value(rsk,i);
                        pkey=X509_get_pubkey(ri->cert);
                        jj=EVP_PKEY_encrypt(tmp,key,keylen,pkey);
+                       EVP_PKEY_free(pkey);
                        if (jj <= 0)
                                {
                                PKCS7err(PKCS7_F_PKCS7_DATAINIT,ERR_R_EVP_LIB);
@@ -503,6 +505,11 @@ BIO *bio;
        case NID_pkcs7_signed:
                si_sk=p7->d.sign->signer_info;
                os=p7->d.sign->contents->d.data;
+               /* If detached data then the content is excluded */
+               if(p7->detached) {
+                       ASN1_OCTET_STRING_free(os);
+                       p7->d.sign->contents->d.data = NULL;
+               }
                break;
                }
 
@@ -608,9 +615,7 @@ BIO *bio;
                        }
                }
 
-       if (p7->detached)
-               ASN1_OCTET_STRING_set(os,(unsigned char *)"",0);
-       else
+       if (!p7->detached)
                {
                btmp=BIO_find_type(bio,BIO_TYPE_MEM);
                if (btmp == NULL)
@@ -648,6 +653,7 @@ PKCS7_SIGNER_INFO *si;
        STACK *sk,*cert;
        BIO *btmp;
        X509 *x509;
+       EVP_PKEY *pkey;
 
        if (PKCS7_type_is_signed(p7))
                {
@@ -742,22 +748,27 @@ for (ii=0; ii<md_len; ii++) printf("%02X",md_dat[ii]); printf(" calc\n");
                        }
 
                EVP_VerifyInit(&mdc_tmp,EVP_get_digestbynid(md_type));
+               /* Note: when forming the encoding of the attributes we
+                * shouldn't reorder them or this will break the signature.
+                * This is done by using the IS_SEQUENCE flag.
+                */
                i=i2d_ASN1_SET(sk,NULL,i2d_X509_ATTRIBUTE,
-                       V_ASN1_SET,V_ASN1_UNIVERSAL, IS_SET);
+                       V_ASN1_SET,V_ASN1_UNIVERSAL, IS_SEQUENCE);
                pp=(unsigned char *)Malloc(i);
                p=pp;
                i2d_ASN1_SET(sk,&p,i2d_X509_ATTRIBUTE,
-                       V_ASN1_SET,V_ASN1_UNIVERSAL, IS_SET);
+                       V_ASN1_SET,V_ASN1_UNIVERSAL, IS_SEQUENCE);
                EVP_VerifyUpdate(&mdc_tmp,pp,i);
+
                Free(pp);
                }
 
        os=si->enc_digest;
-       if (X509_get_pubkey(x509)->type == EVP_PKEY_DSA)
-               mdc_tmp.digest=EVP_dss1();
+       pkey = X509_get_pubkey(x509);
+       if(pkey->type == EVP_PKEY_DSA) mdc_tmp.digest=EVP_dss1();
 
-       i=EVP_VerifyFinal(&mdc_tmp,os->data,os->length,
-               X509_get_pubkey(x509));
+       i=EVP_VerifyFinal(&mdc_tmp,os->data,os->length, pkey);
+       EVP_PKEY_free(pkey);
        if (i <= 0)
                {
                PKCS7err(PKCS7_F_PKCS7_DATAVERIFY,PKCS7_R_SIGNATURE_FAILURE);

diff --git a/crypto/pkcs7/verify.c b/crypto/pkcs7/verify.c
index 7e0f6e5fee0d35f625fa0fc6ec8c165e015ca88e..38b89b508072c6969f9a5a5918668db7a7f4b513 100644 (file)
--- a/crypto/pkcs7/verify.c
+++ b/crypto/pkcs7/verify.c
@@ -190,7 +190,7 @@ again:
                        BIO_printf(bio_out,"String 1 is %s\n",str1);
                        BIO_printf(bio_out,"String 2 is %s\n",str2);
                        }
-                       
+
                }
 
        X509_STORE_free(cert_store);