Skip to content

Commit

Permalink
Backed out changeset 1e86f5cfc1cd
Browse files Browse the repository at this point in the history
--HG--
branch : NSS_3_69_1_BRANCH
  • Loading branch information
beurdouche committed Aug 26, 2021
1 parent 769b5ee commit 0ba972e
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 98 deletions.
1 change: 0 additions & 1 deletion automation/taskcluster/docker-builds/Dockerfile
Expand Up @@ -35,7 +35,6 @@ RUN apt-get update \
valgrind \
zlib1g-dev \
clang-format-3.9 \
sqlite3 \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get autoremove -y && apt-get clean -y

Expand Down
1 change: 0 additions & 1 deletion automation/taskcluster/docker-gcc-4.4/Dockerfile
Expand Up @@ -11,7 +11,6 @@ RUN apt-get update \
make \
patch \
mercurial \
sqlite3 \
zlib1g-dev \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get autoremove -y && apt-get clean -y
Expand Down
1 change: 0 additions & 1 deletion automation/taskcluster/docker/Dockerfile
Expand Up @@ -20,7 +20,6 @@ RUN apt-get update \
mercurial \
ninja-build \
pkg-config \
sqlite3 \
zlib1g-dev \
&& rm -rf /var/lib/apt/lists/* \
&& apt-get autoremove -y && apt-get clean -y
Expand Down
38 changes: 11 additions & 27 deletions lib/softoken/sftkpwd.c
Expand Up @@ -287,12 +287,9 @@ sftkdb_DecryptAttribute(SFTKDBHandle *handle, SECItem *passKey,
}

/* If we are using aes 256, we need to check authentication as well.*/
if ((type != CKT_INVALID_TYPE) &&
(cipherValue.alg == SEC_OID_PKCS5_PBES2) &&
(cipherValue.param->encAlg == SEC_OID_AES_256_CBC)) {
if ((type != CKT_INVALID_TYPE) && (cipherValue.alg == SEC_OID_AES_256_CBC)) {
SECItem signature;
unsigned char signData[SDB_MAX_META_DATA_LEN];
CK_RV crv;

/* if we get here from the old legacy db, there is clearly an
* error, don't return the plaintext */
Expand All @@ -304,28 +301,15 @@ sftkdb_DecryptAttribute(SFTKDBHandle *handle, SECItem *passKey,

signature.data = signData;
signature.len = sizeof(signData);
rv = SECFailure;
/* sign sftkdb_GetAttriibuteSignature returns a crv, not an rv */
crv = sftkdb_GetAttributeSignature(handle, handle, id, type,
&signature);
if (crv == CKR_OK) {
rv = sftkdb_VerifyAttribute(handle, passKey, CK_INVALID_HANDLE,
type, *plain, &signature);
rv = sftkdb_GetAttributeSignature(handle, handle, id, type,
&signature);
if (rv != SECSuccess) {
goto loser;
}
rv = sftkdb_VerifyAttribute(handle, passKey, CK_INVALID_HANDLE, type,
*plain, &signature);
if (rv != SECSuccess) {
/* handle bug 1720226 where old versions of NSS misfiled the signature
* attribute on password update */
id |= SFTK_KEYDB_TYPE | SFTK_TOKEN_TYPE;
signature.len = sizeof(signData);
crv = sftkdb_GetAttributeSignature(handle, handle, id, type,
&signature);
if (crv != CKR_OK) {
rv = SECFailure;
PORT_SetError(SEC_ERROR_BAD_SIGNATURE);
goto loser;
}
rv = sftkdb_VerifyAttribute(handle, passKey, CK_INVALID_HANDLE,
type, *plain, &signature);
goto loser;
}
}

Expand Down Expand Up @@ -1214,7 +1198,6 @@ sftk_updateEncrypted(PLArenaPool *arena, SFTKDBHandle *keydb,
unsigned int i;
for (i = 0; i < privAttrCount; i++) {
// Read the old attribute in the clear.
CK_OBJECT_HANDLE sdbId = id & SFTK_OBJ_ID_MASK;
CK_ATTRIBUTE privAttr = { privAttrTypes[i], NULL, 0 };
CK_RV crv = sftkdb_GetAttributeValue(keydb, id, &privAttr, 1);
if (crv != CKR_OK) {
Expand All @@ -1239,7 +1222,7 @@ sftk_updateEncrypted(PLArenaPool *arena, SFTKDBHandle *keydb,
plainText.data = privAttr.pValue;
plainText.len = privAttr.ulValueLen;
if (sftkdb_EncryptAttribute(arena, keydb, keydb->db, newKey,
iterationCount, sdbId, privAttr.type,
iterationCount, id, privAttr.type,
&plainText, &result) != SECSuccess) {
return CKR_GENERAL_ERROR;
}
Expand All @@ -1249,9 +1232,10 @@ sftk_updateEncrypted(PLArenaPool *arena, SFTKDBHandle *keydb,
PORT_Memset(plainText.data, 0, plainText.len);

// Write the newly encrypted attributes out directly.
CK_OBJECT_HANDLE newId = id & SFTK_OBJ_ID_MASK;
keydb->newKey = newKey;
keydb->newDefaultIterationCount = iterationCount;
crv = (*keydb->db->sdb_SetAttributeValue)(keydb->db, sdbId, &privAttr, 1);
crv = (*keydb->db->sdb_SetAttributeValue)(keydb->db, newId, &privAttr, 1);
keydb->newKey = NULL;
if (crv != CKR_OK) {
return crv;
Expand Down
70 changes: 2 additions & 68 deletions tests/dbtests/dbtests.sh
Expand Up @@ -75,7 +75,6 @@ Echo()
echo "| $*"
echo "---------------------------------------------------------------"
}

dbtest_main()
{
cd ${HOSTDIR}
Expand Down Expand Up @@ -250,9 +249,9 @@ dbtest_main()
${BINDIR}/certutil -L -n bob -d ${CONFLICT_DIR}
ret=$?
if [ $ret -ne 0 ]; then
html_failed "Nickname conflict test-setting nickname conflict incorrectly worked"
html_failed "Nicknane conflict test-setting nickname conflict incorrectly worked"
else
html_passed "Nickname conflict test-setting nickname conflict was correctly rejected"
html_passed "Nicknane conflict test-setting nickname conflict was correctly rejected"
fi
# import a token private key and make sure the corresponding public key is
# created
Expand All @@ -263,71 +262,6 @@ dbtest_main()
else
html_passed "Importing Token Private Key correctly creates the corrresponding Public Key"
fi
#
# If we are testing an sqlite db, make sure we detect corrupted attributes.
# This test only runs if 1) we have sqlite3 (the command line sqlite diagnostic
# tool) and 2) we are using the sql database (rather than the dbm).
#
which sqlite3
ret=$?
KEYDB=${CONFLICT_DIR}/key4.db
# make sure sql database is bing used.
if [ ! -f ${KEYDB} ]; then
Echo "skipping key corruption test: requires sql database"
# make sure sqlite3 is installed.
elif [ $ret -ne 0 ]; then
Echo "skipping key corruption test: sqlite3 command not installed"
else
# we are going to mangle this key database in multiple tests, save a copy
# so that we can restore it later.
cp ${KEYDB} ${KEYDB}.save
# dump the keys in the log for diagnostic purposes
${BINDIR}/certutil -K -d ${CONFLICT_DIR} -f ${R_PWFILE}
# fetch the rsa and ec key ids
rsa_id=$(${BINDIR}/certutil -K -d ${CONFLICT_DIR} -f ${R_PWFILE} | grep rsa | awk '{ print $4}')
ec_id=$(${BINDIR}/certutil -K -d ${CONFLICT_DIR} -f ${R_PWFILE} | grep ' ec ' | awk '{ print $4}')
# now loop through all the private attributes and mangle them one at a time
for i in 120 122 123 124 125 126 127 128 011
do
Echo "testing if key corruption is detected in attribute $i"
cp ${KEYDB}.save ${KEYDB} # restore the saved keydb
# find all the hmacs for this key attribute and mangle each entry
sqlite3 ${KEYDB} ".dump metadata" | sed -e "/sig_key_.*_00000$i/{s/.*VALUES('\\(.*\\)',X'\\(.*\\)',NULL.*/\\1 \\2/;p;};d" | while read sig data
do
# mangle the last byte of the hmac
# The following increments the last nibble by 1 with both F and f
# mapping to 0. This mangles both upper and lower case results, so
# it will work on the mac.
last=$((${#data}-1))
newbyte=$(echo "${data:${last}}" | tr A-Fa-f0-9 B-F0b-f0-9a)
mangle=${data::${last}}${newbyte}
echo " amending ${sig} from ${data} to ${mangle}"
# write the mangled entry, inserting with a key matching an existing
# entry will overwrite the existing entry with the same key (${sig})
sqlite3 ${KEYDB} "BEGIN TRANSACTION; INSERT INTO metaData VALUES('${sig}',X'${mangle}',NULL); COMMIT"
done
# pick the key based on the attribute we are mangling,
# only CKA_VALUE (0x011) is not an RSA attribute, so we choose
# ec for 0x011 and rsa for all the rest. We could use the dsa
# key here, both CKA_VALUE attributes will be modifed in the loop above, but
# ec is more common than dsa these days.
if [ "$i" = "011" ]; then
key_id=$ec_id
else
key_id=$rsa_id
fi
# now try to use the mangled key (try to create a cert request with the key).
echo "${BINDIR}/certutil -R -k ${key_id} -s 'CN=BadTest, E=bad@mozilla.com, O=BOGUS NSS, L=Mountain View, ST=California, C=US' -d ${CONFLICT_DIR} -f ${R_PWFILE} -a"
${BINDIR}/certutil -R -k ${key_id} -s 'CN=BadTest, E=bad@mozilla.com, O=BOGUS NSS, L=Mountain View, ST=California, C=US' -d ${CONFLICT_DIR} -f ${R_PWFILE} -a
ret=$?
if [ ${ret} -eq 0 ]; then
html_failed "Key attribute $i corruption not detected"
else
html_passed "Corrupted key attribute $i correctly disabled key"
fi
done
cp ${KEYDB}.save ${KEYDB} # restore the saved keydb
fi


if [ "${NSS_DEFAULT_DB_TYPE}" = "sql" ] ; then
Expand Down

0 comments on commit 0ba972e

Please sign in to comment.