<html>

  <head>

    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">

  </head>

  <body>

    <p>Hi Max,</p>

    <p>Not all of the comments are related to the changes in the webrev,

      just notice some PSS related inconsistency and thought I will ask:<br>

    </p>

    <p><AlgorithmId.java><br>

    </p>

    <p>- For RSASSA-PSS keys, existing code in

      getDefaultAlgorithmParameterSpec(...) (line 1121) decides the

      default based on key size. I think we should consider checking if

      the key contains PSS parameters. If present, use it as default.</p>

    <p>- In the checkKeyAndSigAlgMatch(...) method (line 1027), don't we

      need to add checking for RSASSA-PSS signature? RSASSA-PSS sig can

      take both RSA and RSASSA-PSS keys.</p>

    <p>- The getEncAlgFromSigAlg(...) method just returns the key

      algorithm as result when the specified signature algorithm does

      not contain "with". As RSASSA-PSS signature can use both

      RSASSA-PSS and RSA keys, should it still return key algorithm?</p>

    <p>- The makeSigAlg(...) method does not work for RSASSA-PSS. <br>

    </p>

    <p><X509CRLImpl.java></p>

    <p>- The sign() methods of X509CertImpl class do not generate

      default parameters automatically. Have you considered adding a

      sign() method to X509CRLImpl class which has extra signature

      parameter spec argument and move the default parameter call to the

      caller instead of inside X509CRLImpl? I think it's more suitable

      for the caller to generate the default unless there are many

      callers all need the same functionality.<br>

    </p>

    <p><span class="new">Thanks,</span></p>

    <p><span class="new">Valerie<br>

      </span></p>

    <div class="moz-cite-prefix">On 4/6/2020 8:11 PM, Weijun Wang wrote:<br>

    </div>

    <blockquote type="cite"

      cite="mid:610E58EF-5A67-421C-996B-BFDAB1CBDBDE@oracle.com">

      <pre class="moz-quote-pre" wrap="">Please review the fix at

   <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~weijun/8242184/webrev.00/">http://cr.openjdk.java.net/~weijun/8242184/webrev.00/</a>

The major change is inside X509CRLImpl.java to allow params setting and reading.

I also take this chance to:

1. Provide a default -sigalg for "keytool -genkeypair -keyalg rsassa-pss".

2. Revert a former change in X509CertImpl.java, which might be a safer call.

Thanks,

Max

</pre>

    </blockquote>

  </body>

</html>