Re: [9] RFR: 8007706: X.509 cert extension SAN should support _ in dNSName
On 08/05/2014 07:52 AM, Jason Uh wrote:
Hi Florian,
I've reviewed the RFC again and think there might be some
misinterpretation. The only part I see about underscores reads:
Implementers should note that the at sign ('@') and underscore ('_')
characters are not supported by the ASN.1 type PrintableString.
These characters often appear in Internet addresses. Such addresses
MUST be encoded using an ASN.1 type that supports them. They are
usually encoded as IA5String in either the emailAddress attribute
within a distinguished name or the rfc822Name field of GeneralName.
Conforming implementations MUST NOT encode strings that include
either the at sign or underscore character as PrintableString.
RFC 5280 doesn't allow underscores for *PrintableString*, but DNSName is
an *IA5String*, which does support them.
By this argument, the patch is still not correct because it leaves in
additional checking incompatible with IA5String. (It is also not clear
to me what exactly is permissible in IA5Strings and how codepoints are
supposedly mapped to their Unicode counterparts if a national variant of
T.50 is used, but that's a different issue.) Relaxing all restrictions
would match what other software does.
My claim that '_' is not allowed in dNSName is based on these two sentences:
When the subjectAltName extension contains a domain name system
label, the domain name MUST be stored in the dNSName (an IA5String).
The name MUST be in the preferred name syntax, as specified by
Section 3.5 of [RFC1034] and as modified by Section 2.1 of
[RFC1123].
Section 3.5 of RFC 1034 and section 2.1 of RFC 1123 deal with host name
syntax, and the grammar in RFC 1034 (and RFC 952, which is referenced in
RFC 1123) does not permit underscores.
--
Florian Weimer / Red Hat Product Security
Request for review : backport of bug# 8031046 to 7u-dev
HI, Request review for the direct backport of bug# 8031046 to 7u-dev. http://cr.openjdk.java.net/~mbankal/8031046/webrev.00/ JDK9 Changeset : http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ab914c760352 JDK8 Changeset : http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/b32e9aba4888 Regression test is not added since it requires special setup, SQE bug filed for the same : https://bugs.openjdk.java.net/browse/INTJDK-7604925 Thanks. rgds mala
Re: Review request for CR 8044193 Need to add known answer tests for AES cipher
The tests look fine. However, can you please update the test policy files with fine-grained permissions for SunJCE provider? Please refer to the current jre/lib/security/java.policy. Thanks, Valerie On 7/28/2014 9:36 PM, zaiyao liu wrote: Hello, Please help to review the tests for AES cipher. This tests test AES ciphers with different modes and padding schemes when provider change,are part of tests for bug 8044193(Open part) Bug - https://bugs.openjdk.java.net/browse/JDK-8044193 webrev- http://cr.openjdk.java.net/~rhalade/8044193/webrev.00/ Thanks Kevin Liu
Re: apple.security.KeychainStore does not load private key (when called from javaws)
On 08/04/2014 11:10 AM, Florian Bruckner (3kraft) wrote:
Hey guys,
any feedback/comments on this?
This seems like a reasonable change to me. In order to proceed with
accepting your patch, you will first need to sign an OCA. See step 0 of
http://openjdk.java.net/contribute/
Thanks,
Sean
Just to summarize again:
KeychainStore does not load private keys when not called with a
passphrase. This is the case in various deployment scenarios (like
javaws), as a consequence identity certificates stored in Apple Keychain
are not available (i.e. being offered for selection like on Windows).
The reason for this is the implementation of KeychainStore, which uses
this API to retrieve a PKCS#12 container from apple keychain:
https://developer.apple.com/library/mac/documentation/security/Reference/keychainservices/Reference/reference.html#jumpTo_63
The API documentation says the PKCS#12 keystore being generated with
this call requires a password. This is either supplied by the caller or
the user is prompted (if a flag is set, which is not the case).
KeychainStore then goes on to extract the private key from the returned
PKCS#12 store and decrypts it with the password.
Therefore, the password passed into engineGetKey is actually used to
encrypt and decrypt only in the scope of this method. Therefore, the
approach is to create a dummy password for this use case.
Please consider these patches to fix this issue:
For jdk7u-dev:
diff -r 35aabd00a534 src/macosx/classes/apple/security/KeychainStore.java
--- a/src/macosx/classes/apple/security/KeychainStore.javaTue Jul 15
02:26:55 2014 +0400
+++ b/src/macosx/classes/apple/security/KeychainStore.javaTue Jul 15
10:52:44 2014 +0200
@@ -134,7 +134,7 @@
* password to recover it.
*
* @param alias the alias name
- * @param password the password for recovering the key
+ * @param password the password for recovering the key. This
password is not used to access the private key in KeyChain, it is used
internally only.
*
* @return the requested key, or null if the given alias does not
exist
* or does not identify a ikey entry/i.
@@ -148,6 +148,16 @@
throws NoSuchAlgorithmException, UnrecoverableKeyException
{
permissionCheck();
+// An empty password is rejected by MacOS API, no private key data
+// is exported. If no password is passed (as is the case when
+// this implementation is used as browser keystore in various
+// deployment scenarios like webstart, JFX and applets), create
+// a dummy password to MacOS API is happy.
+if (password == null || password.length ==0) {
+// Must not be a char array with only a 0, as this is an empty
+// string. Therefore use a single character.
+password = new char[]{'A'}
+}
Object entry = entries.get(alias.toLowerCase());
For jdk8u-dev:
diff -r baec3649f6c0 src/macosx/classes/apple/security/KeychainStore.java
--- a/src/macosx/classes/apple/security/KeychainStore.javaTue Jul 15
02:00:52 2014 +0400
+++ b/src/macosx/classes/apple/security/KeychainStore.javaTue Jul 15
10:54:45 2014 +0200
@@ -140,7 +140,7 @@
* password to recover it.
*
* @param alias the alias name
- * @param password the password for recovering the key
+ * @param password the password for recovering the key. This
password is not used to access the private key in KeyChain, it is used
internally only.
*
* @return the requested key, or null if the given alias does not
exist
* or does not identify a ikey entry/i.
@@ -154,7 +154,16 @@
throws NoSuchAlgorithmException, UnrecoverableKeyException
{
permissionCheck();
-
+// An empty password is rejected by MacOS API, no private key data
+// is exported. If no password is passed (as is the case when
+// this implementation is used as browser keystore in various
+// deployment scenarios like webstart, JFX and applets), create
+// a dummy password to MacOS API is happy.
+if (password == null || password.length ==0) {
+// Must not be a char array with only a 0, as this is an empty
+// string. Therefore use a single character.
+password = new char[]{'A'}
+}
Object entry = entries.get(alias.toLowerCase());
if (entry == null || !(entry instanceof KeyEntry)) {
With best regards,
Florian
Re: RFR 6997010: Consolidate java.security files into one file with modifications
On 07/28/2014 09:53 AM, Wang Weijun wrote: Yes, you are right. Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.02. GendataJavaSecurity.gmk and MakeJavaSecurity.java updated. There's an unnecessary indent at line 1 of GendataJavaSecurity.gmk In CheckSecurityProvider.java, ucrypto is in the closed sources, so Security.getProviders() will not return it if you are testing an OpenJDK build. You need to adjust the test to not include this provider if testing an OpenJDK build. --Sean Thanks Max On Jul 28, 2014, at 19:43, Erik Joelsson erik.joels...@oracle.com wrote: Hello Max, Shouldn't the rule for $(GENDATA_JAVA_SECURITY) depend on $(RESTRICTED_PKGS_SRC) so that updates to the pkgs file triggers a rebuild? For that to work, the variable $(RESTRICTED_PKGS_SRC) needs to be empty for the OPENJDK case rather than have a dummy name and MakeJavaSecurity.java needs to handle missing the last argument. /Erik On 2014-07-28 03:44, Wang Weijun wrote: Webrev updated at http://cr.openjdk.java.net/~weijun/6997010/webrev.01/ New test CheckSecurityProvider.java, and updates to MakeJavaSecurity.addPackages(). Thanks Max On Jul 25, 2014, at 22:44, Wang Weijun weijun.w...@oracle.com wrote: On Jul 25, 2014, at 22:30, Sean Mullan sean.mul...@oracle.com wrote: http://cr.openjdk.java.net/~weijun/6997010/webrev.00/ 4. *IMPORTANT*: In order to easily maintain platform-related entries, every line (including the last line) in package.access and package.definition MUST end with ',\' now. A blank line MUST exist after the last line. This avoid ugly lines like #ifndef windows entry1. #endif #ifdef windows entry1.,\ entry2 #endif What happens if someone (inevitably) adds a new package to the list and forgets to do either of these? Does it result in a build failure? No build failure, but test/java/security/SecurityManager/CheckPackageAccess.java will fail. I can add check in the build tool. Otherwise looks good, although I think it would be useful to write an additional test to make sure the correct providers are installed and ordered correctly on the different platforms, something similar to the java/lang/SecurityManager/CheckPackageAccess.java test but specific to providers. OK. Thanks Max
Re: RFR 8043836: Need new tests for AES cipher
Looks fine to me. Thanks, Valerie On 7/28/2014 1:57 AM, FELIX YANG wrote: May I request you to review these 6 new tests to be added for AES cipher. New tests are added to address following: - Test AES for different modes and padding schemes - Test AES encryption with no padding - same buffer can be used for encrypt and decrypt with AES JDK Issue: https://bugs.openjdk.java.net/browse/JDK-8043836 Webrev: http://cr.openjdk.java.net/~rhalade/8043836/webrev.00/ Thanks,
Re: [9] RFR: 8007706: X.509 cert extension SAN should support _ in dNSName
Thanks, Florian. I will withdraw my review request and close this issue.
I'll file a separate bug to allow the first character to be a digit, as
RFC 1123 relaxed that restriction.
Thanks,
Jason
On 08/04/2014 11:58 PM, Florian Weimer wrote:
On 08/05/2014 07:52 AM, Jason Uh wrote:
Hi Florian,
I've reviewed the RFC again and think there might be some
misinterpretation. The only part I see about underscores reads:
Implementers should note that the at sign ('@') and underscore ('_')
characters are not supported by the ASN.1 type PrintableString.
These characters often appear in Internet addresses. Such addresses
MUST be encoded using an ASN.1 type that supports them. They are
usually encoded as IA5String in either the emailAddress attribute
within a distinguished name or the rfc822Name field of GeneralName.
Conforming implementations MUST NOT encode strings that include
either the at sign or underscore character as PrintableString.
RFC 5280 doesn't allow underscores for *PrintableString*, but DNSName is
an *IA5String*, which does support them.
By this argument, the patch is still not correct because it leaves in
additional checking incompatible with IA5String. (It is also not clear
to me what exactly is permissible in IA5Strings and how codepoints are
supposedly mapped to their Unicode counterparts if a national variant of
T.50 is used, but that's a different issue.) Relaxing all restrictions
would match what other software does.
My claim that '_' is not allowed in dNSName is based on these two
sentences:
When the subjectAltName extension contains a domain name system
label, the domain name MUST be stored in the dNSName (an IA5String).
The name MUST be in the preferred name syntax, as specified by
Section 3.5 of [RFC1034] and as modified by Section 2.1 of
[RFC1123].
Section 3.5 of RFC 1034 and section 2.1 of RFC 1123 deal with host name
syntax, and the grammar in RFC 1034 (and RFC 952, which is referenced in
RFC 1123) does not permit underscores.
Re: Request for review : backport of bug# 8031046 to 7u-dev
Hi Mala Code change looks fine. When you say Direct backport, is it equivalent to applying the same patch with no conflict? :-) Thanks Max On 8/5/2014 17:32, mala bankal wrote: HI, Request review for the direct backport of bug# 8031046 to 7u-dev. http://cr.openjdk.java.net/~mbankal/8031046/webrev.00/ JDK9 Changeset : http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ab914c760352 JDK8 Changeset : http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/b32e9aba4888 Regression test is not added since it requires special setup, SQE bug filed for the same : https://bugs.openjdk.java.net/browse/INTJDK-7604925 Thanks. rgds mala
