PR#156: Make it possible to run NSS db type specific tests separately -

certmonger

#156 Make it possible to run NSS db type specific tests separately

Closed Jun 25, 2020 by rcritten. Opened Jun 10, 2020 by slev.

slev/certmonger dbm_tests into master

make: Add missing files to clean up and to distribute

Stanislav Levin • Jun 10, 2020

0a7615a73bd786559858c2eb896381d42ebd0305

tests: Parametrize 025-casave with NSS db types

Stanislav Levin • Jun 9, 2020

699ea0fb38aa04ef045aeeb45f4df3822e8df926

tests: Split the NSS db specific tests

Stanislav Levin • Jun 9, 2020

a7a9deb3fe6855de8fbaa93ebfeac1f4b432c531

Download 156.patch

slev commented Jun 10, 2020

If NSS is configured with NSS_DISABLE_DBM then Certmonger's tests which are related to DBM fail. The legacy NSS db type(DBM) will be eventually disabled. Thus, Certmonger should handle this.

NSS db specific tests are placed under the corresponding HAVE_SQL_NSSDB and HAVE_DBM_NSSDB sections.

Fixes: https://pagure.io/certmonger/issue/155

slev commented Jun 10, 2020

025-casave-sql fails:

[builder@localhost tests]$ srcdir=$(pwd) builddir=$(pwd) ./run-tests.sh 025-casave-sql:
Running test 025-casave-sql... Files /usr/src/tmp/runtests51ups2 and expected.out differ
FAIL
--- /usr/src/tmp/runtests51ups2 2020-06-10 09:31:28.838170763 +0000
+++ expected.out        2020-06-10 06:16:03.397980276 +0000
@@ -50,13 +50,13 @@
 [bundle-all]
 0
 [db1]
-Root Certificate D1 ,, 
+Root Certificate D1 CT,C,C
 [db2]
 [db3]
 [dba]
 Other Certificate D1 ,, 
-Other Root Certificate D1 ,, 
-Root Certificate D1 ,, 
+Other Root Certificate D1 CT,C,C
+Root Certificate D1 CT,C,C
 [(CAD2)]
 [bundle1]
@@ -69,12 +69,12 @@
 0
 [db1]
 [db2]
-Other Root Certificate D2 ,, 
+Other Root Certificate D2 CT,C,C
 [db3]
 [dba]
 Other Certificate D2 ,, 
-Other Root Certificate D2 ,, 
-Root Certificate D2 ,, 
+Other Root Certificate D2 CT,C,C
+Root Certificate D2 CT,C,C
...

, while 025-casave-dbm passed.

sql db:

[builder@localhost tests]$ certutil -d sql:/usr/src/tmp/runtestsof9jMu/dba -L
Database needs user init
Certificate Nickname                                         Trust Attributes
                                                             SSL,S/MIME,JAR/XPI
Root Certificate D3                                          ,,   
Other Root Certificate D3                                    ,,   
Other Certificate D3                                         ,,

dbm:

[builder@localhost tests]$ certutil -d dbm:/usr/src/tmp/runtestsfG5JsF/dba -L
Database needs user init
Certificate Nickname                                         Trust Attributes
                                                             SSL,S/MIME,JAR/XPI
Root Certificate D3                                          CT,C,C
Other Certificate D3                                         ,,   
Other Root Certificate D3                                    CT,C,C

Metadata Update from @rcritten:
- Request assigned

Jun 11, 2020

rcritten commented Jun 11, 2020

I'll see if I can figure out why the permissions are different, I duplicated it locally as well.

I suspect that the non dbm/sql directories can be removed with this as well

002-keygen
007-certsave
011-dbinit
012-dbadd
013-enckey
015-lockedkey
017-notoken
025-casave
034-perms

rcritten commented Jun 11, 2020

Never mind about removing directories, I see what you did now.

I think the issue is in how the certificate is being added. certmonger is using CERT_ImportCerts() and i think it should be using PK11_ImportCert(). I made similar changes related to tokens.

rcritten commented Jun 11, 2020

I have a candidate patch. The issue is that casave will create the NSS db if it doesn't already exist, as is the case for these tests. It doesn't initialize a PIN in that case and the sqlite implementation requires it apparently. I'm still going to switch to PK11_ImportCert() and add a bit more error handling, plus set an empty PIN if the db doesn't already have one.

rcritten commented Jun 23, 2020

Does my patch work for you? I attached it to the issue.

rcritten commented Jun 25, 2020

Closed in favor PR 159 which combined the two patch sets. That PR was merged.

Pull-Request has been closed by rcritten

Jun 25, 2020