From 580b0f3ef313a673bf299db36a1acd0ba6b05317 Mon Sep 17 00:00:00 2001 From: Nanang Izzuddin Date: Tue, 15 Dec 2020 10:25:13 +0700 Subject: [PATCH] TLS failed to load password-protected private key from buffer (#2606) Fix loading TLS cert from buffer with private key password-protected, adding logs in loading cert/key, updated expired TLS cert in SSL socket unit test. --- pjlib/build/cacert.der | Bin 568 -> 1336 bytes pjlib/build/cacert.pem | 40 ++++++++++++------ pjlib/build/privkey.p12 | Bin 1589 -> 4093 bytes pjlib/build/privkey.pem | 69 +++++++++++++++++++++++++------- pjlib/src/pj/ssl_sock_ossl.c | 62 +++++++++++++++++++++++++--- pjlib/src/pjlib-test/ssl_sock.c | 2 +- pjnath/src/pjnath-test/server.c | 2 +- 7 files changed, 140 insertions(+), 35 deletions(-) diff --git a/pjlib/build/cacert.der b/pjlib/build/cacert.der index ec2f1e7a2b8e80337d7120aef1c1d79df3147c27..255f0a1037ee2c280598a184e7e2efc11942d6ca 100644 GIT binary patch literal 1336 zcmXqLVl^>nVwPFJ%*4pVBvNtA_t1Q{xv%pxGNnFwyUsd&`rvj0UN%mxHjlRNyo`+8 ztPBPshC&7cY|No7%siY0S;d(JdO3+n26E!OMh1pPhK7bf($qLgoYxr0FtCJjap-Dd zQbIPKk(GhDiHV=Vpoxi#sfme^VR`n<@Pm!N&K|hweQ-+J@yQeCT3kFD^X03(n3K={ z`5XQ|X5O;#P{O)*yY??s+|*E^QdmE`eYcz2HRoJCXJgBSwIWvv$`|--Ox^mw`_0VY zKWEP!+@7=HkzK;hbO|=wPsBSNAdoiZ3vebiL+@4eTa!$3hz`lzH7ash+S5l|J^TE;kM6gQGr5&kr`UH~_*X|D5 z|GDaWev5R*w~Y_>J5(z2t#nNcn#j|}T@}NzQ#RE$wNkP8ylDLkXSTCdJLFm`_qn9SEW32USyZB9`#V{dYs_P2t zT+4GTt!MPj@)FHBIvQe*ESzt2L2zv{=b{I)jbWGd$|s#n>wkM>>yq+M|5|VIeTnMl zus$l#nmfbJaOs(|%k>yr)s3b1gzS1*a&N_#%&B`HuxDHQ%Cj7v?0<%}S^KS>GyesZ z^n6v#jEwbnuKV5KysLQekg%81>rIueD-K^3++cH1d+(oc*3NsAU745}85kGW8Ppia z0#lqUABz}^NZjf<^YS;J2rgasKF35NDt*Z**X;)KAZcY52?MbP>fi;{st6E7$c!;^9oB2~Au)H5(JBBxMb1^}i|MuzEYPG02`Xi%IGyJNpm zW${LB&W}9$e(ZsBEL>*umlQ7Axs9R!ap^AUBZ50miCTW%9LYCRIrWV4DFZ`?H~Ks4 zrT@5I(^@j=TKNBYpRUiIcI-g&#HBkj!>-K<=vz8d_`nH9z2_HC2fdiP%2IAk@;uGv z+AB*M#GWpFf99ZIdF97-6Km%&=uPVTT0d*y-DJt8yRJ)Deg0LD_UwPibA`!2m^y!X z{XHS%|8v2ksf-JLnI6s*eC955#F*>%icj~&o?So6v8^X##-1znO1%ep!~KqBtS|pt z?|uEBlz-psJF6{(+8vc%R!kHXSn9b{pKs%<`Fe{j&bOWNO{V2X4KEFHSae&)yg^UFuO$fx^|5DVc1OTO2A(qstg?F4X3_ zyEZ4%YW=?ZhtE%0J$=Q}J-Lw`W#PU%78cJ_SaPS5-RODgwkCslKMb0i3b>y?o27F@ zgFoZ(6^HK(BKco_bafbK8qeArsplRP;ymH|g5U=W*)w0volb>hqbQNa2X=is}|J1|+BaaXzx!7+D#Z8+#cH8atU98yQY~wdJpWo1pRJ97CdJdcUdjn)unXKfkK2d-c5V zEO)%k%78T=ejcB(?AF`2v-KL-#p04@JC(jz!t$eS>bYo_mh|RDJ)b=0PU3waB%|o+ z;NjDV7G7FS6lwTy{ibwcq4l@0e?*^M34q meEJE){Qb)7l@=dZJu~w6wE6l8#|~XHbUXe;+-mg8#?lP8#77c4$+Eo;(}9WO%Y1-~xCKxyiL+Gmsk{xLp+Qb>dDO z@=v)KQdmru5|$8ztpt;r%#*aRW}_RFLn8_vU7mnWq4H;6$l~uU6pxdEG?ID^OkZ9L zCg9+}GoXe8Jz@fPv(-Xp_|K^(BOD%0Ol;kNYWAbz zl#h)#KsmmK!-#+cYP^rSnhEKq^2ZZV&V6z|d=Y-5{5%u8%ySLV7`R-Nqy!yeXLDY0 zCv}4UsL}6*DNT!$!^%B6MT)ptm5WfKcLrRTM^$tA6v+UiG#&T zJ@24tu*4U}-f~E**?ClrGvSX!3?Z}|AK%(jU<<4Ae)T-b??_KnXeJV}ibkmQ#1E^< z3dJpaN6xDTWfzHG=*FQVd*Mi~w2PqMil00SzqA`@o4hOJ>NQM#b23CNeg~P!}ftI2_ zbe8k8tajt4^`s;fz2Qe*YwtC>LDo{{G}c?epUEWo?L}<+SO(S2RdYGW!ukB-E?$h6 zv#Ur!R#<6~A%>$<{jP*^n+~)M>=G#Z?}EBIOR7;W3-jVy*%!t;#2BkdzW+#Wcb6~H z8K>+CoH;H1rNbmV#rU=_Mqk27fIzzg3}|e5?70&P zMT-WUcSaoY{-dW3_>{A>l{jV3@h1(mX-{wuz8mfiQHIA(h8xXXs0ox&kV0DURsu$5 zo=Go;C4c4Fqpv>V%8VnXZ6VqT;@Qimt)tv-KtpsfF$<15->4|$R(cb++djO>iHlm2 zrjtyLH*!?69gy><+FzY0v{&o^EG3*IM~FxeH1$QD<;pFbHCFXb^?1ZZ8zvHR2d?&Z zF;Q)1;T6hoWE>lc?kz;BF0mxIN3g5hG1a>jqwl4ClnhQ?P)I%3wT{_dngwefpBq&~ zq&ww#buux;Xg+@0h6MaAigrOSCK@nQ=mfHlOqQaF3Ag{{YoHKQ1bxGW6L8+eiKH>tw6S2k2RSn(sXjISis~Cu zvL+7*NSWr})SZpvZ+&Kd42L8v&Onj3;$O9+nGQ(Ja=WKKN;Fa7FoFre1_>&LNQUdB4hmk1-{ z+EEc;b6xgiS3z|L;-960d~cF zeF;UNC5bo$@5FQMtLQA3lQS;GBJy@}qlTi6S!rWufi+shlGGgHVOF0N_daAE;K`!L z>I=bnZ3I;_Wpr=oYvkO#H;uc%dD-%%nG=bEk&=1eu}&&a%MAG=qCkj2_fy76l;)C4 zId{GkKiVB1PQ0b91umnM4n1DPOfdz=9yKgStg0^jl9E;gb1celhqrN+{(DX$i~{@v zw>jlHUOO`X67f#{Sb4XAL!@GKA062HcPhRY{e}7{a78(QFbdOif=C%FV;lZI{?&X> z&A${gv3`^hEA$ox%59B4ixXhF@$&e_Mb2Py=)^yyu zQuwQP?zbjq6BvyJQ=uOPVSIRGi|==&5?&$y2i+kUKQT6}xwC|Y(mdV*Wbmp&5rhww zc${aDtxUjbsq{pv%5mvUdCRW$?%EDoozq&2NT>pz#WBJ4xh%pudUH8{YP8F5t$th; z3m<01M}QKL4D8>4_TLPkxIqoG6@$e~Zw5qD`KycFU9={=Bl&M$2P#oY+S$Map$;O^tF_Wc{|rhdnqn}EPd!`$*VdSKaq zdAotQ$8b)%78m{ZF|!Zn1!Ged7Ib*QM1{R9mEm%3v(z56+8-z*-i&oXW?>}ibZbm? z1E8U*gq+%R8ey~*_2wd=C zZiMnGiH!^Qfx~IAi}=nYvQwHfNoPtB{f!CeqQom~VBstK=3ys!3(O+V&2coGKjotj z5OffIp9oDx@_N0u8-VCN;-y4h*`=r)$oLo&P%U$p4L*#E=M!%;VX^oFLo6y$S zl`G1jiV%=Yxwdal&|2RK0EQ?ntsrdiizlO4gimXna?TT@)_VMr!0b-pPWzPVVcZ$1 zy>!aPT%Sr!Xuw&)PI^$$JM}Hgf6aMKgeUKQiRL;0uH+}*`yr>LkrDS;zl=sXgY$YO zk%dQmvBeMlH~0?^QL5jo@ynBi8jlmvfNgAORUNvu{-(cop;qHh8Sqg8QLz3wWf>4` z0vwk|Mj9(5b%jpy+y+sB^8NTepA0A`_z+zqkH_yVYKV2Kjv;oG1dA))6?i?T>_1!B zt%8x(?+-+S(x4j|E4V4G7m3VsmG+|KD16WP$bBjSV*(Sj@hv9>l<2>fH0sPKwbwXG zNv_DY&<#RmnPiS>M$Q7>n;|An&VrnIL1B*`4P`1nktfl#qd$m&M6SR*n;buX#8nK~R25>IeqlGG8Kl$*buq*Ql02Hy>1J>il?@@7T^%Zv zGg$?5_?m?;$o%my%2*MD5;p8$dnhzZg{<2=lwj+gP5u>8Mj6KfB<{;pJEcE>82zxX zjIw14N_`)2%GF#SvtE4}E_Qm43zVru8u6FWr(hnv*Xf06`E|2$_B2!B^RSI|W9Wd) z11Hxj;C#vM_mVUQq+G-xafY{mi6%ZGhQ}0)^dA)`QyoYS?_k(O|KESUc^yMAEI;+Jti0i29tnwp!%ZUxj$f+(v>jhP*4e+AiZ9hhwc-8|Xt5$3d69hiHH zm6gx*P%b|xcZXNP+h&p7F4GrGr3p-{nvvv&T*{nGO25RC-8gY@$pi+t)R(0`((G1~ zV%V=Fq8m(U`)K&=3 zssiZd+OFQpdHuGKqLZG74ot7gq;i4KIXoF?Yf>|h2`*G@< z9DD586BoTSmrc%5D+hd&3Eoue-N!eSfL?x3CD3JgpW^W`O`PTPk*IHslc8R za2;jsXCwMoF;=4Yj1Ot zLS|{v%vtK$m;879YYcO9Uk&`4HY& ziJ^S)4GN$Yqs!|s{a>_X@1E5?andVYWtm2`Yw2hW8Bt2LYgh1?U8V1>`V-1>i7(0^bG+ zDuzgg_YDCD2B3lh&@h4m%mM)bFoFV+KOTQ**_brBj27$y0tf(rf&!S)cW}(Gscz4n zCYB2p;ZWc>4AZWD%$ac&!i|7OGut;#+|FH8jK5vM>|Wh%^gy0KN_Z+(3-Uan4Jpm! zTtO1T2iR!uY?vrP$}u9x0ykHnZpqSspo>%NO)<>yFNf+4ROqF0K1gzg0<24A$bhp-6)i2R+B~l1GvH8L^h=KR3D43 zbTHhktpIPZ#K5Pw`Qu9VaBF`lKEGOyQgyqAKI?LDGuo3MR0+F@$QO8uJv^KW%f^4ISSSkDw@Z3z zmu8--O{QC&w;&FoFX41_>&LNQUflnep_2mpTsf&ze>fK-%olXI+IBp1Ii&$~IXIze#m|FCwl;#6V~myvrY zYzP-?_vmh+qx&TXbUJU>OD}ioB%sORO%*D#rU*QDBsXfGOWDpBl6FZ4|3~LkHxZ(C z4C5|;$3p(Ev`EG1D(L7o=D0Ds=rBvH_g?ER1)a#gf98({TW)_1%n$TQ2M2d~$ho-Y ztMOs)rH&OioH;|H;w5ZWlBx77`mjZ)WV92@jX?3J+WmCl(;G{1^}q-QO9lAcDPQRz zvxRdPJSm!XC6(#q4@-`rwRN^HYYlQ}k<0u)n;}Hr&6Hk+h+83xj%xlf?tIdv5Gfao zC766_GS_LuB>#VugYtj~0m~rX2j=5H0ww83Je*a4Pc?Zg%E<24&LNQUJKN;PpuN^1PEn0YVJCfno$A*2moPNpool->obj_name, + "CA certificates loaded from '%s%s%s'", + cert->CA_file.ptr, + ((cert->CA_file.slen && cert->CA_path.slen)? + " + ":""), + cert->CA_path.ptr)); } } @@ -1062,6 +1069,10 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) cert->cert_file.ptr)); SSL_CTX_free(ctx); return status; + } else { + PJ_LOG(4,(ssock->pool->obj_name, + "Certificate chain loaded from '%s'", + cert->cert_file.ptr)); } } @@ -1079,6 +1090,10 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) cert->privkey_file.ptr)); SSL_CTX_free(ctx); return status; + } else { + PJ_LOG(4,(ssock->pool->obj_name, + "Private key loaded from '%s'", + cert->privkey_file.ptr)); } #if !defined(OPENSSL_NO_DH) @@ -1124,6 +1139,9 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) BIO_free(cbio); SSL_CTX_free(ctx); return status; + } else { + PJ_LOG(4,(ssock->pool->obj_name, + "Certificate chain loaded from buffer")); } X509_free(xcert); } @@ -1141,13 +1159,29 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) NULL, NULL); if (inf != NULL) { - int i = 0; + int i = 0, cnt = 0; for (; i < sk_X509_INFO_num(inf); i++) { X509_INFO *itmp = sk_X509_INFO_value(inf, i); - if (itmp->x509) { - X509_STORE_add_cert(cts, itmp->x509); + if (!itmp->x509) + continue; + + rc = X509_STORE_add_cert(cts, itmp->x509); + if (rc == 1) { + ++cnt; + } else { +#if PJ_LOG_MAX_LEVEL >= 4 + char buf[256]; + PJ_LOG(4,(ssock->pool->obj_name, + "Error adding CA cert: %s", + X509_NAME_oneline( + X509_get_subject_name(itmp->x509), + buf, sizeof(buf)))); +#endif } } + PJ_LOG(4,(ssock->pool->obj_name, + "CA certificates loaded from buffer (cnt=%d)", + cnt)); } sk_X509_INFO_pop_free(inf, X509_INFO_free); BIO_free(cbio); @@ -1161,7 +1195,8 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) kbio = BIO_new_mem_buf((void*)cert->privkey_buf.ptr, cert->privkey_buf.slen); if (kbio != NULL) { - pkey = PEM_read_bio_PrivateKey(kbio, NULL, 0, NULL); + pkey = PEM_read_bio_PrivateKey(kbio, NULL, &password_cb, + cert); if (pkey) { rc = SSL_CTX_use_PrivateKey(ctx, pkey); if (rc != 1) { @@ -1172,9 +1207,16 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) BIO_free(kbio); SSL_CTX_free(ctx); return status; + } else { + PJ_LOG(4,(ssock->pool->obj_name, + "Private key loaded from buffer")); } EVP_PKEY_free(pkey); + } else { + PJ_LOG(1,(ssock->pool->obj_name, + "Error reading private key from buffer")); } + if (ssock->is_server) { dh = PEM_read_bio_DHparams(kbio, NULL, NULL, NULL); if (dh != NULL) { @@ -1319,8 +1361,16 @@ static pj_status_t ssl_create(pj_ssl_sock_t *ssock) BIO_free(new_bio); } - if (ca_dn != NULL) - SSL_CTX_set_client_CA_list(ctx, ca_dn); + if (ca_dn != NULL) { + SSL_CTX_set_client_CA_list(ctx, ca_dn); + PJ_LOG(4,(ssock->pool->obj_name, + "CA certificates loaded from %s", + (cert->CA_file.slen?cert->CA_file.ptr:"buffer"))); + } else { + PJ_LOG(1,(ssock->pool->obj_name, + "Error reading CA certificates from %s", + (cert->CA_file.slen?cert->CA_file.ptr:"buffer"))); + } } /* Early sensitive data cleanup after OpenSSL context setup. However, diff --git a/pjlib/src/pjlib-test/ssl_sock.c b/pjlib/src/pjlib-test/ssl_sock.c index 97d02bfcd..1505d8d16 100644 --- a/pjlib/src/pjlib-test/ssl_sock.c +++ b/pjlib/src/pjlib-test/ssl_sock.c @@ -31,7 +31,7 @@ #endif #define CERT_FILE CERT_DIR "cacert.pem" #define CERT_PRIVKEY_FILE CERT_DIR "privkey.pem" -#define CERT_PRIVKEY_PASS "" +#define CERT_PRIVKEY_PASS "privkeypass" #define TEST_LOAD_FROM_FILES 1 diff --git a/pjnath/src/pjnath-test/server.c b/pjnath/src/pjnath-test/server.c index 5ac1566ec..37f80f893 100644 --- a/pjnath/src/pjnath-test/server.c +++ b/pjnath/src/pjnath-test/server.c @@ -27,7 +27,7 @@ #define CERT_CA_FILE CERT_DIR "cacert.pem" #define CERT_FILE CERT_DIR "cacert.pem" #define CERT_PRIVKEY_FILE CERT_DIR "privkey.pem" -#define CERT_PRIVKEY_PASS "" +#define CERT_PRIVKEY_PASS "privkeypass" #define RETURN_ERROR(rc) {app_perror("", rc);return rc;}