Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Martin Balao]

Hi Sean,

This is a simple patch that contains:

 1) a fix on the SQL db prefix: it should be sql: instead of sql:/ to avoid
path errors (triggered on a new NSS library version);
 2) minor enhancement to specify a pkcs11.txt configuration file (as
secmod.db was used before, for the legacy db); and,
 3) some minor debugging enhancements to get error codes out of NSS.

I'll be grateful if someone can have a look at it. This proposal fixes an
existing bug, that can be verified with Oracle's internal tests.

I'm CC' Max to this email, as he was the one who notified me about the bug.

Kind regards,
Martin.-

On Wed, Mar 14, 2018 at 3:04 PM, Seán Coffey  wrote:

> I'll have a look Martin, but it'll be better if we can get an NSS or
> PKCS11 expert to take a look. Any takers ? Can you expand some bit on the
> exact reason for your 8195607 changes ? Pointers to NSS changes etc. ?
>
> Regards,
> Sean.
>
> On 14/03/18 16:11, Martin Balao wrote:
>
> Hi Sean,
>
> Thanks!
>
> Can you please review the fix [0] so we have it in? As far as I know, fix
> makes Oracle internal tests pass.
>
> Kind regards,
> Martin.-
>
> --
> [0] - http://mail.openjdk.java.net/pipermail/security-dev/2018-
> February/016776.html
>
> On Wed, Mar 14, 2018 at 12:05 PM, Seán Coffey 
> wrote:
>
>> Hi Martin,
>>
>> Thanks for the 8195607 pointer. I'll get this ported to jdk8u also. I
>> didn't see that actual issue during testing but no harm to port it. Will
>> submit a new webrev shortly.
>>
>> Regards,
>> Sean.
>>
>> On 14/03/18 14:55, Martin Balao wrote:
>>
>> Hi Sean,
>>
>> Is this related to http://mail.openjdk.java.ne
>> t/pipermail/security-dev/2018-February/016776.html ?
>>
>> Kind regards,
>> Martin.-
>>
>> On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey 
>> wrote:
>>
>>> Looking to backport this fix to jdk8u-dev.  Contributed to JDK Project
>>> by Martin Balao.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8165996
>>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/
>>>
>>> The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't seem
>>> applicable to jdk8u.
>>>
>>> Also, I edited sun/security/pkcs11/Secmod/TestNssDbSqlite.java to
>>> exclude test where NSS Provider
>>> didn't register. (line 66). In particular, I saw this while running
>>> 32bit JDK tests on 64 bit Linux.
>>>
>>> --
>>> Regards,
>>> Sean.
>>>
>>>
>>
>>
>
>

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Seán Coffey]

I'll have a look Martin, but it'll be better if we can get an NSS or 
PKCS11 expert to take a look. Any takers ? Can you expand some bit on 
the exact reason for your 8195607 changes ? Pointers to NSS changes etc. ?


Regards,
Sean.

On 14/03/18 16:11, Martin Balao wrote:

Hi Sean,

Thanks!

Can you please review the fix [0] so we have it in? As far as I know, 
fix makes Oracle internal tests pass.


Kind regards,
Martin.-

--
[0] - 
http://mail.openjdk.java.net/pipermail/security-dev/2018-February/016776.html 



On Wed, Mar 14, 2018 at 12:05 PM, Seán Coffey > wrote:


Hi Martin,

Thanks for the 8195607 pointer. I'll get this ported to jdk8u
also. I didn't see that actual issue during testing but no harm to
port it. Will submit a new webrev shortly.

Regards,
Sean.

On 14/03/18 14:55, Martin Balao wrote:

Hi Sean,

Is this related to

http://mail.openjdk.java.net/pipermail/security-dev/2018-February/016776.html

 
?

Kind regards,
Martin.-

On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey
> wrote:

Looking to backport this fix to jdk8u-dev.  Contributed to
JDK Project by Martin Balao.

https://bugs.openjdk.java.net/browse/JDK-8165996

webrev :
http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/


The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't
seem applicable to jdk8u.

Also, I edited
sun/security/pkcs11/Secmod/TestNssDbSqlite.java to exclude
test where NSS Provider
didn't register. (line 66). In particular, I saw this while
running 32bit JDK tests on 64 bit Linux.

-- 
Regards,

Sean.

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Martin Balao]

