Skip to content

Commit 2adb01e

Browse files
martinuygnu-andrew
authored andcommitted
8337692: Better TLS connection support
Reviewed-by: abakhtin, andrew Backport-of: 73aa304234f2ec17abbb640b7f2d0503cf1bfc91
1 parent 318c352 commit 2adb01e

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
@@ -384,7 +384,7 @@ private byte[] doFinal() throws BadPaddingException,
384384
byte[] decryptBuffer = RSACore.convert(buffer, 0, bufOfs);
385385
paddingCopy = RSACore.rsa(decryptBuffer, privateKey, false);
386386
result = padding.unpad(paddingCopy);
387-
if (result == null && !forTlsPremasterSecret) {
387+
if (!forTlsPremasterSecret && result == null) {
388388
throw new BadPaddingException
389389
("Padding error in decryption");
390390
}
@@ -404,6 +404,34 @@ private byte[] doFinal() throws BadPaddingException,
404404
}
405405
}
406406

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

478506
update(wrappedKey, 0, wrappedKey.length);
479-
try {
480-
encoded = doFinal();
481-
} catch (BadPaddingException | IllegalBlockSizeException e) {
482-
// BadPaddingException cannot happen for TLS RSA unwrap.
483-
// In that case, padding error is indicated by returning null.
484-
// IllegalBlockSizeException cannot happen in any case,
485-
// because of the length check above.
486-
throw new InvalidKeyException("Unwrapping failed", e);
487-
}
488-
489507
try {
490508
if (isTlsRsaPremasterSecret) {
491509
if (!forTlsPremasterSecret) {
492510
throw new IllegalStateException(
493511
"No TlsRsaPremasterSecretParameterSpec specified");
494512
}
495-
496-
// polish the TLS premaster secret
497-
encoded = KeyUtil.checkTlsPreMasterSecretKey(
498-
((TlsRsaPremasterSecretParameterSpec) spec).getClientVersion(),
499-
((TlsRsaPremasterSecretParameterSpec) spec).getServerVersion(),
500-
random, encoded, encoded == null);
513+
TlsRsaPremasterSecretParameterSpec parameterSpec =
514+
(TlsRsaPremasterSecretParameterSpec) spec;
515+
encoded = doFinalForTls(parameterSpec.getClientVersion(),
516+
parameterSpec.getServerVersion());
517+
} else {
518+
encoded = doFinal();
501519
}
502-
503520
return ConstructKeys.constructKey(encoded, algorithm, type);
521+
522+
} catch (BadPaddingException | IllegalBlockSizeException e) {
523+
// BadPaddingException cannot happen for TLS RSA unwrap.
524+
// Neither padding error nor server version error is indicated
525+
// for TLS, but a fake unwrapped value is returned.
526+
// IllegalBlockSizeException cannot happen in any case,
527+
// because of the length check above.
528+
throw new InvalidKeyException("Unwrapping failed", e);
504529
} finally {
505530
if (encoded != null) {
506531
Arrays.fill(encoded, (byte) 0);
507532
}
508533
}
509534
}
510-
511535
// see JCE spec
512536
protected int engineGetKeySize(Key key) throws InvalidKeyException {
513537
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, 2020, 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
@@ -327,48 +327,103 @@ private byte[] padV15(byte[] data, int ofs, int len) {
327327
* Note that we want to make it a constant-time operation
328328
*/
329329
private byte[] unpadV15(byte[] padded) {
330-
int k = 0;
331-
boolean bp = false;
330+
int paddedLength = padded.length;
332331

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

358363
// copy useless padding array for a constant-time method
359-
byte[] padding = new byte[p];
360-
System.arraycopy(padded, 0, padding, 0, p);
364+
byte[] padding = new byte[padLen + 2];
365+
for (int i = 0; i < padLen; i++) {
366+
padding[i] = padded[i];
367+
}
361368

362369
byte[] data = new byte[n];
363-
System.arraycopy(padded, p, data, 0, n);
370+
for (int i = 0; i < n; i++) {
371+
data[i] = padded[padLen + i];
372+
}
364373

365-
if (bp) {
374+
if ((bp | padding[bp]) != 0) {
375+
// using the array padding here hoping that this way
376+
// the compiler does not eliminate the above useless copy
366377
return null;
367378
} else {
368379
return data;
369380
}
370381
}
371382

383+
public byte[] unpadForTls(byte[] padded, int clientVersion,
384+
int serverVersion) {
385+
int paddedLength = padded.length;
386+
387+
// bp is positive if the padding is bad and 0 if it is good so far
388+
int bp = (((int) padded[0] | ((int)padded[1] - PAD_BLOCKTYPE_2)) &
389+
0xFFF);
390+
391+
int k = 2;
392+
while (k < paddedLength - 49) {
393+
int b = padded[k++] & 0xFF;
394+
bp = bp | (1 - (-b >>> 31)); // if (padded[k] == 0) bp |= 1;
395+
}
396+
bp |= ((int)padded[k++] & 0xFF);
397+
int encodedVersion = ((padded[k] & 0xFF) << 8) | (padded[k + 1] & 0xFF);
398+
399+
int bv1 = clientVersion - encodedVersion;
400+
bv1 |= -bv1;
401+
int bv3 = serverVersion - encodedVersion;
402+
bv3 |= -bv3;
403+
int bv2 = (0x301 - clientVersion);
404+
405+
bp |= ((bv1 & (bv2 | bv3)) >>> 28);
406+
407+
byte[] data = Arrays.copyOfRange(padded, paddedLength - 48,
408+
paddedLength);
409+
if (random == null) {
410+
random = JCAUtil.getSecureRandom();
411+
}
412+
413+
byte[] fake = new byte[48];
414+
random.nextBytes(fake);
415+
416+
bp = (-bp >> 24);
417+
418+
// Now bp is 0 if the padding and version number were good and
419+
// -1 otherwise.
420+
for (int i = 0; i < 48; i++) {
421+
data[i] = (byte)((~bp & data[i]) | (bp & fake[i]));
422+
}
423+
424+
return data;
425+
}
426+
372427
/**
373428
* PKCS#1 v2.0 OAEP padding (MGF1).
374429
* 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
@@ -288,19 +288,31 @@ public static byte[] checkTlsPreMasterSecretKey(
288288
tmp = encoded;
289289
}
290290

291+
// At this point tmp.length is 48
291292
int encodedVersion =
292293
((tmp[0] & 0xFF) << 8) | (tmp[1] & 0xFF);
293-
int check1 = 0;
294-
int check2 = 0;
295-
int check3 = 0;
296-
if (clientVersion != encodedVersion) check1 = 1;
297-
if (clientVersion > 0x0301) check2 = 1;
298-
if (serverVersion != encodedVersion) check3 = 1;
299-
if ((check1 & (check2 | check3)) == 1) {
300-
return replacer;
301-
} else {
302-
return tmp;
294+
295+
// The following code is a time-constant version of
296+
// if ((clientVersion != encodedVersion) ||
297+
// ((clientVersion > 0x301) && (serverVersion != encodedVersion))) {
298+
// return replacer;
299+
// } else { return tmp; }
300+
int check1 = (clientVersion - encodedVersion) |
301+
(encodedVersion - clientVersion);
302+
int check2 = 0x0301 - clientVersion;
303+
int check3 = (serverVersion - encodedVersion) |
304+
(encodedVersion - serverVersion);
305+
306+
check1 = (check1 & (check2 | check3)) >> 24;
307+
308+
// Now check1 is either 0 or -1
309+
check2 = ~check1;
310+
311+
for (int i = 0; i < 48; i++) {
312+
tmp[i] = (byte) ((tmp[i] & check2) | (replacer[i] & check1));
303313
}
314+
315+
return tmp;
304316
}
305317

306318
/**

0 commit comments

Comments
 (0)