crypto/pkcs12: facilitate accessing data with non-interoperable password.

Originally PKCS#12 subroutines treated password strings as ASCII.
It worked as long as they were pure ASCII, but if there were some
none-ASCII characters result was non-interoperable. But fixing it
poses problem accessing data protected with broken password. In
order to make asscess to old data possible add retry with old-style
password.

Reviewed-by: Richard Levitte <levitte@openssl.org>

.gitattributes

@@ -1,2 +1,3 @@
 *.der binary
 /fuzz/corpora/** binary
+*.pfx binary

crypto/pkcs12/p12_crpt.c

@@ -17,6 +17,16 @@ 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)
@@ -25,6 +35,19 @@ int PKCS12_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
     int saltlen, iter, ret;
     unsigned char *salt;
     unsigned char key[EVP_MAX_KEY_LENGTH], iv[EVP_MAX_IV_LENGTH];
+    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);
+
+    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;
 
     if (cipher == NULL)
         return 0;
@@ -43,14 +66,14 @@ int PKCS12_PBE_keyivgen(EVP_CIPHER_CTX *ctx, const char *pass, int passlen,
         iter = ASN1_INTEGER_get(pbe->iter);
     salt = pbe->salt->data;
     saltlen = pbe->salt->length;
-    if (!PKCS12_key_gen(pass, passlen, salt, saltlen, PKCS12_KEY_ID,
+    if (!(*pkcs12_key_gen)(pass, passlen, salt, saltlen, PKCS12_KEY_ID,
-                        iter, EVP_CIPHER_key_length(cipher), key, md)) {
+                           iter, EVP_CIPHER_key_length(cipher), key, md)) {
         PKCS12err(PKCS12_F_PKCS12_PBE_KEYIVGEN, PKCS12_R_KEY_GEN_ERROR);
         PBEPARAM_free(pbe);
         return 0;
     }
-    if (!PKCS12_key_gen(pass, passlen, salt, saltlen, PKCS12_IV_ID,
+    if (!(*pkcs12_key_gen)(pass, passlen, salt, saltlen, PKCS12_IV_ID,
-                        iter, EVP_CIPHER_iv_length(cipher), iv, md)) {
+                           iter, EVP_CIPHER_iv_length(cipher), iv, md)) {
         PKCS12err(PKCS12_F_PKCS12_PBE_KEYIVGEN, PKCS12_R_IV_GEN_ERROR);
         PBEPARAM_free(pbe);
         return 0;

crypto/pkcs12/p12_lcl.h

@@ -42,3 +42,12 @@ struct pkcs12_bag_st {
     } 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);

crypto/pkcs12/p12_mutl.c

@@ -66,9 +66,40 @@ 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 */
-int PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
+static int pkcs12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
-                   unsigned char *mac, unsigned int *maclen)
+                          unsigned char *mac, unsigned int *maclen,
+                          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))
 {
     const EVP_MD *md_type;
     HMAC_CTX *hmac = NULL;
@@ -79,6 +110,11 @@ 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;
+
     if (!PKCS7_type_is_data(p12->authsafes)) {
         PKCS12err(PKCS12_F_PKCS12_GEN_MAC, PKCS12_R_CONTENT_TYPE_NOT_DATA);
         return 0;
@@ -111,8 +147,8 @@ int PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
             return 0;
         }
     } else
-        if (!PKCS12_key_gen(pass, passlen, salt, saltlen, PKCS12_MAC_ID, iter,
+        if (!(*pkcs12_key_gen)(pass, passlen, salt, saltlen, PKCS12_MAC_ID,
-                            md_size, key, md_type)) {
+                               iter, md_size, key, md_type)) {
         PKCS12err(PKCS12_F_PKCS12_GEN_MAC, PKCS12_R_KEY_GEN_ERROR);
         return 0;
     }
@@ -128,6 +164,12 @@ int PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
     return 1;
 }
 
+int PKCS12_gen_mac(PKCS12 *p12, const char *pass, int passlen,
+                   unsigned char *mac, unsigned int *maclen)
+{
+    return pkcs12_gen_mac(p12, pass, passlen, mac, maclen, NULL);
+}
+
 /* Verify the mac */
 int PKCS12_verify_mac(PKCS12 *p12, const char *pass, int passlen)
 {
@@ -139,14 +181,36 @@ int PKCS12_verify_mac(PKCS12 *p12, const char *pass, int passlen)
         PKCS12err(PKCS12_F_PKCS12_VERIFY_MAC, PKCS12_R_MAC_ABSENT);
         return 0;
     }