Hi Sean,

Thanks!

Can you please review the fix [0] so we have it in? As far as I know, fix
makes Oracle internal tests pass.

Kind regards,
Martin.-

--
[0] - http://mail.openjdk.java.net/pipermail/security-dev/
2018-February/016776.html

On Wed, Mar 14, 2018 at 12:05 PM, Seán Coffey 
wrote:

> Hi Martin,
>
> Thanks for the 8195607 pointer. I'll get this ported to jdk8u also. I
> didn't see that actual issue during testing but no harm to port it. Will
> submit a new webrev shortly.
>
> Regards,
> Sean.
>
> On 14/03/18 14:55, Martin Balao wrote:
>
> Hi Sean,
>
> Is this related to http://mail.openjdk.java.net/pipermail/security-dev/
> 2018-February/016776.html ?
>
> Kind regards,
> Martin.-
>
> On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey 
> wrote:
>
>> Looking to backport this fix to jdk8u-dev.  Contributed to JDK Project by
>> Martin Balao.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8165996
>> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/
>>
>> The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't seem
>> applicable to jdk8u.
>>
>> Also, I edited sun/security/pkcs11/Secmod/TestNssDbSqlite.java to
>> exclude test where NSS Provider
>> didn't register. (line 66). In particular, I saw this while running 32bit
>> JDK tests on 64 bit Linux.
>>
>> --
>> Regards,
>> Sean.
>>
>>
>
>

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Seán Coffey]

Hi Martin,

Thanks for the 8195607 pointer. I'll get this ported to jdk8u also. I 
didn't see that actual issue during testing but no harm to port it. Will 
submit a new webrev shortly.


Regards,
Sean.

On 14/03/18 14:55, Martin Balao wrote:

Hi Sean,

Is this related to 
http://mail.openjdk.java.net/pipermail/security-dev/2018-February/016776.html ?


Kind regards,
Martin.-

On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey > wrote:


Looking to backport this fix to jdk8u-dev.  Contributed to JDK
Project by Martin Balao.

https://bugs.openjdk.java.net/browse/JDK-8165996

webrev :
http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/


The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't seem
applicable to jdk8u.

Also, I edited sun/security/pkcs11/Secmod/TestNssDbSqlite.java to
exclude test where NSS Provider
didn't register. (line 66). In particular, I saw this while
running 32bit JDK tests on 64 bit Linux.

-- 
Regards,

Sean.

Re: RFR: 8165996:PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Martin Balao]

Hi Sean,

Is this related to
http://mail.openjdk.java.net/pipermail/security-dev/2018-February/016776.html
 ?

Kind regards,
Martin.-

On Wed, Mar 14, 2018 at 11:48 AM, Seán Coffey 
wrote:

> Looking to backport this fix to jdk8u-dev.  Contributed to JDK Project by
> Martin Balao.
>
> https://bugs.openjdk.java.net/browse/JDK-8165996
> webrev : http://cr.openjdk.java.net/~coffeys/webrev.8165996.8u/webrev/
>
> The test/jdk/sun/security/pkcs11/PKCS11Test.java edits didn't seem
> applicable to jdk8u.
>
> Also, I edited sun/security/pkcs11/Secmod/TestNssDbSqlite.java to exclude
> test where NSS Provider
> didn't register. (line 66). In particular, I saw this while running 32bit
> JDK tests on 64 bit Linux.
>
> --
> Regards,
> Sean.
>
>

Re: RFR 8165996: PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Weijun Wang]

http://hg.openjdk.java.net/jdk/jdk/rev/55b9b1e184c6

> On Dec 13, 2017, at 1:17 AM, Martin Balao  wrote:
> 
> Hi Max,
> 
> Thanks for your time and review.
> 
> Test refactorings look good to me :-)
> 
> In regard to pkcs11.txt, we are currently using the cfg file to store 
> configuration information (as before). I suggest to keep using it to leverage 
> on previous work. The only change we are actually doing to configuration 
> information is the "sql:/" prefix on the db path (to indicate a sqllite db).
> 
> Kind regards,
> Martin.-

Re: RFR 8165996: PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Martin Balao]

Hi Max,

Thanks for your time and review.

Test refactorings look good to me :-)

