1282627 - Merge can be confused with a modified trust flags set.

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Robert Relyea]

Description of problem:
Having sql database with a certificate that has modified a trust attribute and trying to merge this db with another one, it fails with error:

certutil: Could not merge object unnamed (type Trust): Unknown code ___P 3


Version-Release number of selected component (if applicable):
nss-tools-3.19.1-3.el6_6.x86_64


How reproducible:
100%


Steps to Reproduce:
rm -rf db1 db2
mkdir db1
echo 123456 > db1/pwdfile
certutil -N -d sql:./db1 -f ./db1/pwdfile
certutil -A -d sql:./db1 -i test.crt -n test -t P,, -f ./db1/pwdfile
certutil -M -d sql:./db1 -n test -t T,, -f ./db1/pwdfile

mkdir db2
echo 123456 > db2/pwdfile
certutil -N -d sql:./db2 -f ./db2/pwdfile

certutil --merge -d sql:./db2  --source-dir sql:./db1 -f ./db2/pwdfile -@ ./db1/pwdfile


Actual results:
certutil --merge fails with:
certutil: Could not merge object unnamed (type Trust): Unknown code ___P 3


Expected results:
certutil --merge works well


Additional info:
commenting out "certutil -M" or replacing it by deletion and re-creating test's certificate prevents this bug. I.e. the problem originates in certutil modifying the trust attributes. Therefore either:
- this "-M" operation is the buggy one, 
- or at least it leaves the sql database in such (valid) state that --merge operation cant cope with


From: https://bugzilla.redhat.com/show_bug.cgi?id=1294606

I'm presuming the actual bug is probably in softoken.

Comment 1

[Robert Relyea]

There are 3 bugs here:

1) pk11_copyAttributes in pk11merge.c is not setting the correct error code (or any error code for that matter). This is because a PKCS #11 CRV is being returned as a SECStatus. It should instead be mapped to an NSS error and the SECStatus set to failure.

2) pk11_copyAttributes needs to be more tolerant of missing attributes when trying to copy attributes.

3) sdb_SetAttributeValue in sdb.c is incorrectly recording NULL attributes because of an index problem (copy and paste code from sdb_CreateObject didn't have it's index offset adjusted properly).

Fixing 1 will fix the error message.
Fixing 2 will allow merging of already corrupted databases.
Fixing 3 will prevent future databases from being corrupted.

1 & 2 

bob

Comment 2

[Robert Relyea]

1 & 2 are in nss proper
3 is in softoken.

I'll attach 2 different patches. One for 1 &2 and one for 3.

bob

Comment 3

[Robert Relyea]

Attachment: Fix PK11_Merge issues with corrupted databases (issue 1 & 2).

Comment 4

[Robert Relyea]

Attachment: write NULL attributes to the database correctly if we are using the set attribute interface.

Comment 5

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8803008 [details] [diff] [review]
Fix PK11_Merge issues with corrupted databases (issue 1 & 2).

Review of attachment 8803008 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me in general but formatting is off and there are a couple trailing white space.
Since the problem is reproducible I'd like to see a test case that makes sure that this is fixed.

Comment 6

[Robert Relyea]

Attachment: fix_merge_problem_nss_v2.patch

Thanks Franziskus. I've fixed the white space problems and added a test, You meantioned formatting problems, but everything looks right for a file in pk11wrap (which hasn't been ran through any reformatter). If you can give me specific things I'll fix them, otherwise I think this patch should be good to go.

Comment 7

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8804503 [details] [diff] [review]
fix_merge_problem_nss_v2.patch

Review of attachment 8804503 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/merge/merge.sh
@@ +103,5 @@
>  
>    certutil -N -d ${CONFLICT1DIR} -f ${R_PWFILE}
>    certutil -N -d ${CONFLICT2DIR} -f ${R_PWFILE}
>    certutil -A -n Alice -t ,, -i ${R_CADIR}/TestUser41.cert -d ${CONFLICT1DIR}
> +  # modify CONFLICTDIR potentially corruptint the database

corrupting

Comment 8

[Robert Relyea]

NSS:
http://hg.mozilla.org/projects/nss/rev/24232beee1f2

Softoken:
https://hg.mozilla.org/projects/nss/rev/7701b1b052aa31ddf3159d489ebba777094e055c