RFR 8208648: ECC Field Arithmetic Enhancements

[Jamil Nimeh]

Hi Adam, comments/questions below (mostly simple stuff, nothing major):

  * IntegerPolynomial.java
      o The comment block for multByInt should reflect your changes you
        made, namely the removal of "r" from the signature.
      o 88: In the case of multiplying two very large long values would
        we ever see those cause an overflow.  If so, is it OK to have
        them overflow before the reduce operation?
      o 420-425: Looks like this was a copy of the comment block from
        conditionalSwap().  Maybe needs to be tailored to what the
        conditionalAssign method does?
      o IntegerPolynomialP256.java, IntegerPolynomialP384.java,
        IntegerPolynomialP521.java
          + In the carryReduce0 and carryReduce methods, you have many
            uses of an integer literal (33554432 for P256, 134217728 for
            P384/521).  Should these be made as private static final
            long values?
      o P256OrderField.java, P384OrderField.java, P521OrderField.java
          + Similar question about using a static final long vs.
            repeated int literal values in the carryReduce* methods.
      o With respect to the last two main bullet items: Would conversion
        to a static final value be difficult due to the fact that they
        are generated from Fieldgen.jsh?

--Jamil

On 11/30/2018 11:59 AM, Adam Petcher wrote:
> JBS: https://bugs.openjdk.java.net/browse/JDK-8208648
> webrev: http://cr.openjdk.java.net/~apetcher/8208648/webrev.00/
>
> This changeset includes the enhancements to the finite field 
> arithmetic library that are necessary for the new implementation of 
> ECDSA and ECDH[1]. The actual ECDH and ECDSA changes will be reviewed 
> separately. Please see the JBS tickets for more information about the 
> changes, and here are some additional notes:
>
> 1) This change includes a code generator that produces finite field 
> implementations. The generated code is included in the review, and it 
> will be checked in to the repository with a comment indicating that it 
> is generated and should not be modified directly. Another option is to 
> put the code generator into the build process so the generated code 
> does not need to be checked in, but it's not clear whether this is a 
> better option.
> 2) Reviewing every line of the generated code is probably not 
> necessary, and it is probably better to focus on the code generator 
> itself (Fieldgen.jsh). Of course, it is probably a good idea to review 
> the generated code for its structure and for opportunities for 
> improvement.
>
>
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8208698
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20181206/264e4852/attachment.htm>