Skip to content

Commit

Permalink
Push password encoding back into pkcs12_key_gen.
Browse files Browse the repository at this point in the history
With PKCS8_encrypt_pbe and PKCS8_decrypt_pbe gone in
3e8b782, we can restore the old
arrangement where the password encoding was handled in pkcs12_key_gen.
This simplifies the interface for the follow-up crypto/asn1 split.

Note this change is *not* a no-op for PKCS#12 files which use PBES2.
Before, we would perform the PKCS#12 password encoding for all parts of
PKCS#12 processing. The new behavior is we only perform it for the parts
that go through the PKCS#12 KDF. For such a file, it would only be the
MAC.

I believe the specification supports our new behavior. Although RFC 7292
B.1 says something which implies that the transformation is about
converting passwords to byte strings and would thus be universal,
appendix B itself is prefaced with:

   Note that this method for password privacy mode is not recommended
   and is deprecated for new usage.  The procedures and algorithms
   defined in PKCS google#5 v2.1 [13] [22] should be used instead.
   Specifically, PBES2 should be used as encryption scheme, with PBKDF2
   as the key derivation function.

"This method" refers to the key derivation and not the password
formatting, but it does give support to the theory that password
formatting is tied to PKCS#12 key derivation.

(Of course, if one believes PKCS#12's assertion that their inane
encoding (NUL-terminated UTF-16!) is because PKCS#5 failed to talk about
passwords as Unicode strings, one would think that PBES2 (also in
PKCS#5) would have the same issue and thus need PKCS#12 to valiantly
save the day with an encoding...)

This matches OpenSSL's behavior and that of recent versions of NSS. See
https://bugzilla.mozilla.org/show_bug.cgi?id=1268141. I was unable to
figure out what variants, if any, macOS accepts.

BUG=54

Change-Id: I9a1bb4d5e168e6e76b82241e4634b1103e620b9b
Reviewed-on: https://boringssl-review.googlesource.com/14213
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
  • Loading branch information
davidben authored and CQ bot account: commit-bot@chromium.org committed Mar 25, 2017
1 parent 3cb047e commit 8cd7bbf
Show file tree
Hide file tree
Showing 4 changed files with 308 additions and 161 deletions.
14 changes: 5 additions & 9 deletions crypto/pkcs8/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,37 +63,33 @@ extern "C" {
#endif


#define PBE_UCS2_CONVERT_PASSWORD 0x1

struct pbe_suite {
int pbe_nid;
uint8_t oid[10];
uint8_t oid_len;
const EVP_CIPHER *(*cipher_func)(void);
const EVP_MD *(*md_func)(void);
/* decrypt_init initialize |ctx| for decrypting. The password is specified by
* |pass_raw| and |pass_raw_len|. |param| contains the serialized parameters
* field of the AlgorithmIdentifier.
* |pass| and |pass_len|. |param| contains the serialized parameters field of
* the AlgorithmIdentifier.
*
* It returns one on success and zero on error. */
int (*decrypt_init)(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx,
const uint8_t *pass_raw, size_t pass_raw_len, CBS *param);
int flags;
const char *pass, size_t pass_len, CBS *param);
};

#define PKCS5_DEFAULT_ITERATIONS 2048
#define PKCS5_SALT_LEN 8

int PKCS5_pbe2_decrypt_init(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx,
const uint8_t *pass_raw, size_t pass_raw_len,
CBS *param);
const char *pass, size_t pass_len, CBS *param);

/* PKCS5_pbe2_encrypt_init configures |ctx| for encrypting with PKCS #5 PBES2,
* as defined in RFC 2998, with the specified parameters. It writes the
* corresponding AlgorithmIdentifier to |out|. */
int PKCS5_pbe2_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx,
const EVP_CIPHER *cipher, unsigned iterations,
const uint8_t *pass_raw, size_t pass_raw_len,
const char *pass, size_t pass_len,
const uint8_t *salt, size_t salt_len);


Expand Down
22 changes: 10 additions & 12 deletions crypto/pkcs8/p5_pbev2.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ static int add_cipher_oid(CBB *out, int nid) {
}

static int pkcs5_pbe2_cipher_init(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher,
unsigned iterations, const uint8_t *pass_raw,
size_t pass_raw_len, const uint8_t *salt,
unsigned iterations, const char *pass,
size_t pass_len, const uint8_t *salt,
size_t salt_len, const uint8_t *iv,
size_t iv_len, int enc) {
if (iv_len != EVP_CIPHER_iv_length(cipher)) {
Expand All @@ -150,8 +150,7 @@ static int pkcs5_pbe2_cipher_init(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher,
}

uint8_t key[EVP_MAX_KEY_LENGTH];
int ret = PKCS5_PBKDF2_HMAC_SHA1((const char *)pass_raw, pass_raw_len, salt,
salt_len, iterations,
int ret = PKCS5_PBKDF2_HMAC_SHA1(pass, pass_len, salt, salt_len, iterations,
EVP_CIPHER_key_length(cipher), key) &&
EVP_CipherInit_ex(ctx, cipher, NULL /* engine */, key, iv, enc);
OPENSSL_cleanse(key, EVP_MAX_KEY_LENGTH);
Expand All @@ -160,7 +159,7 @@ static int pkcs5_pbe2_cipher_init(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher,

int PKCS5_pbe2_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx,
const EVP_CIPHER *cipher, unsigned iterations,
const uint8_t *pass_raw, size_t pass_raw_len,
const char *pass, size_t pass_len,
const uint8_t *salt, size_t salt_len) {
int cipher_nid = EVP_CIPHER_nid(cipher);
if (cipher_nid == NID_undef) {
Expand Down Expand Up @@ -202,14 +201,13 @@ int PKCS5_pbe2_encrypt_init(CBB *out, EVP_CIPHER_CTX *ctx,
return 0;
}

return pkcs5_pbe2_cipher_init(ctx, cipher, iterations, pass_raw, pass_raw_len,
salt, salt_len, iv,
EVP_CIPHER_iv_length(cipher), 1 /* encrypt */);
return pkcs5_pbe2_cipher_init(ctx, cipher, iterations, pass, pass_len, salt,
salt_len, iv, EVP_CIPHER_iv_length(cipher),
1 /* encrypt */);
}

int PKCS5_pbe2_decrypt_init(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx,
const uint8_t *pass_raw, size_t pass_raw_len,
CBS *param) {
const char *pass, size_t pass_len, CBS *param) {
CBS pbe_param, kdf, kdf_obj, enc_scheme, enc_obj;
if (!CBS_get_asn1(param, &pbe_param, CBS_ASN1_SEQUENCE) ||
CBS_len(param) != 0 ||
Expand Down Expand Up @@ -303,7 +301,7 @@ int PKCS5_pbe2_decrypt_init(const struct pbe_suite *suite, EVP_CIPHER_CTX *ctx,
return 0;
}

return pkcs5_pbe2_cipher_init(ctx, cipher, (unsigned)iterations, pass_raw,
pass_raw_len, CBS_data(&salt), CBS_len(&salt),
return pkcs5_pbe2_cipher_init(ctx, cipher, (unsigned)iterations, pass,
pass_len, CBS_data(&salt), CBS_len(&salt),
CBS_data(&iv), CBS_len(&iv), 0 /* decrypt */);
}
Loading

0 comments on commit 8cd7bbf

Please sign in to comment.