The branch openssl-3.0 has been updated via be44d58e80fdab137aef73d610c165acc991fcf9 (commit) via c7eba968f4f070077f9638884abff68d0c161aac (commit) via daf0b77e34468696158e7ec92d636f8dbb578ffd (commit) via 7e1fba44e0c2e5a2f6fb86bcabe5c2b160f678ca (commit) via cb6f2f179dc5b4cb00250222e28cd52a858e9562 (commit) from e1436d54b9de5012d1716212c7329e46cf21a24a (commit)
- Log ----------------------------------------------------------------- commit be44d58e80fdab137aef73d610c165acc991fcf9 Author: Tomas Mraz <to...@openssl.org> Date: Wed Dec 29 09:26:58 2021 +0100 try_pkcs12(): cleanse passphrase so it is not left on the stack Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/17320) (cherry picked from commit da7db7ae6d7d1929893a58e41335c88e472fc364) commit c7eba968f4f070077f9638884abff68d0c161aac Author: Tomas Mraz <to...@openssl.org> Date: Tue Dec 28 12:46:31 2021 +0100 try_pkcs12(): Correct handling of NUL termination of passphrases Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/17320) (cherry picked from commit 1dfef929e43ebfa3a7f1108317f75747f92effb6) commit daf0b77e34468696158e7ec92d636f8dbb578ffd Author: Tomas Mraz <to...@openssl.org> Date: Tue Dec 21 16:05:52 2021 +0100 Test that PEM_BUFSIZE is passed into pem_password_cb When pem_password_cb is used from SSL_CTX, its size parameter should be equal to PEM_BUFSIZE. Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/17320) (cherry picked from commit c7debe811123951a60cdfe73716184ca8fdd79d2) commit 7e1fba44e0c2e5a2f6fb86bcabe5c2b160f678ca Author: Tomas Mraz <to...@openssl.org> Date: Tue Dec 21 15:58:44 2021 +0100 pem_password_cb: Clarify the documentation on passphrases Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/17320) (cherry picked from commit 5b5342e04ff24d5138c054c1677c32729d47e938) commit cb6f2f179dc5b4cb00250222e28cd52a858e9562 Author: Tomas Mraz <to...@openssl.org> Date: Tue Dec 21 12:26:05 2021 +0100 Compensate for UI method always adding NUL termination The UI method always adds NUL termination and we need to compensate for that when using it from a pem_password_cb because the buffer used in pem_password_cb does not account for that and the returned password should be able fill the whole buffer. Fixes #16601 Reviewed-by: Ben Kaduk <ka...@mit.edu> (Merged from https://github.com/openssl/openssl/pull/17320) (cherry picked from commit ef65bbb96352650bf9ce4ff46c60c71d9f138d08) ----------------------------------------------------------------------- Summary of changes: crypto/passphrase.c | 33 ++++++++++++++++++++++++--------- crypto/store/store_result.c | 19 +++++++++++++------ doc/man3/PEM_read_bio_PrivateKey.pod | 5 +++-- test/certs/leaf-encrypted.key | 30 ++++++++++++++++++++++++++++++ test/sslapitest.c | 14 +++++++++++++- 5 files changed, 83 insertions(+), 18 deletions(-) create mode 100644 test/certs/leaf-encrypted.key diff --git a/crypto/passphrase.c b/crypto/passphrase.c index d61e249440..cb1bc66958 100644 --- a/crypto/passphrase.c +++ b/crypto/passphrase.c @@ -109,7 +109,8 @@ int ossl_pw_disable_passphrase_caching(struct ossl_passphrase_data_st *data) * UI_METHOD processor. It differs from UI_UTIL_read_pw() like this: * * 1. It constructs a prompt on its own, based on |prompt_info|. - * 2. It allocates a buffer for verification on its own. + * 2. It allocates a buffer for password and verification on its own + * to compensate for NUL terminator in UI password strings. * 3. It raises errors. * 4. It reports back the length of the prompted pass phrase. */ @@ -117,8 +118,8 @@ static int do_ui_passphrase(char *pass, size_t pass_size, size_t *pass_len, const char *prompt_info, int verify, const UI_METHOD *ui_method, void *ui_data) { - char *prompt = NULL, *vpass = NULL; - int prompt_idx = -1, verify_idx = -1; + char *prompt = NULL, *ipass = NULL, *vpass = NULL; + int prompt_idx = -1, verify_idx = -1, res; UI *ui = NULL; int ret = 0; @@ -145,9 +146,16 @@ static int do_ui_passphrase(char *pass, size_t pass_size, size_t *pass_len, goto end; } + /* Get a buffer for verification prompt */ + ipass = OPENSSL_zalloc(pass_size + 1); + if (ipass == NULL) { + ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE); + goto end; + } + prompt_idx = UI_add_input_string(ui, prompt, UI_INPUT_FLAG_DEFAULT_PWD, - pass, 0, pass_size - 1) - 1; + ipass, 0, pass_size) - 1; if (prompt_idx < 0) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_UI_LIB); goto end; @@ -155,15 +163,15 @@ static int do_ui_passphrase(char *pass, size_t pass_size, size_t *pass_len, if (verify) { /* Get a buffer for verification prompt */ - vpass = OPENSSL_zalloc(pass_size); + vpass = OPENSSL_zalloc(pass_size + 1); if (vpass == NULL) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE); goto end; } verify_idx = UI_add_verify_string(ui, prompt, UI_INPUT_FLAG_DEFAULT_PWD, - vpass, 0, pass_size - 1, - pass) - 1; + vpass, 0, pass_size, + ipass) - 1; if (verify_idx < 0) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_UI_LIB); goto end; @@ -178,13 +186,20 @@ static int do_ui_passphrase(char *pass, size_t pass_size, size_t *pass_len, ERR_raise(ERR_LIB_CRYPTO, ERR_R_UI_LIB); break; default: - *pass_len = (size_t)UI_get_result_length(ui, prompt_idx); + res = UI_get_result_length(ui, prompt_idx); + if (res < 0) { + ERR_raise(ERR_LIB_CRYPTO, ERR_R_UI_LIB); + break; + } + *pass_len = (size_t)res; + memcpy(pass, ipass, *pass_len); ret = 1; break; } end: - OPENSSL_free(vpass); + OPENSSL_clear_free(vpass, pass_size + 1); + OPENSSL_clear_free(ipass, pass_size + 1); OPENSSL_free(prompt); UI_free(ui); return ret; diff --git a/crypto/store/store_result.c b/crypto/store/store_result.c index 3a0dc9dfba..f705ac3d69 100644 --- a/crypto/store/store_result.c +++ b/crypto/store/store_result.c @@ -525,7 +525,7 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v, if (p12 != NULL) { char *pass = NULL; - char tpass[PEM_BUFSIZE]; + char tpass[PEM_BUFSIZE + 1]; size_t tpass_len; EVP_PKEY *pkey = NULL; X509 *cert = NULL; @@ -547,17 +547,23 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v, OSSL_PARAM_END }; - if (!ossl_pw_get_passphrase(tpass, sizeof(tpass), &tpass_len, + if (!ossl_pw_get_passphrase(tpass, sizeof(tpass) - 1, + &tpass_len, pw_params, 0, &ctx->pwdata)) { ERR_raise(ERR_LIB_OSSL_STORE, OSSL_STORE_R_PASSPHRASE_CALLBACK_ERROR); goto p12_end; } pass = tpass; - if (!PKCS12_verify_mac(p12, pass, strlen(pass))) { + /* + * ossl_pw_get_passphrase() does not NUL terminate but + * we must do it for PKCS12_parse() + */ + pass[tpass_len] = '\0'; + if (!PKCS12_verify_mac(p12, pass, tpass_len)) { ERR_raise_data(ERR_LIB_OSSL_STORE, OSSL_STORE_R_ERROR_VERIFYING_PKCS12_MAC, - strlen(pass) == 0 ? "empty password" : + tpass_len == 0 ? "empty password" : "maybe wrong password"); goto p12_end; } @@ -613,9 +619,10 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v, } ctx->cached_info = infos; } + p12_end: + OPENSSL_cleanse(tpass, sizeof(tpass)); + PKCS12_free(p12); } - p12_end: - PKCS12_free(p12); *v = sk_OSSL_STORE_INFO_shift(ctx->cached_info); } diff --git a/doc/man3/PEM_read_bio_PrivateKey.pod b/doc/man3/PEM_read_bio_PrivateKey.pod index a71907b170..27e7849ef7 100644 --- a/doc/man3/PEM_read_bio_PrivateKey.pod +++ b/doc/man3/PEM_read_bio_PrivateKey.pod @@ -335,7 +335,7 @@ I<klen> bytes at I<kstr> are used as the passphrase and I<cb> is ignored. If the I<cb> parameters is set to NULL and the I<u> parameter is not -NULL then the I<u> parameter is interpreted as a null terminated string +NULL then the I<u> parameter is interpreted as a NUL terminated string to use as the passphrase. If both I<cb> and I<u> are NULL then the default callback routine is used which will typically prompt for the passphrase on the current terminal with echoing turned off. @@ -355,7 +355,8 @@ value as the I<u> parameter passed to the PEM routine. It allows arbitrary data to be passed to the callback by the application (for example a window handle in a GUI application). The callback I<must> return the number of characters in the passphrase or -1 if -an error occurred. +an error occurred. The passphrase can be arbitrary data; in the case where it +is a string, it is not NUL terminated. See the L</EXAMPLES> section below. Some implementations may need to use cryptographic algorithms during their operation. If this is the case and I<libctx> and I<propq> parameters have been diff --git a/test/certs/leaf-encrypted.key b/test/certs/leaf-encrypted.key new file mode 100644 index 0000000000..99a802dbe6 --- /dev/null +++ b/test/certs/leaf-encrypted.key @@ -0,0 +1,30 @@ +-----BEGIN ENCRYPTED PRIVATE KEY----- +MIIFLTBXBgkqhkiG9w0BBQ0wSjApBgkqhkiG9w0BBQwwHAQIEBBNanZFjs8CAggA +MAwGCCqGSIb3DQIJBQAwHQYJYIZIAWUDBAECBBDr8bhquxPf762O3jk0LAtJBIIE +0FQB7c06dpoHn1KBn8jTzsLIdVR0SeKUvq4edZfUPbB/6go97j48BwSzAaXY7BNL +90GRMrNNjKZDLeNf0wwf1+67YX7neGnb+LdxpQdqEjOTGQdwTx9SG6XIqT8x4R67 +rI2DQqI937FSor9292koXQNM9Asoenn6kOCITaa8chsPdKCtFjfVmqZRMaewr5PW +W1rooFuCVAIfgBOOaEeN7OMTJRdAGtWWOJqyLB29gXxwaI1+PnXmkHPgRGXZYz7W +N5lTp1xvFPY+Rp/cK0DfeR5MrMYSVvrXbi6usjteJ2h0Rzcy8SY6Jnvuaoowi+rj +lDUP0K/51tTQBd6bpsvcmc2cBx+7pg4BAkf9SnuKQpYCWPjiwrCiDJIP/o5GYIn2 +m/3K2pLahjOeGZAmhGUi0fZPZhaq37IQKwuzLDuYw1CkR7LhaJcJ9V1vXMPePgCY ++BvjFG5z0mLDwUNvzCHQokav3Z/QT6CfOgTL10qKuBgylT1d5Cw7bfv8Lnc6C/YK +aVXosCaKTJO8r4t7NgJX4PYQP/DZIl5CJIoUzJkrAkShLwcGtXMHVNSWx4LS60QY +lfjz80cWWE6Tx/XjBkae0AQJW8S9nDB8/X80ox8jJ/sdd5XNZqUQhDxBP5/4GiAS +pZlgp/IwssoG5HUnwn/4AUgD7Gdo5QRqFlkXeCFlHgjBrEHBkevHECRHAdWwrK7X +5td662K1B9hm6EfA1R51jiOKBuM0bwYtI+tpmpT5zeDGeaOWuPUYPUFjfo9xt1Lx +cmX3ouBt34uT/cQesPxP8gJwRdo0KqPK+KLjtQazXmHFu+FStZ29gUvhqAw9kcxq +ps9neGAl3DJgYbB1QqqefGqFWBhJzt4toqxcgm6Z0PJSYQlxJEC3yWWs5w5wfLJJ +KGfnpsY1IGYsbw9Caa84XqnzHosGWx724GJeb3YSwwMj311oMi9s8J/d/NpJZHOu +uk/mQWezCfdEFSnkOtIDJWTQUtRtRfIZQp243c25E3/rJySuSoMfn4eolAGurse8 +6r7SEJ6MUjCTd3ZcA+XZAtFxPQnNBYm691hvGE6uclxYy9L6bmws9dosNlpCyvIQ ++OYdB9Mvx9hs0KwAWZ6bnIxa3tc6Ob9mxV7ycMS43d4ShEqzy44DZD02Z0iQIRym +1AoGwgLbc2d9NouUiw2ur5n6ByYCTHwmMSAstVovuBoS2XDF23BzLL7KuCnkHH0y ++M6CRaXW0ceTP4DfEvBphxfj4NNEZpjm8j6ERvnnQvC5tRAaMglhg1WOvUVUtPg5 +cJPIiSn+yVuoFDnLKJ53N9NzDtUKSBQgwNGyVVPTzpfxLmjg00bNQ7eyoRr6uK0l +ezmHemo52JpCaBGV01tnvVKzGouFN/KxP9GxvPQY8UQxVkE+E/p0UjGOpNLIDmzl +/qVKxky9lMBoHc+neeCbOrtgwkyYgpPkKlmTTsi/yUxpbUmobFZJTUbOWrpeRbw3 +Pt9u8NeVmD4Ys/NenHIJwksOqmWxSy7IjJpzQsee1CZXV7McAYsg24tP4Bdj9aGT +hsMyiaiNB+rjkNxhUCm39nJsaN1AoTZ3Br1UYfHrfocif12yNGOEBy2swfjQIGNH +fjGk3px34MZZv3S0bM/ZPi9ankzAZnf8qkHoDVtsP+Gk +-----END ENCRYPTED PRIVATE KEY----- diff --git a/test/sslapitest.c b/test/sslapitest.c index 041bc5f210..9056fa28f1 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -672,15 +672,27 @@ end: return ret; } +static int get_password_cb(char *buf, int size, int rw_flag, void *userdata) +{ + static const char pass[] = "testpass"; + + if (!TEST_int_eq(size, PEM_BUFSIZE)) + return -1; + + memcpy(buf, pass, sizeof(pass) - 1); + return sizeof(pass) - 1; +} + static int test_ssl_ctx_build_cert_chain(void) { int ret = 0; SSL_CTX *ctx = NULL; - char *skey = test_mk_file_path(certsdir, "leaf.key"); + char *skey = test_mk_file_path(certsdir, "leaf-encrypted.key"); char *leaf_chain = test_mk_file_path(certsdir, "leaf-chain.pem"); if (!TEST_ptr(ctx = SSL_CTX_new_ex(libctx, NULL, TLS_server_method()))) goto end; + SSL_CTX_set_default_passwd_cb(ctx, get_password_cb); /* leaf_chain contains leaf + subinterCA + interCA + rootCA */ if (!TEST_int_eq(SSL_CTX_use_certificate_chain_file(ctx, leaf_chain), 1) || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(ctx, skey,