Skip to content

Commit

Permalink
8333583: Crypto-XDH.generateSecret regression after JDK-8329538
Browse files Browse the repository at this point in the history
Reviewed-by: thartmann, sviswanathan
Backport-of: f101e15
  • Loading branch information
vpaprotsk authored and Sandhya Viswanathan committed Jun 26, 2024
1 parent 2086b0f commit b5fbdb2
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 90 deletions.
10 changes: 2 additions & 8 deletions make/jdk/src/classes/build/tools/intpoly/FieldGen.java
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ private String generate(FieldParams params) throws IOException {
result.appendLine("}");

result.appendLine("@Override");
result.appendLine("protected int mult(long[] a, long[] b, long[] r) {");
result.appendLine("protected void mult(long[] a, long[] b, long[] r) {");
result.incrIndent();
for (int i = 0; i < 2 * params.getNumLimbs() - 1; i++) {
result.appendIndent();
Expand All @@ -804,9 +804,6 @@ private String generate(FieldParams params) throws IOException {
}
}
result.append(");\n");
result.appendIndent();
result.append("return 0;");
result.appendLine();
result.decrIndent();
result.appendLine("}");

Expand Down Expand Up @@ -836,7 +833,7 @@ private String generate(FieldParams params) throws IOException {
// }
// }
result.appendLine("@Override");
result.appendLine("protected int square(long[] a, long[] r) {");
result.appendLine("protected void square(long[] a, long[] r) {");
result.incrIndent();
for (int i = 0; i < 2 * params.getNumLimbs() - 1; i++) {
result.appendIndent();
Expand Down Expand Up @@ -877,9 +874,6 @@ private String generate(FieldParams params) throws IOException {
}
}
result.append(");\n");
result.appendIndent();
result.append("return 0;");
result.appendLine();
result.decrIndent();
result.appendLine("}");

Expand Down
1 change: 0 additions & 1 deletion src/hotspot/cpu/x86/stubGenerator_x86_64_poly_mont.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,6 @@ address StubGenerator::generate_intpoly_montgomeryMult_P256() {
const Register tmp = r9;

montgomeryMultiply(aLimbs, bLimbs, rLimbs, tmp, _masm);
__ mov64(rax, 0x1); // Return 1 (Fig. 5, Step 6 [1] skipped in montgomeryMultiply)

__ leave();
__ ret(0);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/classfile/vmIntrinsics.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -529,8 +529,8 @@ class methodHandle;
/* support for sun.security.util.math.intpoly.MontgomeryIntegerPolynomialP256 */ \
do_class(sun_security_util_math_intpoly_MontgomeryIntegerPolynomialP256, "sun/security/util/math/intpoly/MontgomeryIntegerPolynomialP256") \
do_intrinsic(_intpoly_montgomeryMult_P256, sun_security_util_math_intpoly_MontgomeryIntegerPolynomialP256, intPolyMult_name, intPolyMult_signature, F_R) \
do_name(intPolyMult_name, "mult") \
do_signature(intPolyMult_signature, "([J[J[J)I") \
do_name(intPolyMult_name, "multImpl") \
do_signature(intPolyMult_signature, "([J[J[J)V") \
\
do_class(sun_security_util_math_intpoly_IntegerPolynomial, "sun/security/util/math/intpoly/IntegerPolynomial") \
do_intrinsic(_intpoly_assign, sun_security_util_math_intpoly_IntegerPolynomial, intPolyAssign_name, intPolyAssign_signature, F_S) \
Expand Down
2 changes: 0 additions & 2 deletions src/hotspot/share/opto/library_call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7554,8 +7554,6 @@ bool LibraryCallKit::inline_intpoly_montgomeryMult_P256() {
OptoRuntime::intpoly_montgomeryMult_P256_Type(),
stubAddr, stubName, TypePtr::BOTTOM,
a_start, b_start, r_start);
Node* result = _gvn.transform(new ProjNode(call, TypeFunc::Parms));
set_result(result);
return true;
}

Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/opto/runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1414,8 +1414,8 @@ const TypeFunc* OptoRuntime::intpoly_montgomeryMult_P256_Type() {

// result type needed
fields = TypeTuple::fields(1);
fields[TypeFunc::Parms + 0] = TypeInt::INT; // carry bits in output
const TypeTuple* range = TypeTuple::make(TypeFunc::Parms+1, fields);
fields[TypeFunc::Parms + 0] = nullptr; // void
const TypeTuple* range = TypeTuple::make(TypeFunc::Parms, fields);
return TypeFunc::make(domain, range);
}

Expand All @@ -1434,7 +1434,7 @@ const TypeFunc* OptoRuntime::intpoly_assign_Type() {

// result type needed
fields = TypeTuple::fields(1);
fields[TypeFunc::Parms + 0] = NULL; // void
fields[TypeFunc::Parms + 0] = nullptr; // void
const TypeTuple* range = TypeTuple::make(TypeFunc::Parms, fields);
return TypeFunc::make(domain, range);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,11 @@ public abstract sealed class IntegerPolynomial implements IntegerFieldModuloP
* store the result in an IntegerPolynomial representation in a. Requires
* that a.length == numLimbs.
*/
protected int multByInt(long[] a, long b) {
protected void multByInt(long[] a, long b) {
for (int i = 0; i < a.length; i++) {
a[i] *= b;
}
reduce(a);
return 0;
}

/**
Expand All @@ -104,15 +103,15 @@ protected int multByInt(long[] a, long b) {
* a.length == b.length == r.length == numLimbs. It is allowed for a and r
* to be the same array.
*/
protected abstract int mult(long[] a, long[] b, long[] r);
protected abstract void mult(long[] a, long[] b, long[] r);

/**
* Multiply an IntegerPolynomial representation (a) with itself and store
* the result in an IntegerPolynomialRepresentation (r). Requires that
* a.length == r.length == numLimbs. It is allowed for a and r
* to be the same array.
*/
protected abstract int square(long[] a, long[] r);
protected abstract void square(long[] a, long[] r);

IntegerPolynomial(int bitsPerLimb,
int numLimbs,
Expand Down Expand Up @@ -622,8 +621,8 @@ public ImmutableElement multiply(IntegerModuloP genB) {
}

long[] newLimbs = new long[limbs.length];
int numAdds = mult(limbs, b.limbs, newLimbs);
return new ImmutableElement(newLimbs, numAdds);
mult(limbs, b.limbs, newLimbs);
return new ImmutableElement(newLimbs, 0);
}

@Override
Expand All @@ -635,8 +634,8 @@ public ImmutableElement square() {
}

long[] newLimbs = new long[limbs.length];
int numAdds = IntegerPolynomial.this.square(limbs, newLimbs);
return new ImmutableElement(newLimbs, numAdds);
IntegerPolynomial.this.square(limbs, newLimbs);
return new ImmutableElement(newLimbs, 0);
}

public void addModPowerTwo(IntegerModuloP arg, byte[] result) {
Expand Down Expand Up @@ -751,7 +750,8 @@ public MutableElement setProduct(IntegerModuloP genB) {
b.numAdds = 0;
}

numAdds = mult(limbs, b.limbs, limbs);
mult(limbs, b.limbs, limbs);
numAdds = 0;
return this;
}

Expand All @@ -764,7 +764,8 @@ public MutableElement setProduct(SmallValue v) {
}

int value = ((Limb)v).value;
numAdds += multByInt(limbs, value);
multByInt(limbs, value);
numAdds = 0;
return this;
}

Expand Down Expand Up @@ -824,7 +825,8 @@ public MutableElement setSquare() {
numAdds = 0;
}

numAdds = IntegerPolynomial.this.square(limbs, limbs);
IntegerPolynomial.this.square(limbs, limbs);
numAdds = 0;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private IntegerPolynomial1305() {
super(BITS_PER_LIMB, NUM_LIMBS, 1, MODULUS);
}

protected int mult(long[] a, long[] b, long[] r) {
protected void mult(long[] a, long[] b, long[] r) {

// Use grade-school multiplication into primitives to avoid the
// temporary array allocation. This is equivalent to the following
Expand All @@ -73,7 +73,6 @@ protected int mult(long[] a, long[] b, long[] r) {
long c8 = (a[4] * b[4]);

carryReduce(r, c0, c1, c2, c3, c4, c5, c6, c7, c8);
return 0;
}

private void carryReduce(long[] r, long c0, long c1, long c2, long c3,
Expand All @@ -100,7 +99,7 @@ private void carryReduce(long[] r, long c0, long c1, long c2, long c3,
}

@Override
protected int square(long[] a, long[] r) {
protected void square(long[] a, long[] r) {
// Use grade-school multiplication with a simple squaring optimization.
// Multiply into primitives to avoid the temporary array allocation.
// This is equivalent to the following code:
Expand All @@ -123,7 +122,6 @@ protected int square(long[] a, long[] r) {
long c8 = (a[4] * a[4]);

carryReduce(r, c0, c1, c2, c3, c4, c5, c6, c7, c8);
return 0;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,11 @@ private void multOnly(long[] a, long[] b, long[] c) {
}

@Override
protected int mult(long[] a, long[] b, long[] r) {
protected void mult(long[] a, long[] b, long[] r) {

long[] c = new long[2 * numLimbs];
multOnly(a, b, c);
carryReduce(c, r);
return 0;
}

private void modReduceInBits(long[] limbs, int index, int bits, long x) {
Expand Down Expand Up @@ -189,7 +188,7 @@ protected void reduce(long[] a) {
}

@Override
protected int square(long[] a, long[] r) {
protected void square(long[] a, long[] r) {

long[] c = new long[2 * numLimbs];
for (int i = 0; i < numLimbs; i++) {
Expand All @@ -200,7 +199,6 @@ protected int square(long[] a, long[] r) {
}

carryReduce(c, r);
return 0;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import sun.security.util.math.IntegerFieldModuloP;
import java.math.BigInteger;
import jdk.internal.vm.annotation.IntrinsicCandidate;
import jdk.internal.vm.annotation.ForceInline;

// Reference:
// - [1] Shay Gueron and Vlad Krasnov "Fast Prime Field Elliptic Curve
Expand Down Expand Up @@ -103,8 +104,8 @@ public ImmutableElement getElement(BigInteger v) {
setLimbsValuePositive(v, vLimbs);

// Convert to Montgomery domain
int numAdds = mult(vLimbs, h, montLimbs);
return new ImmutableElement(montLimbs, numAdds);
mult(vLimbs, h, montLimbs);
return new ImmutableElement(montLimbs, 0);
}

@Override
Expand All @@ -114,24 +115,6 @@ public SmallValue getSmallValue(int value) {
return super.getSmallValue(value);
}

/*
* This function is used by IntegerPolynomial.setProduct(SmallValue v) to
* multiply by a small constant (i.e. (int) 1,2,3,4). Instead of doing a
* montgomery conversion followed by a montgomery multiplication, just use
* the spare top (64-BITS_PER_LIMB) bits to multiply by a constant. (See [1]
* Section 4 )
*
* Will return an unreduced value
*/
@Override
protected int multByInt(long[] a, long b) {
assert (b < (1 << BITS_PER_LIMB));
for (int i = 0; i < a.length; i++) {
a[i] *= b;
}
return (int) (b - 1);
}

@Override
public ImmutableIntegerModuloP fromMontgomery(ImmutableIntegerModuloP n) {
assert n.getField() == MontgomeryIntegerPolynomialP256.ONE;
Expand Down Expand Up @@ -163,19 +146,27 @@ private void halfLimbs(long[] a, long[] r) {
}

@Override
protected int square(long[] a, long[] r) {
return mult(a, a, r);
protected void square(long[] a, long[] r) {
mult(a, a, r);
}


/**
* Unrolled Word-by-Word Montgomery Multiplication r = a * b * 2^-260 (mod P)
*
* See [1] Figure 5. "Algorithm 2: Word-by-Word Montgomery Multiplication
* for a Montgomery Friendly modulus p". Note: Step 6. Skipped; Instead use
* numAdds to reuse existing overflow logic.
*/
@Override
protected void mult(long[] a, long[] b, long[] r) {
multImpl(a, b, r);
reducePositive(r);
}

@ForceInline
@IntrinsicCandidate
protected int mult(long[] a, long[] b, long[] r) {
private void multImpl(long[] a, long[] b, long[] r) {
long aa0 = a[0];
long aa1 = a[1];
long aa2 = a[2];
Expand Down Expand Up @@ -408,36 +399,16 @@ protected int mult(long[] a, long[] b, long[] r) {
d4 += n4 & LIMB_MASK;

c5 += d1 + dd0 + (d0 >>> BITS_PER_LIMB);
c6 += d2 + dd1 + (c5 >>> BITS_PER_LIMB);
c7 += d3 + dd2 + (c6 >>> BITS_PER_LIMB);
c8 += d4 + dd3 + (c7 >>> BITS_PER_LIMB);
c9 = dd4 + (c8 >>> BITS_PER_LIMB);

c5 &= LIMB_MASK;
c6 &= LIMB_MASK;
c7 &= LIMB_MASK;
c8 &= LIMB_MASK;

// At this point, the result could overflow by one modulus.
c0 = c5 - modulus[0];
c1 = c6 - modulus[1] + (c0 >> BITS_PER_LIMB);
c0 &= LIMB_MASK;
c2 = c7 - modulus[2] + (c1 >> BITS_PER_LIMB);
c1 &= LIMB_MASK;
c3 = c8 - modulus[3] + (c2 >> BITS_PER_LIMB);
c2 &= LIMB_MASK;
c4 = c9 - modulus[4] + (c3 >> BITS_PER_LIMB);
c3 &= LIMB_MASK;

long mask = c4 >> BITS_PER_LIMB; // Signed shift!

r[0] = ((c5 & mask) | (c0 & ~mask));
r[1] = ((c6 & mask) | (c1 & ~mask));
r[2] = ((c7 & mask) | (c2 & ~mask));
r[3] = ((c8 & mask) | (c3 & ~mask));
r[4] = ((c9 & mask) | (c4 & ~mask));

return 0;
c6 += d2 + dd1;
c7 += d3 + dd2;
c8 += d4 + dd3;
c9 = dd4;

r[0] = c5;
r[1] = c6;
r[2] = c7;
r[3] = c8;
r[4] = c9;
}

@Override
Expand Down Expand Up @@ -516,8 +487,8 @@ public ImmutableElement getElement(byte[] v, int offset, int length,
super.encode(v, offset, length, highByte, vLimbs);

// Convert to Montgomery domain
int numAdds = mult(vLimbs, h, montLimbs);
return new ImmutableElement(montLimbs, numAdds);
mult(vLimbs, h, montLimbs);
return new ImmutableElement(montLimbs, 0);
}

/*
Expand Down Expand Up @@ -556,4 +527,27 @@ protected void reduceIn(long[] limbs, long v, int i) {
limbs[i - 5] += (v << 4) & LIMB_MASK;
limbs[i - 4] += v >> 48;
}

// Used when limbs a could overflow by one modulus.
@ForceInline
protected void reducePositive(long[] a) {
long aa0 = a[0];
long aa1 = a[1] + (aa0>>BITS_PER_LIMB);
long aa2 = a[2] + (aa1>>BITS_PER_LIMB);
long aa3 = a[3] + (aa2>>BITS_PER_LIMB);
long aa4 = a[4] + (aa3>>BITS_PER_LIMB);

long c0 = a[0] - modulus[0];
long c1 = a[1] - modulus[1] + (c0 >> BITS_PER_LIMB);
long c2 = a[2] - modulus[2] + (c1 >> BITS_PER_LIMB);
long c3 = a[3] - modulus[3] + (c2 >> BITS_PER_LIMB);
long c4 = a[4] - modulus[4] + (c3 >> BITS_PER_LIMB);
long mask = c4 >> BITS_PER_LIMB; // Signed shift!

a[0] = ((aa0 & mask) | (c0 & ~mask)) & LIMB_MASK;
a[1] = ((aa1 & mask) | (c1 & ~mask)) & LIMB_MASK;
a[2] = ((aa2 & mask) | (c2 & ~mask)) & LIMB_MASK;
a[3] = ((aa3 & mask) | (c3 & ~mask)) & LIMB_MASK;
a[4] = ((aa4 & mask) | (c4 & ~mask));
}
}

3 comments on commit b5fbdb2

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JesperIRL
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/tag jdk-23+29

@openjdk
Copy link

@openjdk openjdk bot commented on b5fbdb2 Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JesperIRL The tag jdk-23+29 was successfully created.

Please sign in to comment.