[jdk8u-dev] RFR: 8195607: sun/security/pkcs11/Secmod/TestNssDbSqlite.java failed with "NSS initialization failed" on NSS 3.34.1

[Martin Balao]

On Tue, 23 Aug 2022 18:28:32 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Hi,
>> 
>> I'd like to propose the backport of the JDK-8195607 fix to 8u because this release is affected by this issue. Given that the legacy NSS DB has been deprecated for a long time and it's increasingly replaced by SQLite DBs, the chances that JDK-8 builds hit this problem are now higher.
>> 
>> The 11u backport applies cleanly, once paths are fixed.
>> 
>> To minimize risks, I manually debugged the OpenJDK code that connected to an NSS DB using the sql: prefix and everything was as expected.
>> 
>> I also run all the tests under the jdk/sun/security/pkcs11 category and noticed no-regressions strictly related to this fix. The test 'jdk/sun/security/pkcs11/Secmod/TrustAnchors.java' requires a special note, though. This test passes if run standalone but fails when run as part of a group. The reason is because when run as part of a group, other tests copy the file 'pkcs11.txt' to the temporary nssdb directory which is shared between all of them. This DB does not register the trust anchors module called 'Builtin Roots Module', which was available in the old secmod.db and is required by the test (which expects to see a Secmod$Module of TRUSTANCHOR type). While the test leaves the sun.security.pkcs11.SecmodTest::useSqlite field set to false and tries to access the legacy DB (also contained in the shared temporary nssdb directory), NSS realizes that there is a new SQLite DB in the same directory and reads the configuration from pkcs11.txt. Otherwise, if pkcs11.txt does not exist (as
  when the test is run standalone), it creates one with the information in secmod.db and it registers the 'Builtin Roots Module' module. All these findings have been verified with the 'modutil -list' command. While the issue is unrelated to what JDK-8195607 fixes, it has been uncovered by the existence of the pkcs11.txt file introduced with this change.
>> 
>> Why newer releases are not affected? Because jtreg runs tests (including TrustAnchors.java) in separated directories, so they don't share the nssdb temporary directory. In newer releases, TEST.ROOT has a requiredVersion attribute [1] indicating to jtreg that this can be done [2].
>> 
>> I've analyzed several ways to fix this issue but the one that I'd like to propose is to clean up the temporary nssdb directory for every test and avoid conflicts, giving every test the isolation that jtreg does when it sets test.classes uniquely per test in newer JDK releases.
>> 
>> Thanks,
>> Martin.-
>> 
>> --
>> [1] - https://github.com/openjdk/jdk11u/blob/master/test/jdk/TEST.ROOT#L61
>> [2] - https://github.com/openjdk/jtreg/blob/jtreg-6.2%2B1/src/share/classes/com/sun/javatest/regtest/config/Locations.java#L174
>
>> The 11u backport applies cleanly, once paths are fixed.
> 
> Yes, that seems to be the case if https://github.com/openjdk/jdk8u-dev/pull/106/commits/73173863ab9da7ee2ec080401180475bf7ee22e1 is to be looked at on its own.
>  
>> To minimize risks, I manually debugged the OpenJDK code that connected to an NSS DB using the sql: prefix and everything was as expected.
> 
> OK.
> 
>> I also run all the tests under the jdk/sun/security/pkcs11 category and noticed no-regressions strictly related to this fix. The test 'jdk/sun/security/pkcs11/Secmod/TrustAnchors.java' requires a special note, though. This test passes if run standalone but fails when run as part of a group. The reason is because when run as part of a group, other tests copy the file 'pkcs11.txt' to the temporary nssdb directory which is shared between all of them. This DB does not register the trust anchors module called 'Builtin Roots Module', which was available in the old secmod.db and is required by the test (which expects to see a Secmod$Module of TRUSTANCHOR type). While the test leaves the sun.security.pkcs11.SecmodTest::useSqlite field set to false and tries to access the legacy DB (also contained in the shared temporary nssdb directory), NSS realizes that there is a new SQLite DB in the same directory and reads the configuration from pkcs11.txt. Otherwise, if pkcs11.txt does not exist (as
  when the test is run standalone), it creates one with the information in secmod.db and it registers the 'Builtin Roots Module' module. All these findings have been verified with the 'modutil -list' command. While the issue is unrelated to what JDK-8195607 fixes, it has been uncovered by the existence of the pkcs11.txt file introduced with this change.
>> 
>> Why newer releases are not affected? Because jtreg runs tests (including TrustAnchors.java) in separated directories, so they don't share the nssdb temporary directory. In newer releases, TEST.ROOT has a requiredVersion attribute [1] indicating to jtreg that this can be done [2].
>> 
>> I've analyzed several ways to fix this issue but the one that I'd like to propose is to clean up the temporary nssdb directory for every test and avoid conflicts, giving every test the isolation that jtreg does when it sets test.classes uniquely per test in newer JDK releases.
> 
> This seems strange, wouldn't a jtreg version `> 4.2 b08` not run into this problem? I.e. if I run with jtreg 5 (oldest version I have here), I wouldn't see this issue? If so I'm not sure this is worth adding https://github.com/openjdk/jdk8u-dev/pull/106/commits/22c1df05d2ab51a400ebdc69566e84fdaf32efec to this backport. It's not in head either. So at the very least this should be an 8u-only bug. But if that's only happening with jtreg 4, I'd say this is not worth fixing at all (or perhaps add jtreg 5 as a minimal version. Everyone should be on newer versions these days anyway.

Hi @jerboaa ,

Thanks for your review.

Let me clarify the jtreg issue. It's not that the test fails because it's run with an old jtreg. In fact, I'm running with jtreg 5 and it's failing. The problem is that the TEST.ROOT in JDK-8 does not set a "minimum jtreg required" (probably because it's a newer feature than JDK-8). In these cases, jtreg takes a conservative approach and does not split the text execution into different directories. Setting a "minimum jtreg required" in JDK-8 would be a high risk as we don't know the impact of other tests in newer or older jtreg releases. Having this test broken would be a regression. I hope we can have this fix, or you might come up with a different one. Let me know if that helps for a better understanding.

Martin.-

-------------

PR: https://git.openjdk.org/jdk8u-dev/pull/106