Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]
On Wed, 22 Dec 2021 01:18:58 GMT, Stuart Marks wrote: >> Enable the security manager in rmiregistry's launcher arguments. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Change java.security.manager to "allow"; filter warning lines from > VersionCheck Enabling the security manager using a property, versus setting the property to `allow` and enabling it in code, is mostly a distinction without a difference. I don't think there is really a need to have the different tools be consistent. Local tool considerations probably swamp the need for any cross-tool consistency. In this case some of the RMI registry tests set the property to `allow` on the command line and then rely on the code to enable the security manager using the API, so it's much easier to switch the `rmiregistry` launcher to use the `allow` technique instead. This is sort-of the tail (the tests) wagging the doc (the tool) but there doesn't seem to any benefit to be gained from fiddling around with the tests and the RMI test library. I've also updated VersionCheck.java to filter out the security manager warning messages. - PR: https://git.openjdk.java.net/jdk18/pull/45
Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled [v2]
> Enable the security manager in rmiregistry's launcher arguments. Stuart Marks has updated the pull request incrementally with one additional commit since the last revision: Change java.security.manager to "allow"; filter warning lines from VersionCheck - Changes: - all: https://git.openjdk.java.net/jdk18/pull/45/files - new: https://git.openjdk.java.net/jdk18/pull/45/files/e899bd2d..f713e731 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk18=45=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk18=45=00-01 Stats: 7 lines in 2 files changed: 3 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk18/pull/45.diff Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/45/head:pull/45 PR: https://git.openjdk.java.net/jdk18/pull/45
Re: RFR: 8209398: sun/security/pkcs11/KeyStore/SecretKeysBasic.sh failed with "PKCS11Exception: CKR_ATTRIBUTE_SENSITIVE"
On Tue, 14 Dec 2021 18:33:47 GMT, Valerie Peng wrote: > Can someone help review this small fix? NSS returns PKCS11 > CKR_ATTRIBUTE_SENSITIVE error when trying to retrieve CKA_VALUE out of its > token keys. So this fix is to add special handling for NSS token secret keys. > There is already an existing regression test which detects this and disabled > in ProblemList.txt. Removing that test from ProblemList.txt to verify this > fix. > > Thanks, > Valerie Since the return error code is PKCS11 CKR_ATTRIBUTE_SENSITIVE, does it make sense to assign `sensitive = true` right at the beginning? I'm not a PKCS11 expert and not sure if this has any negative effect on https://github.com/openjdk/jdk/blob/ea8d3c92c69c393cdbc6c62398f1e9c6adc708d3/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java#L1394 (where the `sensitive` flag is used). - PR: https://git.openjdk.java.net/jdk/pull/6837
Re: RFR: 8209398: sun/security/pkcs11/KeyStore/SecretKeysBasic.sh failed with "PKCS11Exception: CKR_ATTRIBUTE_SENSITIVE"
On Tue, 14 Dec 2021 18:33:47 GMT, Valerie Peng wrote: > Can someone help review this small fix? NSS returns PKCS11 > CKR_ATTRIBUTE_SENSITIVE error when trying to retrieve CKA_VALUE out of its > token keys. So this fix is to add special handling for NSS token secret keys. > There is already an existing regression test which detects this and disabled > in ProblemList.txt. Removing that test from ProblemList.txt to verify this > fix. > > Thanks, > Valerie What about here? https://github.com/openjdk/jdk/blob/a5d7de235101696463dba22792703c6809ff7fc4/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11RSAKeyFactory.java#L280 - PR: https://git.openjdk.java.net/jdk/pull/6837
Re: RFR: 8209398: sun/security/pkcs11/KeyStore/SecretKeysBasic.sh failed with "PKCS11Exception: CKR_ATTRIBUTE_SENSITIVE"
On Tue, 14 Dec 2021 18:33:47 GMT, Valerie Peng wrote: > Can someone help review this small fix? NSS returns PKCS11 > CKR_ATTRIBUTE_SENSITIVE error when trying to retrieve CKA_VALUE out of its > token keys. So this fix is to add special handling for NSS token secret keys. > There is already an existing regression test which detects this and disabled > in ProblemList.txt. Removing that test from ProblemList.txt to verify this > fix. > > Thanks, > Valerie Marked as reviewed by hchao (Committer). Looks good. - PR: https://git.openjdk.java.net/jdk/pull/6837
Re: RFR: 8279066: Still see private key entries without certificates in a PKCS12 keystore
On 12/21/2021 1:26 PM, Sean Mullan wrote: On Tue, 21 Dec 2021 16:31:57 GMT, Weijun Wang wrote: Before password-less PKCS12 keystores are supported, certificates in a PKCS12 file are always encrypted. Therefore if one loads the keystore with a null pass, it contains `PrivateKeyEntry`s without certificates. This has always been awkward (and most likely useless) so when JDK-8076190 introduced the password-less feature I also added a line to remove such an entry. https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 Unfortunately, the line is not coded correctly, it should have been `remove(key)` but here it's `remove(value)`. This code change correctly removes the entry. That said, this behavior, although weird, has been there from the beginning since PKCS12 keystore was introduced. If you can find out a usage of a private key entry without any certificate and think it's worth kept that way, I can simply remove the `remove` call and leave the entry there. I still think it's useful even if I can't see the certificate chain. I'd rather see the entry if it actually exists in the keystore and I think removing it is odd because it still exists in the keystore. Also, sometimes I use keytool without a storepass just to see what is in it, and then if I see the certificates are not showing up, I can try again with the password. - PR: https://git.openjdk.java.net/jdk/pull/6910 I got curious and took a look at P11KeyStore.java - https://github.com/openjdk/jdk/blob/master/src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11KeyStore.java - and it appears to do exactly the opposite of what you've got here. E.g. if there's no certificate that matches up with the private key, then it returns a null - that's in engineGetEntry() around line 963. Speaking personally, I've always found it a bit annoying that a KeyStore.PrivateKeyEntry required a certificate(chain). It actually made dealing with PKCS11 painful as the Java conventions didn't always conform to the actual structure of the PKCS11 contents. Not suggesting that a change necessarily needs to be made, but perhaps you might want to have a consistent behavior between the two? Mike
Integrated: 8279066: entries.remove(entry) is useless in PKCS12KeyStore
On Tue, 21 Dec 2021 16:31:57 GMT, Weijun Wang wrote: > Before password-less PKCS12 keystores are supported, certificates in a PKCS12 > file are always encrypted. Therefore if one loads the keystore with a null > pass, it contains `PrivateKeyEntry`s without certificates. This has always > been awkward (and most likely useless) so when JDK-8076190 introduced the > password-less feature I also added a line to remove such an entry. > > https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 > > > Unfortunately, the line is not coded correctly, it should have been > `remove(key)` but here it's `remove(value)`. > > This code change correctly removes the entry. > > That said, this behavior, although weird, has been there from the beginning > since PKCS12 keystore was introduced. If you can find out a usage of a > private key entry without any certificate and think it's worth kept that way, > I can simply remove the `remove` call and leave the entry there. This pull request has now been integrated. Changeset: fb623f1d Author:Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/fb623f1d2ee858fbc6edfeaaa702b5fcd832a0aa Stats: 5 lines in 1 file changed: 0 ins; 5 del; 0 mod 8279066: entries.remove(entry) is useless in PKCS12KeyStore Reviewed-by: mullan - PR: https://git.openjdk.java.net/jdk/pull/6910
Re: [jdk18] RFR: JDK-8278967 rmiregistry fails to start because SecurityManager is disabled
On Sun, 19 Dec 2021 07:37:19 GMT, Alan Bateman wrote: >> Enable the security manager in rmiregistry's launcher arguments. > > As things stand, `rmiregsitry -J-Djava.security.manager` and `rmiregistry > -J-Djava.security.manager=allow` are equivalent because rmiregistry sets the > default SM. Some difference may be observed if someone is running rmiregistry > with a custom system class loader set, or custom file system provider, or > running it with a JVM TI agent that is looking at events between VMStart and > VMInit but these seem somewhat obscure scenarios for a command line program > If rmiregstry worked with ToolProvider then I think there would be more to > discuss. If the final patch is to have the launcher set the default security > manager then we may want to change RegistryImpl.createRegistry to fail if not > set. > > The warning that is emitted for both cases is expected. JEP 411 is very clear > that it there is no mechanism to suppress it. We may need to add a release > note to document that rmiregistry will emit a warning a startup, that will at > least be something to point at in the event that anyone asks or reports an > issue. In the same kind of PR (https://github.com/openjdk/jdk18/pull/53) for `jstatd` @AlanBateman writes: > This looks okay in that it is now worked exactly as it did in JDK 17. And that PR uses `allow` as I have suggested here (to preserve existing behaviour). All the affected launchers should be fixed in the same consistent way. - PR: https://git.openjdk.java.net/jdk18/pull/45
Re: RFR: 8279066: entries.remove(entry) is useless in PKCS12KeyStore [v2]
On Tue, 21 Dec 2021 21:39:23 GMT, Weijun Wang wrote: >> Before password-less PKCS12 keystores are supported, certificates in a >> PKCS12 file are always encrypted. Therefore if one loads the keystore with a >> null pass, it contains `PrivateKeyEntry`s without certificates. This has >> always been awkward (and most likely useless) so when JDK-8076190 introduced >> the password-less feature I also added a line to remove such an entry. >> >> https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 >> >> >> Unfortunately, the line is not coded correctly, it should have been >> `remove(key)` but here it's `remove(value)`. >> >> This code change correctly removes the entry. >> >> That said, this behavior, although weird, has been there from the beginning >> since PKCS12 keystore was introduced. If you can find out a usage of a >> private key entry without any certificate and think it's worth kept that >> way, I can simply remove the `remove` call and leave the entry there. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > just ignore it Marked as reviewed by mullan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6910
Re: RFR: 8279066: entries.remove(entry) is useless in PKCS12KeyStore
On Tue, 21 Dec 2021 16:31:57 GMT, Weijun Wang wrote: > Before password-less PKCS12 keystores are supported, certificates in a PKCS12 > file are always encrypted. Therefore if one loads the keystore with a null > pass, it contains `PrivateKeyEntry`s without certificates. This has always > been awkward (and most likely useless) so when JDK-8076190 introduced the > password-less feature I also added a line to remove such an entry. > > https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 > > > Unfortunately, the line is not coded correctly, it should have been > `remove(key)` but here it's `remove(value)`. > > This code change correctly removes the entry. > > That said, this behavior, although weird, has been there from the beginning > since PKCS12 keystore was introduced. If you can find out a usage of a > private key entry without any certificate and think it's worth kept that way, > I can simply remove the `remove` call and leave the entry there. OK, I've updated the change to simply removing that `remove` all. - PR: https://git.openjdk.java.net/jdk/pull/6910
Re: RFR: 8279066: entries.remove(entry) is useless in PKCS12KeyStore [v2]
> Before password-less PKCS12 keystores are supported, certificates in a PKCS12 > file are always encrypted. Therefore if one loads the keystore with a null > pass, it contains `PrivateKeyEntry`s without certificates. This has always > been awkward (and most likely useless) so when JDK-8076190 introduced the > password-less feature I also added a line to remove such an entry. > > https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 > > > Unfortunately, the line is not coded correctly, it should have been > `remove(key)` but here it's `remove(value)`. > > This code change correctly removes the entry. > > That said, this behavior, although weird, has been there from the beginning > since PKCS12 keystore was introduced. If you can find out a usage of a > private key entry without any certificate and think it's worth kept that way, > I can simply remove the `remove` call and leave the entry there. Weijun Wang has updated the pull request incrementally with one additional commit since the last revision: just ignore it - Changes: - all: https://git.openjdk.java.net/jdk/pull/6910/files - new: https://git.openjdk.java.net/jdk/pull/6910/files/ed6ee075..844bf487 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6910=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6910=00-01 Stats: 98 lines in 3 files changed: 0 ins; 92 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6910.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6910/head:pull/6910 PR: https://git.openjdk.java.net/jdk/pull/6910
Re: RFR: 8277881 Missing SessionID in TLS1.3 resumption in compatibility mode
On Mon, 29 Nov 2021 08:42:22 GMT, Daniel Jeliński wrote: > All TLS 1.3 handshakes in compatibility mode must send a non-empty SessionID > field. Currently TLS1.3 session resumptions are sending empty session ID. > This patch fixes that problem. > > All jdk_core tests passed. The newly added check passes with the patch, fails > without it. Please add " 2021," to the copyright of ResumeTLS13withSNI.java. I have run all the tests and they pass. Have you run this fix on your customer's setup or similar setup to confirm this fixed their problem? - Changes requested by ascarpino (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6583
Integrated: 8279043: Some Security Exception Messages Miss Spaces
On Mon, 20 Dec 2021 14:50:08 GMT, Daniel Jeliński wrote: > Trivial change. Issue reported by > [lgtm.com](https://lgtm.com/projects/g/openjdk/jdk/alerts/?mode=tree=java=1952840153). This pull request has now been integrated. Changeset: f31dead6 Author:Daniel Jelinski Committer: Weijun Wang URL: https://git.openjdk.java.net/jdk/commit/f31dead6c550444a836655ffdf97df8816e2d546 Stats: 21 lines in 16 files changed: 0 ins; 0 del; 21 mod 8279043: Some Security Exception Messages Miss Spaces Reviewed-by: weijun - PR: https://git.openjdk.java.net/jdk/pull/6894
Re: RFR: 8279066: Still see private key entries without certificates in a PKCS12 keystore
On Tue, 21 Dec 2021 16:31:57 GMT, Weijun Wang wrote: > Before password-less PKCS12 keystores are supported, certificates in a PKCS12 > file are always encrypted. Therefore if one loads the keystore with a null > pass, it contains `PrivateKeyEntry`s without certificates. This has always > been awkward (and most likely useless) so when JDK-8076190 introduced the > password-less feature I also added a line to remove such an entry. > > https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 > > > Unfortunately, the line is not coded correctly, it should have been > `remove(key)` but here it's `remove(value)`. > > This code change correctly removes the entry. > > That said, this behavior, although weird, has been there from the beginning > since PKCS12 keystore was introduced. If you can find out a usage of a > private key entry without any certificate and think it's worth kept that way, > I can simply remove the `remove` call and leave the entry there. I still think it's useful even if I can't see the certificate chain. I'd rather see the entry if it actually exists in the keystore and I think removing it is odd because it still exists in the keystore. Also, sometimes I use keytool without a storepass just to see what is in it, and then if I see the certificates are not showing up, I can try again with the password. - PR: https://git.openjdk.java.net/jdk/pull/6910
Re: RFR: 8279043: Some Security Exception Messages Miss Spaces
On Mon, 20 Dec 2021 14:50:08 GMT, Daniel Jeliński wrote: > Trivial change. Issue reported by > [lgtm.com](https://lgtm.com/projects/g/openjdk/jdk/alerts/?mode=tree=java=1952840153). Thanks. I'll run some tests before sponsoring it. Hopefully no one uses the exact exception message as a golden copy. - PR: https://git.openjdk.java.net/jdk/pull/6894
Re: RFR: 8279043: Some Security Exception Messages Miss Spaces
On Mon, 20 Dec 2021 14:50:08 GMT, Daniel Jeliński wrote: > Trivial change. Issue reported by > [lgtm.com](https://lgtm.com/projects/g/openjdk/jdk/alerts/?mode=tree=java=1952840153). Thanks! bug title fixed. - PR: https://git.openjdk.java.net/jdk/pull/6894
Re: RFR: 8279043 Some Security Exception Messages Miss Spaces
On Mon, 20 Dec 2021 14:50:08 GMT, Daniel Jeliński wrote: > Trivial change. Issue reported by > [lgtm.com](https://lgtm.com/projects/g/openjdk/jdk/alerts/?mode=tree=java=1952840153). Change looks good. Please add a colon after the bug ID in the title. Otherwise Skara might not integrate it. - Marked as reviewed by weijun (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6894
Re: Incorrect exception strings
Great, thanks for confirming! Daniel wt., 21 gru 2021 o 17:22 Wei-Jun Wang napisał(a): > > Hi Daniel, > > Yes, http://openjdk.java.net/projects/#project-author page contains a > template. You mainly need to talk about your previous contributions to > OpenJDK. > > And yes, the Project Lead is Mark Reinhold, mark dot reinhold at oracle dot > com. > > Good luck! > > --Weijun > > > On Dec 21, 2021, at 5:29 AM, Daniel Jeliński wrote: > > > > Hi Weijun, > > > >> The "Account Help" link on https://id.openjdk.java.net/console/login leads > >> to https://wiki.openjdk.java.net/display/general/FAQ. It seems you need to > >> become an OpenJDK author first to get an account. > > > > Right. This page then leads to > > http://openjdk.java.net/projects/#project-author, which leads to > > http://openjdk.java.net/census, which suggests that I should PM Mark > > Reinhold at an address that I need to find somewhere else. Is that > > about right? > > Thanks, > > Daniel >
RFR: 8279066: Still see private key entries without certificates in a PKCS12 keystore
Before password-less PKCS12 keystores are supported, certificates in a PKCS12 file are always encrypted. Therefore if one loads the keystore with a null pass, it contains `PrivateKeyEntry`s without certificates. This has always been awkward (and most likely useless) so when JDK-8076190 introduced the password-less feature I also added a line to remove such an entry. https://github.com/openjdk/jdk/blob/a729a70c0119ed071ff490b0dfd4e3e2cb1a5ae4/src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java#L2272 Unfortunately, the line is not coded correctly, it should have been `remove(key)` but here it's `remove(value)`. This code change correctly removes the entry. That said, this behavior, although weird, has been there from the beginning since PKCS12 keystore was introduced. If you can find out a usage of a private key entry without any certificate and think it's worth kept that way, I can simply remove the `remove` call and leave the entry there. - Commit messages: - 8279066: Still see private key entries without certificates in a PKCS12 keystore Changes: https://git.openjdk.java.net/jdk/pull/6910/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6910=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279066 Stats: 94 lines in 3 files changed: 87 ins; 0 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/6910.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6910/head:pull/6910 PR: https://git.openjdk.java.net/jdk/pull/6910
Re: Incorrect exception strings
Hi Daniel, Yes, http://openjdk.java.net/projects/#project-author page contains a template. You mainly need to talk about your previous contributions to OpenJDK. And yes, the Project Lead is Mark Reinhold, mark dot reinhold at oracle dot com. Good luck! --Weijun > On Dec 21, 2021, at 5:29 AM, Daniel Jeliński wrote: > > Hi Weijun, > >> The "Account Help" link on https://id.openjdk.java.net/console/login leads >> to https://wiki.openjdk.java.net/display/general/FAQ. It seems you need to >> become an OpenJDK author first to get an account. > > Right. This page then leads to > http://openjdk.java.net/projects/#project-author, which leads to > http://openjdk.java.net/census, which suggests that I should PM Mark > Reinhold at an address that I need to find somewhere else. Is that > about right? > Thanks, > Daniel
Re: Incorrect exception strings
Hi Weijun, > The "Account Help" link on https://id.openjdk.java.net/console/login leads to > https://wiki.openjdk.java.net/display/general/FAQ. It seems you need to > become an OpenJDK author first to get an account. Right. This page then leads to http://openjdk.java.net/projects/#project-author, which leads to http://openjdk.java.net/census, which suggests that I should PM Mark Reinhold at an address that I need to find somewhere else. Is that about right? Thanks, Daniel
Re: Incorrect exception strings
Thank you Valerie. Without a JBS account I'd have to get a review on the bug report first, and given that this is a trivial fix I wanted to avoid going through that. Thanks again, Daniel pon., 20 gru 2021 o 22:36 Valerie Peng napisał(a): > > I filed https://bugs.openjdk.java.net/browse/JDK-8279043 for this and > added a noreg-trivial label. > > The usual process is to submit the PR with the associated bug id AFAIK. > > Thanks, > > Valerie > > On 12/20/2021 11:04 AM, Daniel Jeliński wrote: > > Hello, > > I fixed a few exception strings that had missing spaces between words; > > the PR can be found here https://github.com/openjdk/jdk/pull/6894. > > > > I could use some assistance with a JBS ticket and reviews here. > > > > On a side note, how can I get a JBS account? > > Regards, > > Daniel
RFR: 8279043 Some Security Exception Messages Miss Spaces
Trivial change. Issue reported by [lgtm.com](https://lgtm.com/projects/g/openjdk/jdk/alerts/?mode=tree=java=1952840153). - Commit messages: - Add missing spaces Changes: https://git.openjdk.java.net/jdk/pull/6894/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6894=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8279043 Stats: 21 lines in 16 files changed: 0 ins; 0 del; 21 mod Patch: https://git.openjdk.java.net/jdk/pull/6894.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6894/head:pull/6894 PR: https://git.openjdk.java.net/jdk/pull/6894
