Resolves: rhbz#2041832

openssl pkcs12 unable to process nss pk12util generated pkcs12 file if its password length is >= 64 chars
This commit is contained in:
Bob Relyea 2022-02-16 12:55:59 -08:00
parent fd0aecc80b
commit abcefb3fa4
2 changed files with 264 additions and 1 deletions

View File

@ -0,0 +1,257 @@
diff --git a/cmd/pk12util/pk12util.c b/cmd/pk12util/pk12util.c
--- a/cmd/pk12util/pk12util.c
+++ b/cmd/pk12util/pk12util.c
@@ -660,16 +660,27 @@ P12U_ExportPKCS12Object(char *nn, char *
}
/* Password to use for PKCS12 file. */
pwitem = P12U_GetP12FilePassword(PR_TRUE, p12FilePw);
if (!pwitem) {
goto loser;
}
+ /* we are passing UTF8, drop the NULL in the normal password value.
+ * UCS2 conversion will add it back if necessary. This only affects
+ * password > Blocksize of the Hash function and pkcs5v2 pbe (if password
+ * <=Blocksize then the password is zero padded anyway, so an extra NULL
+ * at the end has not effect). This is allows us to work with openssl and
+ * gnutls. Older versions of NSS already fail to decrypt long passwords
+ * in this case, so we aren't breaking anyone with this code */
+ if ((pwitem->len > 1) && (!pwitem->data[pwitem->len-1])) {
+ pwitem->len--;
+ }
+
p12cxt = p12u_InitContext(PR_FALSE, outfile);
if (!p12cxt) {
SECU_PrintError(progName, "Initialization failed: %s", outfile);
pk12uErrno = PK12UERR_INIT_FILE;
goto loser;
}
if (certlist) {
diff --git a/lib/pkcs12/p12local.c b/lib/pkcs12/p12local.c
--- a/lib/pkcs12/p12local.c
+++ b/lib/pkcs12/p12local.c
@@ -903,31 +903,35 @@ sec_pkcs12_find_object(SEC_PKCS12SafeCon
i++;
}
}
PORT_SetError(SEC_ERROR_PKCS12_UNABLE_TO_LOCATE_OBJECT_BY_NAME);
return NULL;
}
-/* this function converts a password to unicode and encures that the
- * required double 0 byte be placed at the end of the string
+/* this function converts a password to unicode and ensures that the
+ * required double 0 byte be placed at the end of the string (if zeroTerm
+ * is set), or the 0 bytes at the end are dropped (if zeroTerm is not set).
*/
PRBool
sec_pkcs12_convert_item_to_unicode(PLArenaPool *arena, SECItem *dest,
SECItem *src, PRBool zeroTerm,
PRBool asciiConvert, PRBool toUnicode)
{
PRBool success = PR_FALSE;
+ int bufferSize;
+
if (!src || !dest) {
PORT_SetError(SEC_ERROR_INVALID_ARGS);
return PR_FALSE;
}
- dest->len = src->len * 3 + 2;
+ bufferSize = src->len * 3 + 2;
+ dest->len = bufferSize;
if (arena) {
dest->data = (unsigned char *)PORT_ArenaZAlloc(arena, dest->len);
} else {
dest->data = (unsigned char *)PORT_ZAlloc(dest->len);
}
if (!dest->data) {
dest->len = 0;
@@ -951,34 +955,44 @@ sec_pkcs12_convert_item_to_unicode(PLAre
if (!arena) {
PORT_Free(dest->data);
dest->data = NULL;
dest->len = 0;
}
return PR_FALSE;
}
- if ((dest->len >= 2) &&
- (dest->data[dest->len - 1] || dest->data[dest->len - 2]) && zeroTerm) {
- if (dest->len + 2 > 3 * src->len) {
- if (arena) {
- dest->data = (unsigned char *)PORT_ArenaGrow(arena,
- dest->data, dest->len,
- dest->len + 2);
- } else {
- dest->data = (unsigned char *)PORT_Realloc(dest->data,
- dest->len + 2);
+ /* in some cases we need to add NULL terminations and in others
+ * we need to drop null terminations */
+ if (zeroTerm) {
+ /* unicode adds two nulls a the end */
+ if (toUnicode) {
+ if ((dest->len >= 2) &&
+ (dest->data[dest->len - 1] || dest->data[dest->len - 2])) {
+ /* we've already allocated space for these new NULLs */
+ PORT_Assert(dest->len + 2 <= bufferSize);
+ dest->len += 2;
+ dest->data[dest->len - 1] = dest->data[dest->len - 2] = 0;
}
-
- if (!dest->data) {
- return PR_FALSE;
+ /* ascii/utf-8 adds just 1 */
+ } else if ((dest->len >= 1) && dest->data[dest->len-1]) {
+ PORT_Assert(dest->len + 1 <= bufferSize);
+ dest->len ++;
+ dest->data[dest->len-1] = 0;
+ }
+ } else {
+ /* handle the drop case, no need to do any allocations here. */
+ if (toUnicode) {
+ while ((dest->len >=2) && !dest->data[dest->len - 1] &&
+ !dest->data[dest->len - 2]) {
+ dest->len -= 2;
}
+ } else while (dest->len && !dest->data[dest->len-1]) {
+ dest->len--;
}
- dest->len += 2;
- dest->data[dest->len - 1] = dest->data[dest->len - 2] = 0;
}
return PR_TRUE;
}
PRBool
sec_pkcs12_is_pkcs12_pbe_algorithm(SECOidTag algorithm)
{
@@ -1006,27 +1020,28 @@ sec_pkcs12_is_pkcs12_pbe_algorithm(SECOi
}
}
/* this function decodes a password from Unicode if necessary,
* according to the PBE algorithm.
*
* we assume that the pwitem is already encoded in Unicode by the
* caller. if the encryption scheme is not the one defined in PKCS
- * #12, decode the pwitem back into UTF-8. */
+ * #12, decode the pwitem back into UTF-8. NOTE: UTF-8 strings are
+ * used in the PRF without the trailing NULL */
PRBool
sec_pkcs12_decode_password(PLArenaPool *arena,
SECItem *result,
SECOidTag algorithm,
const SECItem *pwitem)
{
if (!sec_pkcs12_is_pkcs12_pbe_algorithm(algorithm))
return sec_pkcs12_convert_item_to_unicode(arena, result,
(SECItem *)pwitem,
- PR_TRUE, PR_FALSE, PR_FALSE);
+ PR_FALSE, PR_FALSE, PR_FALSE);
return SECITEM_CopyItem(arena, result, pwitem) == SECSuccess;
}
/* this function encodes a password into Unicode if necessary,
* according to the PBE algorithm.
*
* we assume that the pwitem holds a raw password. if the encryption
diff --git a/tests/common/init.sh b/tests/common/init.sh
--- a/tests/common/init.sh
+++ b/tests/common/init.sh
@@ -78,25 +78,27 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOU
CERT_EXTENSIONS_DIR=${HOSTDIR}/cert_extensions
STAPLINGDIR=${HOSTDIR}/stapling
NOLOGINDIR=${HOSTDIR}/nologin
SSLGTESTDIR=${HOSTDIR}/ssl_gtests
GTESTDIR=${HOSTDIR}/gtests
PWFILE=${HOSTDIR}/tests.pw
+ LONGPWFILE=${HOSTDIR}/tests.longpw
EMPTY_FILE=${HOSTDIR}/tests_empty
NOISE_FILE=${HOSTDIR}/tests_noise
CORELIST_FILE=${HOSTDIR}/clist
FIPSPWFILE=${HOSTDIR}/tests.fipspw
FIPSBADPWFILE=${HOSTDIR}/tests.fipsbadpw
FIPSP12PWFILE=${HOSTDIR}/tests.fipsp12pw
echo nss > ${PWFILE}
+ echo "nss123456789012345678901234567890123456789012345678901234567890_" > ${LONGPWFILE}
echo > ${EMPTY_FILE}
echo "fIps140" > ${FIPSPWFILE}
echo "fips104" > ${FIPSBADPWFILE}
echo "pKcs12fips140" > ${FIPSP12PWFILE}
noise
P_SERVER_CADIR=${SERVER_CADIR}
@@ -656,16 +658,17 @@ if [ -z "${INIT_SOURCED}" -o "${INIT_SOU
P_R_NOLOGINDIR="multiaccess:${D_NOLOGIN}"
P_R_EXT_SERVERDIR="multiaccess:${D_EXT_SERVER}"
P_R_EXT_CLIENTDIR="multiaccess:${D_EXT_CLIENT}"
P_R_IMPLICIT_INIT_DIR="multiaccess:${D_IMPLICIT_INIT}"
P_R_RSAPSSDIR="multiaccess:${D_RSAPSS}"
fi
R_PWFILE=../tests.pw
+ R_LONGPWFILE=../tests.longpw
R_EMPTY_FILE=../tests_empty
R_NOISE_FILE=../tests_noise
R_FIPSPWFILE=../tests.fipspw
R_FIPSBADPWFILE=../tests.fipsbadpw
R_FIPSP12PWFILE=../tests.fipsp12pw
trap "Exit $0 Signal_caught" 2 3
diff --git a/tests/tools/tools.sh b/tests/tools/tools.sh
--- a/tests/tools/tools.sh
+++ b/tests/tools/tools.sh
@@ -382,16 +382,40 @@ tools_p12_export_list_import_with_defaul
check_tmpfile
echo "$SCRIPTNAME: Listing Alice's pk12 EC file -----------------"
echo "pk12util -l Alice-ec.p12 -w ${R_PWFILE}"
${BINDIR}/pk12util -l Alice-ec.p12 -w ${R_PWFILE} 2>&1
ret=$?
html_msg $ret 0 "Listing Alice's pk12 EC file (pk12util -l)"
check_tmpfile
+
+ echo "$SCRIPTNAME: Exporting Alice's email EC cert & key with long pw------"
+ echo "pk12util -o Alice-ec-long.p12 -n \"Alice-ec\" -d ${P_R_ALICEDIR} -k ${R_PWFILE} \\"
+ echo " -w ${R_LONGPWFILE}"
+ ${BINDIR}/pk12util -o Alice-ec-long.p12 -n "Alice-ec" -d ${P_R_ALICEDIR} -k ${R_PWFILE} \
+ -w ${R_LONGPWFILE} 2>&1
+ ret=$?
+ html_msg $ret 0 "Exporting Alice's email EC cert & key with long pw (pk12util -o)"
+ check_tmpfile
+ verify_p12 Alice-ec-long.p12 "default" "default" "default"
+
+ echo "$SCRIPTNAME: Importing Alice's email EC cert & key with long pw-----"
+ echo "pk12util -i Alice-ec-long.p12 -d ${P_R_COPYDIR} -k ${R_PWFILE} -w ${R_LONGPWFILE}"
+ ${BINDIR}/pk12util -i Alice-ec-long.p12 -d ${P_R_COPYDIR} -k ${R_PWFILE} -w ${R_LONGPWFILE} 2>&1
+ ret=$?
+ html_msg $ret 0 "Importing Alice's email EC cert & key with long pw (pk12util -i)"
+ check_tmpfile
+
+ echo "$SCRIPTNAME: Listing Alice's pk12 EC file with long pw ------------"
+ echo "pk12util -l Alice-ec-long.p12 -w ${R_LONGPWFILE}"
+ ${BINDIR}/pk12util -l Alice-ec-long.p12 -w ${R_LONGPWFILE} 2>&1
+ ret=$?
+ html_msg $ret 0 "Listing Alice's pk12 EC file with long pw (pk12util -l)"
+ check_tmpfile
}
tools_p12_import_old_files()
{
echo "$SCRIPTNAME: Importing PKCS#12 files created with older NSS --------------"
echo "pk12util -i TestOldCA.p12 -d ${P_R_COPYDIR} -k ${R_PWFILE} -w ${R_PWFILE}"
${BINDIR}/pk12util -i ${TOOLSDIR}/data/TestOldCA.p12 -d ${P_R_COPYDIR} -k ${R_PWFILE} -w ${R_PWFILE} 2>&1
ret=$?

View File

@ -4,7 +4,7 @@
# - increment %%{nspr_version}, when updating the NSS part only
# - put the nss_release number here next to nspr, as they both
# need to be updated on a given release
%global nss_release 6
%global nss_release 7
%global nspr_release %[ %nss_release+2]
%global nss_version 3.71.0
# only need to update this as we added new
@ -150,6 +150,8 @@ Patch70: nss-3.67-cve-2021-43527-test.patch
Patch80: nss-3.71-fix-lto-gtests.patch
# camellia pkcs12 docs.
patch85: nss-3.71-camellia-pkcs12-doc.patch
# fix issue with long passwords in pkcs12
patch90: nss-3.75-fix-pkcs12-passwords.patch
Patch100: nspr-config-pc.patch
Patch101: nspr-gcc-atomics.patch
@ -1105,6 +1107,10 @@ update-crypto-policies &> /dev/null || :
%changelog
* Wed Feb 16 2022 Bob Relyea <rrelyea@redhat.com> - 3.71.0-7
- Fix handling of pkcs12 passwords for PKCS5v2 cases which causes failures
on long passwords.
* Wed Jan 26 2022 Bob Relyea <rrelyea@redhat.com> - 3.71.0-6
- update pkcs12 documentation to include camellia
- turn on lto