Looks fine to me Max. regards, Sean.
On 23/02/2017 03:42, Weijun Wang wrote:
Updated again at http://cr.openjdk.java.net/~weijun/8171319/webrev.02/ Changes:1. Combined printWeakWarningsWithoutNewLine() and printWeakWarnings() to one method.2. New X509CRLImpl.toStringWithAlgName(String name). Nothing more. Thanks Max On 02/15/2017 11:04 PM, Seán Coffey wrote:Hi Weijun, That's looks good to me and will be a big help for keytool usability. some thoughts : Main.java : in your printCRL method, would you consider editing the X509CRLImpl class to print with a customized string ? It'll make the code more resilient to future changes in this area i.e. something like this in X509CRLImpl : public String toString() { printCRL(null); } public String printCRL(String custom) { // transfer the toString() code to here // and allow for 'custom' string to be injected if non-null .. } in Main.java, I'd suggest an instanceof check for X509CRLImpl before calling printCRL(..). Could X509CRL.getSigAlgName() then be used for passing into the withWeak method call ? === Also in Main.java, maybe you could reduce printWeakWarningsWithoutNewLine and printWeakWarnings() to one method - e.g. printWeakWarnings(boolean newline) === Regards, Sean. On 15/02/17 01:16, Weijun Wang wrote:Ping again. Also, must we resolve this one before ZBB? --Max On 02/09/2017 10:26 AM, Weijun Wang wrote:An update webrev is at http://cr.openjdk.java.net/~weijun/8171319/webrev.01/ The major change is that every risk warning has a owner now, i.e. instead of just saying "MD5withRSA is weak", it prints out whose algorithm is weak. For example: The generated CRL uses the MD5withRSA signature algorithm which is considered a security risk. Please take a look at the output of the newly added regression test at http://cr.openjdk.java.net/~weijun/8171319/webrev.01/examples.txt Thanks Max On 01/23/2017 06:02 PM, Weijun Wang wrote:Hi All Please take a review at http://cr.openjdk.java.net/~weijun/8171319/webrev.00/ Warnings are printed to System.err when weak algorithms/keysizes are detected during the execution, this includes input, output, and any certs used. The detection applies to many keytool functions: - generation of certificate, certificate request, CRL - reading (printing, listing, exporting) of above - importing of certificate or certificates replyThe behavior of most functions remains unchanged. The only exception is"keytool -importcert", where the user must reply to a prompt if weakalgorithms/keysizes are detected, unless -noprompt is specified on thecommand line. Warnings are either printed at the end, or before a prompt. If there are multiple weak points, multiple warnings will be printed. The detection is based on the security property jdk.certpath.disabledAlgorithms. For example: $ keytool -genkeypair -alias a -dname CN=a -keyalg RSA -sigalg MD5withRSA Warning: The MD5withRSA signature algorithm is considered a security risk. $ keytool -keystore ks -storepass changeit -keypass changeit -list Keystore type: JKS Keystore provider: SUN Your keystore contains 3 entries b, Jan 23, 2017, PrivateKeyEntry, Certificate fingerprint (SHA-256):D8:46:B7:0B:8B:97:C2:DE:A2:17:62:01:27:82:2B:CE:B1:9B:12:0B:24:D5:47:BF:BD:54:EE:8A:71:29:2B:CEa, Jan 23, 2017, PrivateKeyEntry, Certificate fingerprint (SHA-256):66:70:DF:11:14:A1:96:58:92:F5:6A:10:09:B1:2F:CC:1C:CC:2D:55:47:1D:EE:74:75:AA:26:63:E4:9D:EA:83Warning: <b>'s 512-bit RSA key is considered a security risk. <a>'s MD5withRSA signature algorithm is considered a security risk. $ keytool -importcert -alias a -file b+a.certs Warning: Reply #2 of 2's 512-bit RSA key is considered a security risk. Install reply anyway? [no]:no Certificate reply was not installed in keystore Thanks Max