From d8c9ca49e10b6404fed74001d3f6378fec39dad2 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 5 May 2022 16:11:21 +0200 Subject: [PATCH] Be user-friendly when opening a pfx/p12 file Before this change the user had to provide a password callback, even if the PKCS#12 encoded file has no or an empty password. This changes the behavior, so we first try to open the file without a password and only then ask the user to provide one. This fixes #204. Signed-off-by: Steffen Jaeckel --- Makefile.am | 2 ++ src/tls_gnutls.c | 21 +++++++++++++++++---- src/tls_openssl.c | 37 ++++++++++++++++++++++++++----------- tests/cert.emptypass.pfx | Bin 0 -> 1132 bytes tests/cert.nopass.pfx | Bin 0 -> 1031 bytes tests/test_xmppaddr.c | 18 ++++++++++++------ 6 files changed, 57 insertions(+), 21 deletions(-) create mode 100644 tests/cert.emptypass.pfx create mode 100644 tests/cert.nopass.pfx diff --git a/Makefile.am b/Makefile.am index c57dd38..58c1d0f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,6 +116,8 @@ EXTRA_DIST = \ src/tls_schannel.c \ tests/cert.pem \ tests/cert.pfx \ + tests/cert.emptypass.pfx \ + tests/cert.nopass.pfx \ tests/key.pem \ tests/key_encrypted.pem \ tests/res_query_dump.c diff --git a/src/tls_gnutls.c b/src/tls_gnutls.c index ac3d027..5b14ca6 100644 --- a/src/tls_gnutls.c +++ b/src/tls_gnutls.c @@ -93,10 +93,6 @@ static gnutls_x509_crt_t _tls_load_cert_p12(xmpp_conn_t *conn) gnutls_x509_privkey_t key; unsigned int cert_num = 0, retries = 0; int err = -1; - if (!conn->password_callback) { - strophe_error(conn->ctx, "tls", "No password callback set"); - return NULL; - } if (gnutls_pkcs12_init(&p12) < 0) return NULL; if (gnutls_load_file(conn->tls_client_key, &data) < 0) @@ -104,6 +100,22 @@ static gnutls_x509_crt_t _tls_load_cert_p12(xmpp_conn_t *conn) if (gnutls_pkcs12_import(p12, &data, GNUTLS_X509_FMT_DER, 0) < 0) goto error_out2; + /* First try to open file with no pass */ + if ((err = gnutls_pkcs12_simple_parse(p12, NULL, &key, &cert, &cert_num, + NULL, NULL, NULL, 0)) == 0) { + goto done; + } + /* Now let's try to open file with an empty pass */ + if ((err = gnutls_pkcs12_simple_parse(p12, "", &key, &cert, &cert_num, NULL, + NULL, NULL, 0)) == 0) { + goto done; + } + + if (!conn->password_callback) { + strophe_error(conn->ctx, "tls", "No password callback set"); + goto error_out2; + } + /* ... and only now ask the user for a password */ while (retries++ < conn->password_retries) { char pass[GNUTLS_PKCS11_MAX_PIN_LEN]; int passlen = @@ -125,6 +137,7 @@ static gnutls_x509_crt_t _tls_load_cert_p12(xmpp_conn_t *conn) strophe_debug(conn->ctx, "tls", "wrong password?"); } +done: gnutls_pkcs12_deinit(p12); gnutls_free(data.data); if (err < 0) diff --git a/src/tls_openssl.c b/src/tls_openssl.c index 4a52947..097b666 100644 --- a/src/tls_openssl.c +++ b/src/tls_openssl.c @@ -948,6 +948,25 @@ static X509 *_tls_cert_read_x509(xmpp_conn_t *conn) return c; } +static int _tls_parse_p12(PKCS12 *p12, + const char *pass, + EVP_PKEY **pkey, + X509 **cert, + STACK_OF(X509) * *ca) +{ + /* For some reason `PKCS12_parse()` fails without a `EVP_PKEY` + * so if the user doesn't want it, use a local one and free it + * again directly after parsing. + */ + EVP_PKEY *pkey_; + if (!pkey) + pkey = &pkey_; + int parse_ok = PKCS12_parse(p12, pass, pkey, cert, ca); + if (pkey == &pkey_ && pkey_) + EVP_PKEY_free(pkey_); + return parse_ok; +} + static X509 * _tls_cert_read_p12(xmpp_conn_t *conn, EVP_PKEY **pkey, STACK_OF(X509) * *ca) { @@ -967,6 +986,12 @@ _tls_cert_read_p12(xmpp_conn_t *conn, EVP_PKEY **pkey, STACK_OF(X509) * *ca) goto error_out; } + /* First try to open file w/o a pass */ + if (_tls_parse_p12(p12, NULL, pkey, &cert, ca)) { + goto success; + } + cert = NULL; + unsigned int retries = 0; pem_password_cb *cb = PEM_def_callback; @@ -981,17 +1006,7 @@ _tls_cert_read_p12(xmpp_conn_t *conn, EVP_PKEY **pkey, STACK_OF(X509) * *ca) int passlen = cb(pass, PEM_BUFSIZE, 0, userdata); if (passlen < 0 || passlen > PEM_BUFSIZE) goto error_out; - - /* For some reason `PKCS12_parse()` fails without a `EVP_PKEY` - * so if the user doesn't want it, use a local one and free it - * again directly after parsing. - */ - EVP_PKEY *pkey_; - if (!pkey) - pkey = &pkey_; - int parse_ok = PKCS12_parse(p12, pass, pkey, &cert, ca); - if (pkey == &pkey_ && pkey_) - EVP_PKEY_free(pkey_); + int parse_ok = _tls_parse_p12(p12, pass, pkey, &cert, ca); if (parse_ok) { goto success; } diff --git a/tests/cert.emptypass.pfx b/tests/cert.emptypass.pfx new file mode 100644 index 0000000000000000000000000000000000000000..b556eeebbf08fb8dd3c2753825931de7b8b13b21 GIT binary patch literal 1132 zcmXqLV##1)WHxAG(PQJ(YV&CO&dbQoxS)weo~4OJ8YnDo(8R2ckYZcV#4G_65@BLw z08+dN8AbycHZG_MJdA7xd@LN>*B??7QG5S|iHU=up^51M-*5A?ee0h^|9&L&=feDF zLKk#%5)~QGUDix$dHZ@b$IW-PM{i9y-=6+G%ika@P4DZBgSs!?2+4h5RpV!n{Ji+v zjqnqP*=Id*w-2gVKZXBs?hmuSX?G`@o!HK|^>ekQg1zmV=V#BqI}xO|GcxjU>uuQ# zZ}rvBH&v9*IPdnM~hs~aezn*`}R_fQ;&EJaz#J4yW@e6%-Qr^46uQ;^v>y)Mg zwR+bpPo=P_sOk2*=`Y?Su+=hk+q$2B^?oTlm=wM_J?+?^iw_kz&t4L2UYjgqR@0<3 zW6I2B6PEnh@K7_?C_z^wx0OS$GA&4P?vAhRc{=y)Oq4zsoe{5};Bla?&tLPi?5!Es zk2tHn&oC(8#=p0?(eB1<>8ocG9M!VgzHVQbT%Z{6@66*$Co{%eu6MU9XYP1yq%8M> zeSOgk{X=9`1a# zqxtfhQ;ZARV@sxp{@=&Hw50s5j*8m4=?~TNPHviE$mKZxz`<36ZS05 zGA^L!z|PgY3xnrhXI~T*leOTL=efqFWPO9p4mS>--68$Pd$HhFp4-zRn(geC*>95y zUvtLL!~5~kzlNVGJbK;dKH$9WuqI4d)o29+)v#7zd&(G#Q zSQYrZl07j~vFa&LbX<GS6(2mE)n(|)^EEf#_2iBD zYIb`Pe$VckdLcSj`o@}^gDG1+ZMvMncQ*0G^Gzp3XH;!?v3Twkg}!p;BfIn%ofWmu zWe2@{vG?w@hLRNKsR7F@XHC>hHvfBty(rG4%zW}%o7+zwZs)9=`y}T3ir|*@wfC-B zfBa{tYM>1F1*fQ?7>h_e*Vf3IMSHui8+O0(tKO0y>lwe>z|cU^fRl|?n~#}Eij{#y nWOm+;>;11MR5wwr~0nk1sCXVRYJHt7@8T~cj zV&ecRK4%Mbhk>`Dr-3^gb0`Zlk3(5%QKeo^eqOp>X>n>%USe*lLvg-dNk*z(VsW0K zUQTLWdP#;}X0cwPURh#JW(rU^IWto)IU}(sF}WnQ$Us4y*T~So*uc=x($LVrAWDMY z$Ph(xtR1D?VK?t>YQDAU(0>>_%~NFmvuK@ zl%1yG?h?&pX6QK#K>pR#K>j92@DEZVHPF>1_N0TlaED=MdZh6 zX~mDLf~U?c3i#TPJ@3^$>qCzW+Ggm-ai)-KbuK{BgOf;q{-JKQFr6|!`Z~nFT7JK?h_!YY^Z9W3{T0NqK3f0 zjOW@KS+i(w_jSYWH-6Py@?$;YcN;YBgR25&g~kmmjcW}WR~a-eM^5XE3knVLnHU+c z#yU%=K`;{|Ba1}brHxxIS=rqFzVm4D7U}KXO2u!})BIcf7MhTgFo^Ylp@AYW?XYU| vF*8ZAGO&m|$~v-ho&UN8rQ&v_k6s%cn#63dnT2CFPx*t7mmTbYnUMhiD~U`r literal 0 HcmV?d00001 diff --git a/tests/test_xmppaddr.c b/tests/test_xmppaddr.c index 485004f..7d542b5 100644 --- a/tests/test_xmppaddr.c +++ b/tests/test_xmppaddr.c @@ -33,10 +33,15 @@ int main() xmpp_ctx_t *ctx; xmpp_conn_t *conn; xmpp_log_t *log; - char *client_cert[][2] = { - {"tests/cert.pem", "tests/key.pem"}, - {"tests/cert.pem", "tests/key_encrypted.pem"}, - {NULL, "tests/cert.pfx"}, + struct { + int needs_callback; + char *pem, *key; + } client_cert[] = { + {0, "tests/cert.pem", "tests/key.pem"}, + {1, "tests/cert.pem", "tests/key_encrypted.pem"}, + {0, NULL, "tests/cert.emptypass.pfx"}, + {0, NULL, "tests/cert.nopass.pfx"}, + {1, NULL, "tests/cert.pfx"}, }; char xmppaddr_num[] = "0"; @@ -49,9 +54,10 @@ int main() for (m = 0; m < sizeof(client_cert) / sizeof(client_cert[0]); ++m) { conn = xmpp_conn_new(ctx); - xmpp_conn_set_password_callback(conn, password_callback, NULL); + if (client_cert[m].needs_callback) + xmpp_conn_set_password_callback(conn, password_callback, NULL); - xmpp_conn_set_client_cert(conn, client_cert[m][0], client_cert[m][1]); + xmpp_conn_set_client_cert(conn, client_cert[m].pem, client_cert[m].key); xmppaddr_num[0] = '0' + xmpp_conn_cert_xmppaddr_num(conn);