In regard to pkcs11.txt, we are currently using the cfg file to store
configuration information (as before). I suggest to keep using it to
leverage on previous work. The only change we are actually doing to
configuration information is the "sql:/" prefix on the db path (to indicate
a sqllite db).

Kind regards,
Martin.-


On Mon, Dec 11, 2017 at 11:40 AM, Weijun Wang 
wrote:

> Hi Martin
>
> Your src change looks fine, and if you think my test update is good, I can
> push the changeset.
>
> Still, I need one confirmation. The modutil man page has "modutil supports
> two types of databases: the legacy security
> databases (cert8.db, key3.db, and secmod.db) and new SQLite databases
> (cert9.db, key4.db, and pkcs11.txt)". So it looks like the pkcs11.txt file
> is optional?
>
> Thanks
> Max
>
>
> > On Dec 11, 2017, at 11:16 AM, Weijun Wang 
> wrote:
> >
> >
> >
> >> On Dec 8, 2017, at 4:55 PM, Weijun Wang  wrote:
> >>
> >> Hi Martin
> >>
> >> I've made some change and post a new webrev at
> >>
> >> http://cr.openjdk.java.net/~weijun/8165996/webrev.00/
> >
> > More change in the same URL.
> >
> > - key4.db and cert9.db are saved in Secmod.
> >
> > - I modified the existing PKCS11Test/SecmodTest to load sqlite based dbs.
> >
> > Thanks
> > Max
> >
> >>
> >> The src part is unchanged. Major changes to test are:
> >>
> >> 1. PKCS11Test.getNSSLibDir() is used to get the NSS lib dir. Honestly
> this is my 1st time touching NSS so hopefully it's not wrong.
> >>
> >> 2. I didn't used your private key and certs. Instead, an internal class
> CertAndKeyGen is used.
> >>
> >> 3. I've saved "key4.db" and "cert9.db" as real files inside nss/sqlite.
> I know binary files are extremely unwelcome in an open source project, but
> maybe this time this is acceptable. We already have nss/db and nss/sqlite
> is certainly not worse, and maybe we can write more test using this backend
> later.
> >>
> >> 4. I also moved "nssdbsqlite" from /tmp to the current working
> directory. For jtreg, cwd is always empty and will be cleaned/retained
> after a test run. More importantly, no two test runs will use the same cwd.
> >>
> >> So nothing really changed. I still need to read about sql:/ to
> understand the src fix.
> >>
> >> Thanks
> >> Max
> >>
> >>> On Dec 8, 2017, at 2:33 PM, Weijun Wang 
> wrote:
> >>>
> >>> Hi Martin
> >>>
> >>> I'm just starting to read this patch. Two questions:
> >>>
> >>> 1. Is there a webpage on configDir using sql:/?
> >>>
> >>> 2. Your test hardcoded nssLibraryDirectory to be "/lib64". It would
> need to be changed to either those inclosed the repository (macOS and
> Windows) or in the system (others). Is there a version requirement?
> >>>
> >>> 3. The test contains a lot of binary data. Can you describe more
> clearly on which is from where? Especially key4Content and cert9Content? In
> fact, can they be recreated from the existing file based db inside
> test/jdk/sun/security/pkcs11/nss/db? If yes, the test will be much
> shorter. Please at least use multiple lines for the 2 keys.
> >>>
> >>> Thanks
> >>> Max
> >>>
>  On Nov 29, 2017, at 10:11 PM, Martin Balao  wrote:
> 
>  Hi,
> 
>  I'd like to propose a fix for JDK-8165996 - PKCS11 using NSS throws
> an error regarding secmod.db when NSS uses sqlite [1].
> 
>  Webrev01:
> 
>  * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01/
> (browse online)
>  * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01.zip
> (download)
> 
>  Kind regards,
>  Martin.-
> 
>  --
>  [1] - https://bugs.openjdk.java.net/browse/JDK-8165996
> >>>
> >>
> >
>
>

Re: RFR 8165996: PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Weijun Wang]

Hi Martin

Your src change looks fine, and if you think my test update is good, I can push 
the changeset.

Still, I need one confirmation. The modutil man page has "modutil supports two 
types of databases: the legacy security
databases (cert8.db, key3.db, and secmod.db) and new SQLite databases 
(cert9.db, key4.db, and pkcs11.txt)". So it looks like the pkcs11.txt file is 
optional?

Thanks
Max


