Don't switch password formats using global state.

To avoid possible race conditions don't switch password format using global state in crypto/pkcs12 Reviewed-by: Richard Levitte <[email protected]>
wsware · Aug 25, 2016 · 0fe1749 · 0fe1749
1 parent cc06906
commit 0fe1749
Show file tree

Hide file tree

Showing 3 changed files with 3 additions and 77 deletions.
diff --git a/crypto/pkcs12/p12_crpt.c b/crypto/pkcs12/p12_crpt.c
@@ -17,16 +17,6 @@ void PKCS12_PBE_add(void)
 {
 }
 
-#undef PKCS12_key_gen
-/*
- * See p12_multi.c:PKCS12_verify_mac() for details...
- */
-extern int (*PKCS12_key_gen)(const char *pass, int passlen,
-                             unsigned char *salt, int slen,
-                             int id, int iter, int n,
-                             unsigned char *out,
-                             const EVP_MD *md_type);
-
 int PKCS12_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
                         ASN1_TYPE *param, const EVP_CIPHER *cipher,
                         const EVP_MD *md, int en_de)
@@ -41,13 +31,7 @@ int PKCS12_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
                           unsigned char *out,
                           const EVP_MD *md_type);
 
-    if (PKCS12_key_gen == NULL || en_de)
-        /*
-         * Default to UTF-8, but force it in encrypt case.
-         */
-        pkcs12_key_gen = PKCS12_key_gen_utf8;
-    else
-        pkcs12_key_gen = PKCS12_key_gen;
+    pkcs12_key_gen = PKCS12_key_gen_utf8;
 
     if (cipher == NULL)
         return 0;

diff --git a/crypto/pkcs12/p12_lcl.h b/crypto/pkcs12/p12_lcl.h
@@ -41,13 +41,3 @@ struct pkcs12_bag_st {
         ASN1_TYPE *other;       /* Secret or other bag */
     } value;
 };
-
-#undef PKCS12_key_gen
-/*
- * See p12_multi.c:PKCS12_verify_mac() for details...
- */
-extern int (*PKCS12_key_gen)(const char *pass, int passlen,
-                             unsigned char *salt, int slen,
-                             int id, int iter, int n,
-                             unsigned char *out,
-                             const EVP_MD *md_type);
diff --git a/crypto/pkcs12/p12_mutl.c b/crypto/pkcs12/p12_mutl.c
@@ -66,32 +66,6 @@ static int pkcs12_gen_gost_mac_key(const char *pass, int passlen,
     return 1;
 }
 
-#undef PKCS12_key_gen
-/*
- * |PKCS12_key_gen| is used to convey information about old-style broken
- * password being used to PKCS12_PBE_keyivgen in decrypt cases. Workflow
- * is if PKCS12_verify_mac notes that password encoded with compliant
- * PKCS12_key_gen_utf8 conversion subroutine isn't right, while encoded
- * with legacy non-compliant one is, then it sets |PKCS12_key_gen| to
- * legacy PKCS12_key_gen_asc conversion subroutine, which is then picked
- * by PKCS12_PBE_keyivgen. This applies to reading data. Written data
- * on the other hand is protected with standard-compliant encoding, i.e.
- * in backward-incompatible manner. Note that formally the approach is
- * not MT-safe. Rationale is that in order to access PKCS#12 files from
- * MT or even production application, you would be required to convert
- * data to correct interoperable format. In which case this variable
- * won't have to change. Conversion would have to be done with pkcs12
- * utility, which is not MT, and hence can tolerate it. In other words
- * goal is not to make this heuristic approach work in general case,
- * but in one specific one, apps/pkcs12.c.
- */
-int (*PKCS12_key_gen)(const char *pass, int passlen,
-                      unsigned char *salt, int slen,
-                      int id, int iter, int n,
-                      unsigned char *out,
-                      const EVP_MD *md_type) = NULL;
-
-
 /* Generate a MAC */
 static int pkcs12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
                           unsigned char *mac, unsigned int *maclen,
@@ -110,8 +84,6 @@ static int pkcs12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
     const X509_ALGOR *macalg;
     const ASN1_OBJECT *macoid;
 
-    if (pkcs12_key_gen == NULL)
-        pkcs12_key_gen = PKCS12_key_gen;
     if (pkcs12_key_gen == NULL)
         pkcs12_key_gen = PKCS12_key_gen_utf8;
 
@@ -187,30 +159,10 @@ int PKCS12_verify_mac(PKCS12 *p12, const char *pass, int passlen)
         return 0;
     }
     X509_SIG_get0(p12->mac->dinfo, NULL, &macoct);
-    if (maclen != (unsigned int)ASN1_STRING_length(macoct))
+    if ((maclen != (unsigned int)ASN1_STRING_length(macoct))
+        || CRYPTO_memcmp(mac, ASN1_STRING_get0_data(macoct), maclen) != 0)
         return 0;
 
-    if (CRYPTO_memcmp(mac, ASN1_STRING_get0_data(macoct), maclen) != 0) {
-        if (pass == NULL)
-            return 0;
-        /*
-         * In order to facilitate accessing old data retry with
-         * old-style broken password ...
-         */
-        if (!pkcs12_gen_mac(p12, pass, passlen, mac, &maclen,
-                            PKCS12_key_gen_asc)) {
-            PKCS12err(PKCS12_F_PKCS12_VERIFY_MAC, PKCS12_R_MAC_GENERATION_ERROR);
-            return 0;
-        }
-        if ((maclen != (unsigned int)ASN1_STRING_length(macoct))
-            || CRYPTO_memcmp(mac, ASN1_STRING_get0_data(macoct), maclen) != 0)
-            return 0;
-        else
-            PKCS12_key_gen = PKCS12_key_gen_asc;
-        /*
-         * ... and if suceeded, pass it on to PKCS12_PBE_keyivgen.
-         */
-    }
     return 1;
 }