-    if (!PKCS12_gen_mac(p12, pass, passlen, mac, &maclen)) {
+    if (!pkcs12_gen_mac(p12, pass, passlen, mac, &maclen,
+                        PKCS12_key_gen_utf8)) {
         PKCS12err(PKCS12_F_PKCS12_VERIFY_MAC, PKCS12_R_MAC_GENERATION_ERROR);
         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))
         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;
 }
 
@@ -166,7 +230,11 @@ int PKCS12_set_mac(PKCS12 *p12, const char *pass, int passlen,
         PKCS12err(PKCS12_F_PKCS12_SET_MAC, PKCS12_R_MAC_SETUP_ERROR);
         return 0;
     }
-    if (!PKCS12_gen_mac(p12, pass, passlen, mac, &maclen)) {
+    /*
+     * Note that output mac is forced to UTF-8...
+     */
+    if (!pkcs12_gen_mac(p12, pass, passlen, mac, &maclen,
+                        PKCS12_key_gen_utf8)) {
         PKCS12err(PKCS12_F_PKCS12_SET_MAC, PKCS12_R_MAC_GENERATION_ERROR);
         return 0;
     }

crypto/pkcs12/p12_utl.c

@@ -173,11 +173,16 @@ char *OPENSSL_uni2utf8(const unsigned char *uni, int unilen)
     int asclen, i, j;
     char *asctmp;
 
+    /* string must contain an even number of bytes */
+    if (unilen & 1)
+        return NULL;
+
     for (asclen = 0, i = 0; i < unilen; ) {
         j = bmp_to_utf8(NULL, uni+i, unilen-i);
         /*
-         * falling back to OPENSSL_uni2asc makes lesser sense, it's
+         * falling back to OPENSSL_uni2asc makes lesser sense [than
-         * done rather to maintain symmetry...
+         * falling back to OPENSSL_asc2uni in OPENSSL_utf82uni above],
+         * it's done rather to maintain symmetry...
          */
         if (j < 0) return OPENSSL_uni2asc(uni, unilen);
         if (j == 4) i += 4;

doc/apps/pkcs12.pod

@@ -325,6 +325,16 @@ encrypted private keys, then the option B<-keypbe PBE-SHA1-RC2-40> can
 be used to reduce the private key encryption to 40 bit RC2. A complete
 description of all algorithms is contained in the B<pkcs8> manual page.
 
+Prior 1.1 release passwords containing non-ASCII characters were encoded
+in non-compliant manner, which limited interoperability, in first hand
+with Windows. But switching to standard-compliant password encoding
+poses problem accessing old data protected with broken encoding. For
+this reason even legacy encodings is attempted when reading the
+data. If you use PKCS#12 files in production application you are advised
+to convert the data, because implemented heuristic approach is not
+MT-safe, its sole goal is to facilitate the data upgrade with this
+utility.
+
 =head1 EXAMPLES
 
 Parse a PKCS#12 file and output it to a file:

include/openssl/pkcs12.h

@@ -30,19 +30,9 @@ extern "C" {
 
 # define PKCS12_SALT_LEN 8
 
-/* Uncomment out next line for unicode password and names, otherwise ASCII */
+/* It's not clear if these are actually needed... */
-
+# define PKCS12_key_gen PKCS12_key_gen_utf8
-/*
+# define PKCS12_add_friendlyname PKCS12_add_friendlyname_utf8
- * #define PBE_UNICODE
- */
-
-# ifdef PBE_UNICODE
-#  define PKCS12_key_gen PKCS12_key_gen_uni
-#  define PKCS12_add_friendlyname PKCS12_add_friendlyname_uni
-# else
-#  define PKCS12_key_gen PKCS12_key_gen_utf8
-#  define PKCS12_add_friendlyname PKCS12_add_friendlyname_utf8
-# endif
 
 /* MS key usage constants */
 

test/recipes/80-test_pkcs12.t

@@ -20,6 +20,17 @@ if (eval { require Win32::Console; 1; }) {
     $savedcp = Win32::Console::OutputCP();
     Win32::Console::OutputCP(1253);
     $pass = Encode::encode("cp1253",Encode::decode("utf-8",$pass));
+} else {
+    # Running MinGW tests transparenly under Wine apparently requires
+    # UTF-8 locale...
+
+    foreach(`locale -a`) {
+        s/\R$//;
+        if ($_ =~ m/^C\.UTF\-?8/i) {
+            $ENV{LC_ALL} = $_;
+            last;
+        }
+    }
 }
 
 # just see that we can read shibboleth.pfx protected with $pass