> On Dec 11, 2017, at 11:16 AM, Weijun Wang  wrote:
> 
> 
> 
>> On Dec 8, 2017, at 4:55 PM, Weijun Wang  wrote:
>> 
>> Hi Martin
>> 
>> I've made some change and post a new webrev at
>> 
>> http://cr.openjdk.java.net/~weijun/8165996/webrev.00/
> 
> More change in the same URL.
> 
> - key4.db and cert9.db are saved in Secmod.
> 
> - I modified the existing PKCS11Test/SecmodTest to load sqlite based dbs.
> 
> Thanks
> Max
> 
>> 
>> The src part is unchanged. Major changes to test are:
>> 
>> 1. PKCS11Test.getNSSLibDir() is used to get the NSS lib dir. Honestly this 
>> is my 1st time touching NSS so hopefully it's not wrong.
>> 
>> 2. I didn't used your private key and certs. Instead, an internal class 
>> CertAndKeyGen is used.
>> 
>> 3. I've saved "key4.db" and "cert9.db" as real files inside nss/sqlite. I 
>> know binary files are extremely unwelcome in an open source project, but 
>> maybe this time this is acceptable. We already have nss/db and nss/sqlite is 
>> certainly not worse, and maybe we can write more test using this backend 
>> later.
>> 
>> 4. I also moved "nssdbsqlite" from /tmp to the current working directory. 
>> For jtreg, cwd is always empty and will be cleaned/retained after a test 
>> run. More importantly, no two test runs will use the same cwd.
>> 
>> So nothing really changed. I still need to read about sql:/ to understand 
>> the src fix.
>> 
>> Thanks
>> Max
>> 
>>> On Dec 8, 2017, at 2:33 PM, Weijun Wang  wrote:
>>> 
>>> Hi Martin
>>> 
>>> I'm just starting to read this patch. Two questions:
>>> 
>>> 1. Is there a webpage on configDir using sql:/?
>>> 
>>> 2. Your test hardcoded nssLibraryDirectory to be "/lib64". It would need to 
>>> be changed to either those inclosed the repository (macOS and Windows) or 
>>> in the system (others). Is there a version requirement?
>>> 
>>> 3. The test contains a lot of binary data. Can you describe more clearly on 
>>> which is from where? Especially key4Content and cert9Content? In fact, can 
>>> they be recreated from the existing file based db inside 
>>> test/jdk/sun/security/pkcs11/nss/db? If yes, the test will be much shorter. 
>>> Please at least use multiple lines for the 2 keys.
>>> 
>>> Thanks
>>> Max
>>> 
 On Nov 29, 2017, at 10:11 PM, Martin Balao  wrote:
 
 Hi,
 
 I'd like to propose a fix for JDK-8165996 - PKCS11 using NSS throws an 
 error regarding secmod.db when NSS uses sqlite [1].
 
 Webrev01:
 
 * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01/ (browse 
 online)
 * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01.zip 
 (download)
 
 Kind regards,
 Martin.-
 
 --
 [1] - https://bugs.openjdk.java.net/browse/JDK-8165996
>>> 
>> 
> 

Re: RFR 8165996: PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Weijun Wang]

> On Dec 8, 2017, at 4:55 PM, Weijun Wang  wrote:
> 
> Hi Martin
> 
> I've made some change and post a new webrev at
> 
>  http://cr.openjdk.java.net/~weijun/8165996/webrev.00/

More change in the same URL.

- key4.db and cert9.db are saved in Secmod.

- I modified the existing PKCS11Test/SecmodTest to load sqlite based dbs.

Thanks
Max

