contribute to the OpenJDK security group
Hello Everyone, I'd like to introduce myself. I'm Leo Grove, founder of SSL.com and also Java Certified Programmer ('98). Although I'm not so much into coding these days, I'm always looking for ways to contribute to internet security and the public WebPKI. We do have some very sharp java developers that specialize in PKI and certs, so if there is something you need a hand with (or a pair of eyeballs on), please let me know, thanks. -- Leo Grove President SSL.com w: https://www.ssl.com PKI • EV SSL/TLS • S/MIME • EV Code Signing
Re: [PATCH]: Support for brainpool curves from CurveDB in SunEC
Great! I took a look at the patch, and I have some comments, the first of which probably needs to be addressed before I can test the change: 1) Is this patch against the http://hg.openjdk.java.net/jdk/jdk repository? I suspect it isn't because some of the paths are different than what I expect. We have made a lot of changes to the repositories in the last few months. If this patch is against an older repo, please send a patch against http://hg.openjdk.java.net/jdk/jdk . 2) TestECDH.java: It's probably better to remove the provider name check on line 116 and test on any providers that support the curve. 3) oid.c: I think you can remove the comments that say "XXX bounds check" (e.g. line 362). If I am interpreting these comments correctly, they are saying that memcmp may read out of bounds, but you fixed that problem by using oideql. 4) Is there an existing test that exercises ECDSA with the new curves? Maybe there is something in the PKCS11 tests that does this already, but I didn't find it. I think we should have an ECDSA test to make sure that we didn't forget anything. ECDSA test vectors probably aren't necessary---a simple test that signs and verifies using the new curves should be sufficient. On 1/12/2018 9:12 AM, Tobias Wagner wrote: Hi, here is the next patch for brainpool curve support in SunEC. Differences from the first patch: * Brainpool curves with less than 256 bits are removed. Subsequently, the curve oid check is made more robust to avoid null pointer caused Segmentation Faults in memcmp calls. * Bug JDK-8189594 is fixed. * Known answer tests for each new curve are added to sun.security.pkcs11.ec.TestECDH. The tests are only executed, if the tested provider's name is "SunEC" and the tested provider claims to support the respective curve. For SunEC, these tests are executed during sun.security.ec.TestEC. I decided to add these test vectors to TestECDH to avoid code duplications, as TestECDH is describes exactly the test for that kind of test vectors. The superclass to TestECDH, TestPKCS11, is also adapted to provide a method to check, whether one particular curve is supported. While the test vectors for the 256, 384 and 512 bit curve are taken from [1], the test vector for brainpoolP320r1 comes from [2]. The latter one is a draft version of RFC 6954. Regards, Tobias [1] https://tools.ietf.org/html/rfc7027#appendix-A [2] https://tools.ietf.org/html/draft-merkle-ikev2-ke-brainpool-00#appendix-A.5
Re: RFR 8014628: Support AES Encryption with HMAC-SHA2 for Kerberos 5
On 1/14/18 9:12 PM, Weijun Wang wrote: Hi Sean Do you have other comments on the webrev [1]? No. I've also updated the CSR [2]. Ok, thanks. --Sean Thanks Max [1] http://cr.openjdk.java.net/~weijun/8014628/webrev.00/ [2] https://bugs.openjdk.java.net/browse/JDK-8193851 On Jan 13, 2018, at 3:52 AM, Sean Mullanwrote: On 1/9/18 8:40 PM, Weijun Wang wrote: The code can also throw GeneralSecurityException but those are also always suppressed because of the catch block. Is that the right behavior? Not a right behavior but should be harmless here. In my understanding, in the case of PBE, as long the passphrase, salt, iteration count etc are legal, there will be no further problem in generating a key, choosing a cipher, and do the encryption work, unless there is a programming error. I think the original designer (of other etypes) meant to let stringToKey(char,String,byte[]) returning a null when there is an error, and all callers of this method will deal with null instead of an exception. If not programmed carefully, this might turn a GeneralSecurityException to a NullPointerException. Ok, I think this is bad practice, but since it has worked that way since the beginning, I'm ok with leaving it alone. --Sean
Re: RFR 8195119: Fine-tune output text in keytool
Looks good. --Sean On 1/15/18 8:57 PM, Weijun Wang wrote: The translation team suggested a small text change: diff --git a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java --- a/src/java.base/share/classes/sun/security/tools/keytool/Resources.java +++ b/src/java.base/share/classes/sun/security/tools/keytool/Resources.java @@ -462,7 +462,7 @@ {"with.weak", "%s (weak)"}, {"key.bit", "%1$d-bit %2$s key"}, {"key.bit.weak", "%1$d-bit %2$s key (weak)"}, -{"unknown.size.1", "unknown size %s key"}, +{"unknown.size.1", "%s key of unknown size"}, {".PATTERN.printX509Cert.with.weak", "Owner: {0}\nIssuer: {1}\nSerial number: {2}\nValid from: {3} until: {4}\nCertificate fingerprints:\n\t SHA1: {5}\n\t SHA256: {6}\nSignature algorithm name: {7}\nSubject Public Key Algorithm: {8}\nVersion: {9}"}, {"PKCS.10.with.weak", Please take a review. Noreg-trivial. Thanks Max