Skip to content

Commit bd052e9

Browse files
martinuyRealCLanger
authored andcommitted
8337692: Better TLS connection support
Reviewed-by: abakhtin Backport-of: f06ecf8072b39ffb9eedfc629f181bd805115e0e
1 parent 25d9c74 commit bd052e9

File tree

3 files changed

+147
-56
lines changed

3 files changed

+147
-56
lines changed

src/java.base/share/classes/com/sun/crypto/provider/RSACipher.java

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ private byte[] doFinal() throws BadPaddingException,
385385
byte[] decryptBuffer = RSACore.convert(buffer, 0, bufOfs);
386386
paddingCopy = RSACore.rsa(decryptBuffer, privateKey, false);
387387
result = padding.unpad(paddingCopy);
388-
if (result == null && !forTlsPremasterSecret) {
388+
if (!forTlsPremasterSecret && result == null) {
389389
throw new BadPaddingException
390390
("Padding error in decryption");
391391
}
@@ -405,6 +405,34 @@ private byte[] doFinal() throws BadPaddingException,
405405
}
406406
}
407407

408+
// TLS master secret decode version of the doFinal() method.
409+
private byte[] doFinalForTls(int clientVersion, int serverVersion)
410+
throws BadPaddingException, IllegalBlockSizeException {
411+
if (bufOfs > buffer.length) {
412+
throw new IllegalBlockSizeException("Data must not be longer "
413+
+ "than " + buffer.length + " bytes");
414+
}
415+
byte[] paddingCopy = null;
416+
byte[] result = null;
417+
try {
418+
byte[] decryptBuffer = RSACore.convert(buffer, 0, bufOfs);
419+
420+
paddingCopy = RSACore.rsa(decryptBuffer, privateKey, false);
421+
result = padding.unpadForTls(paddingCopy, clientVersion,
422+
serverVersion);
423+
424+
return result;
425+
} finally {
426+
Arrays.fill(buffer, 0, bufOfs, (byte)0);
427+
bufOfs = 0;
428+
if (paddingCopy != null
429+
&& paddingCopy != buffer // already cleaned
430+
&& paddingCopy != result) { // DO NOT CLEAN, THIS IS RESULT
431+
Arrays.fill(paddingCopy, (byte)0);
432+
}
433+
}
434+
}
435+
408436
// see JCE spec
409437
protected byte[] engineUpdate(byte[] in, int inOfs, int inLen) {
410438
update(in, inOfs, inLen);
@@ -477,38 +505,34 @@ protected Key engineUnwrap(byte[] wrappedKey, String algorithm,
477505
byte[] encoded = null;
478506

479507
update(wrappedKey, 0, wrappedKey.length);
480-
try {
481-
encoded = doFinal();
482-
} catch (BadPaddingException | IllegalBlockSizeException e) {
483-
// BadPaddingException cannot happen for TLS RSA unwrap.
484-
// In that case, padding error is indicated by returning null.
485-
// IllegalBlockSizeException cannot happen in any case,
486-
// because of the length check above.
487-
throw new InvalidKeyException("Unwrapping failed", e);
488-
}
489-
490508
try {
491509
if (isTlsRsaPremasterSecret) {
492510
if (!forTlsPremasterSecret) {
493511
throw new IllegalStateException(
494512
"No TlsRsaPremasterSecretParameterSpec specified");
495513
}
496-
497-
// polish the TLS premaster secret
498-
encoded = KeyUtil.checkTlsPreMasterSecretKey(
499-
((TlsRsaPremasterSecretParameterSpec) spec).getClientVersion(),
500-
((TlsRsaPremasterSecretParameterSpec) spec).getServerVersion(),
501-
random, encoded, encoded == null);
514+
TlsRsaPremasterSecretParameterSpec parameterSpec =
515+
(TlsRsaPremasterSecretParameterSpec) spec;
516+
encoded = doFinalForTls(parameterSpec.getClientVersion(),
517+
parameterSpec.getServerVersion());
518+
} else {
519+
encoded = doFinal();
502520
}
503-
504521
return ConstructKeys.constructKey(encoded, algorithm, type);
522+
523+
} catch (BadPaddingException | IllegalBlockSizeException e) {
524+
// BadPaddingException cannot happen for TLS RSA unwrap.
525+
// Neither padding error nor server version error is indicated
526+
// for TLS, but a fake unwrapped value is returned.
527+
// IllegalBlockSizeException cannot happen in any case,
528+
// because of the length check above.
529+
throw new InvalidKeyException("Unwrapping failed", e);
505530
} finally {
506531
if (encoded != null) {
507532
Arrays.fill(encoded, (byte) 0);
508533
}
509534
}
510535
}
511-
512536
// see JCE spec
513537
protected int engineGetKeySize(Key key) throws InvalidKeyException {
514538
RSAKey rsaKey = RSAKeyFactory.toRSAKey(key);

src/java.base/share/classes/sun/security/rsa/RSAPadding.java

Lines changed: 82 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -322,48 +322,103 @@ private byte[] padV15(byte[] data, int ofs, int len) {
322322
* Note that we want to make it a constant-time operation
323323
*/
324324
private byte[] unpadV15(byte[] padded) {
325-
int k = 0;
326-
boolean bp = false;
325+
int paddedLength = padded.length;
327326

328-
if (padded[k++] != 0) {
329-
bp = true;
330-
}
331-
if (padded[k++] != type) {
332-
bp = true;
327+
if (paddedLength < 2) {
328+
return null;
333329
}
334-
int p = 0;
335-
while (k < padded.length) {
330+
331+
// The following check ensures that the lead byte is zero and
332+
// the second byte is equivalent to the padding type. The
333+
// bp (bad padding) variable throughout this unpadding process will
334+
// be updated and remain 0 if good padding, 1 if bad.
335+
int p0 = padded[0];
336+
int p1 = padded[1];
337+
int bp = (-(p0 & 0xff) | ((p1 - type) | (type - p1))) >>> 31;
338+
339+
int padLen = 0;
340+
int k = 2;
341+
// Walk through the random, nonzero padding bytes. For each padding
342+
// byte bp and padLen will remain zero. When the end-of-padding
343+
// byte (0x00) is reached then padLen will be set to the index of the
344+
// first byte of the message content.
345+
while (k < paddedLength) {
336346
int b = padded[k++] & 0xff;
337-
if ((b == 0) && (p == 0)) {
338-
p = k;
339-
}
340-
if ((k == padded.length) && (p == 0)) {
341-
bp = true;
342-
}
343-
if ((type == PAD_BLOCKTYPE_1) && (b != 0xff) &&
344-
(p == 0)) {
345-
bp = true;
347+
padLen += (k * (1 - ((-(b | padLen)) >>> 31)));
348+
if (k == paddedLength) {
349+
bp = bp | (1 - ((-padLen) >>> 31));
346350
}
351+
bp = bp | (1 - (-(((type - PAD_BLOCKTYPE_1) & 0xff) |
352+
padLen | (1 - ((b - 0xff) >>> 31))) >>> 31));
347353
}
348-
int n = padded.length - p;
349-
if (n > maxDataSize) {
350-
bp = true;
351-
}
354+
int n = paddedLength - padLen;
355+
// So long as n <= maxDataSize, bp will remain zero
356+
bp = bp | ((maxDataSize - n) >>> 31);
352357

353358
// copy useless padding array for a constant-time method
354-
byte[] padding = new byte[p];
355-
System.arraycopy(padded, 0, padding, 0, p);
359+
byte[] padding = new byte[padLen + 2];
360+
for (int i = 0; i < padLen; i++) {
361+
padding[i] = padded[i];
362+
}
356363

357364
byte[] data = new byte[n];
358-
System.arraycopy(padded, p, data, 0, n);
365+
for (int i = 0; i < n; i++) {
366+
data[i] = padded[padLen + i];
367+
}
359368

360-
if (bp) {
369+
if ((bp | padding[bp]) != 0) {
370+
// using the array padding here hoping that this way
371+
// the compiler does not eliminate the above useless copy
361372
return null;
362373
} else {
363374
return data;
364375
}
365376
}
366377

378+
public byte[] unpadForTls(byte[] padded, int clientVersion,
379+
int serverVersion) {
380+
int paddedLength = padded.length;
381+
382+
// bp is positive if the padding is bad and 0 if it is good so far
383+
int bp = (((int) padded[0] | ((int)padded[1] - PAD_BLOCKTYPE_2)) &
384+
0xFFF);
385+
386+
int k = 2;
387+
while (k < paddedLength - 49) {
388+
int b = padded[k++] & 0xFF;
389+
bp = bp | (1 - (-b >>> 31)); // if (padded[k] == 0) bp |= 1;
390+
}
391+
bp |= ((int)padded[k++] & 0xFF);
392+
int encodedVersion = ((padded[k] & 0xFF) << 8) | (padded[k + 1] & 0xFF);
393+
394+
int bv1 = clientVersion - encodedVersion;
395+
bv1 |= -bv1;
396+
int bv3 = serverVersion - encodedVersion;
397+
bv3 |= -bv3;
398+
int bv2 = (0x301 - clientVersion);
399+
400+
bp |= ((bv1 & (bv2 | bv3)) >>> 28);
401+
402+
byte[] data = Arrays.copyOfRange(padded, paddedLength - 48,
403+
paddedLength);
404+
if (random == null) {
405+
random = JCAUtil.getSecureRandom();
406+
}
407+
408+
byte[] fake = new byte[48];
409+
random.nextBytes(fake);
410+
411+
bp = (-bp >> 24);
412+
413+
// Now bp is 0 if the padding and version number were good and
414+
// -1 otherwise.
415+
for (int i = 0; i < 48; i++) {
416+
data[i] = (byte)((~bp & data[i]) | (bp & fake[i]));
417+
}
418+
419+
return data;
420+
}
421+
367422
/**
368423
* PKCS#1 v2.0 OAEP padding (MGF1).
369424
* Paragraph references refer to PKCS#1 v2.1 (June 14, 2002)

src/java.base/share/classes/sun/security/util/KeyUtil.java

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -322,19 +322,31 @@ public static byte[] checkTlsPreMasterSecretKey(
322322
tmp = encoded;
323323
}
324324

325+
// At this point tmp.length is 48
325326
int encodedVersion =
326327
((tmp[0] & 0xFF) << 8) | (tmp[1] & 0xFF);
327-
int check1 = 0;
328-
int check2 = 0;
329-
int check3 = 0;
330-
if (clientVersion != encodedVersion) check1 = 1;
331-
if (clientVersion > 0x0301) check2 = 1;
332-
if (serverVersion != encodedVersion) check3 = 1;
333-
if ((check1 & (check2 | check3)) == 1) {
334-
return replacer;
335-
} else {
336-
return tmp;
328+
329+
// The following code is a time-constant version of
330+
// if ((clientVersion != encodedVersion) ||
331+
// ((clientVersion > 0x301) && (serverVersion != encodedVersion))) {
332+
// return replacer;
333+
// } else { return tmp; }
334+
int check1 = (clientVersion - encodedVersion) |
335+
(encodedVersion - clientVersion);
336+
int check2 = 0x0301 - clientVersion;
337+
int check3 = (serverVersion - encodedVersion) |
338+
(encodedVersion - serverVersion);
339+
340+
check1 = (check1 & (check2 | check3)) >> 24;
341+
342+
// Now check1 is either 0 or -1
343+
check2 = ~check1;
344+
345+
for (int i = 0; i < 48; i++) {
346+
tmp[i] = (byte) ((tmp[i] & check2) | (replacer[i] & check1));
337347
}
348+
349+
return tmp;
338350
}
339351

340352
/**

0 commit comments

Comments
 (0)