> 
> The src part is unchanged. Major changes to test are:
> 
> 1. PKCS11Test.getNSSLibDir() is used to get the NSS lib dir. Honestly this is 
> my 1st time touching NSS so hopefully it's not wrong.
> 
> 2. I didn't used your private key and certs. Instead, an internal class 
> CertAndKeyGen is used.
> 
> 3. I've saved "key4.db" and "cert9.db" as real files inside nss/sqlite. I 
> know binary files are extremely unwelcome in an open source project, but 
> maybe this time this is acceptable. We already have nss/db and nss/sqlite is 
> certainly not worse, and maybe we can write more test using this backend 
> later.
> 
> 4. I also moved "nssdbsqlite" from /tmp to the current working directory. For 
> jtreg, cwd is always empty and will be cleaned/retained after a test run. 
> More importantly, no two test runs will use the same cwd.
> 
> So nothing really changed. I still need to read about sql:/ to understand the 
> src fix.
> 
> Thanks
> Max
> 
>> On Dec 8, 2017, at 2:33 PM, Weijun Wang  wrote:
>> 
>> Hi Martin
>> 
>> I'm just starting to read this patch. Two questions:
>> 
>> 1. Is there a webpage on configDir using sql:/?
>> 
>> 2. Your test hardcoded nssLibraryDirectory to be "/lib64". It would need to 
>> be changed to either those inclosed the repository (macOS and Windows) or in 
>> the system (others). Is there a version requirement?
>> 
>> 3. The test contains a lot of binary data. Can you describe more clearly on 
>> which is from where? Especially key4Content and cert9Content? In fact, can 
>> they be recreated from the existing file based db inside 
>> test/jdk/sun/security/pkcs11/nss/db? If yes, the test will be much shorter. 
>> Please at least use multiple lines for the 2 keys.
>> 
>> Thanks
>> Max
>> 
>>> On Nov 29, 2017, at 10:11 PM, Martin Balao  wrote:
>>> 
>>> Hi,
>>> 
>>> I'd like to propose a fix for JDK-8165996 - PKCS11 using NSS throws an 
>>> error regarding secmod.db when NSS uses sqlite [1].
>>> 
>>> Webrev01:
>>> 
>>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01/ (browse 
>>> online)
>>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01.zip (download)
>>> 
>>> Kind regards,
>>> Martin.-
>>> 
>>> --
>>> [1] - https://bugs.openjdk.java.net/browse/JDK-8165996
>> 
> 

Re: RFR 8165996: PKCS11 using NSS throws an error regarding secmod.db when NSS uses sqlite

[Weijun Wang]

Hi Martin

I've made some change and post a new webrev at

  http://cr.openjdk.java.net/~weijun/8165996/webrev.00/

The src part is unchanged. Major changes to test are:

1. PKCS11Test.getNSSLibDir() is used to get the NSS lib dir. Honestly this is 
my 1st time touching NSS so hopefully it's not wrong.

2. I didn't used your private key and certs. Instead, an internal class 
CertAndKeyGen is used.

3. I've saved "key4.db" and "cert9.db" as real files inside nss/sqlite. I know 
binary files are extremely unwelcome in an open source project, but maybe this 
time this is acceptable. We already have nss/db and nss/sqlite is certainly not 
worse, and maybe we can write more test using this backend later.

4. I also moved "nssdbsqlite" from /tmp to the current working directory. For 
jtreg, cwd is always empty and will be cleaned/retained after a test run. More 
importantly, no two test runs will use the same cwd.

So nothing really changed. I still need to read about sql:/ to understand the 
src fix.

Thanks
Max

> On Dec 8, 2017, at 2:33 PM, Weijun Wang  wrote:
> 
> Hi Martin
> 
> I'm just starting to read this patch. Two questions:
> 
> 1. Is there a webpage on configDir using sql:/?
> 
> 2. Your test hardcoded nssLibraryDirectory to be "/lib64". It would need to 
> be changed to either those inclosed the repository (macOS and Windows) or in 
> the system (others). Is there a version requirement?
> 
> 3. The test contains a lot of binary data. Can you describe more clearly on 
> which is from where? Especially key4Content and cert9Content? In fact, can 
> they be recreated from the existing file based db inside 
> test/jdk/sun/security/pkcs11/nss/db? If yes, the test will be much shorter. 
> Please at least use multiple lines for the 2 keys.
> 
> Thanks
> Max
> 
>> On Nov 29, 2017, at 10:11 PM, Martin Balao  wrote:
>> 
>> Hi,
>> 
>> I'd like to propose a fix for JDK-8165996 - PKCS11 using NSS throws an error 
>> regarding secmod.db when NSS uses sqlite [1].
>> 
>> Webrev01:
>> 
>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01/ (browse 
>> online)
>> * http://cr.openjdk.java.net/~akasko/mbalao/8165996.webrev.01.zip (download)
>> 
>> Kind regards,
>> Martin.-
>> 
>> --
>> [1] - https://bugs.openjdk.java.net/browse/JDK-8165996
>