Skip to content

Commit 2c1eb33

Browse files
nibjendjelinski
authored andcommitted
8350830: Values converted incorrectly when reading TLS session tickets
Reviewed-by: djelinski, ascarpino
1 parent daf6fa1 commit 2c1eb33

File tree

2 files changed

+402
-65
lines changed

2 files changed

+402
-65
lines changed

src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java

Lines changed: 35 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.io.IOException;
3030
import java.net.InetAddress;
3131
import java.nio.ByteBuffer;
32+
import java.nio.charset.StandardCharsets;
3233
import java.security.Principal;
3334
import java.security.PrivateKey;
3435
import java.security.cert.X509Certificate;
@@ -310,113 +311,90 @@ final class SSLSessionImpl extends ExtendedSSLSession {
310311
SSLSessionImpl(HandshakeContext hc, ByteBuffer buf) throws IOException {
311312
boundValues = new ConcurrentHashMap<>();
312313
this.protocolVersion =
313-
ProtocolVersion.valueOf(Short.toUnsignedInt(buf.getShort()));
314+
ProtocolVersion.valueOf(Record.getInt16(buf));
314315

315316
// The CH session id may reset this if it's provided
316317
this.sessionId = new SessionId(true,
317318
hc.sslContext.getSecureRandom());
318319

319320
this.cipherSuite =
320-
CipherSuite.valueOf(Short.toUnsignedInt(buf.getShort()));
321+
CipherSuite.valueOf(Record.getInt16(buf));
321322

322323
// Local Supported signature algorithms
323324
ArrayList<SignatureScheme> list = new ArrayList<>();
324-
int i = Byte.toUnsignedInt(buf.get());
325+
int i = Record.getInt8(buf);
325326
while (i-- > 0) {
326327
list.add(SignatureScheme.valueOf(
327-
Short.toUnsignedInt(buf.getShort())));
328+
Record.getInt16(buf)));
328329
}
329330
this.localSupportedSignAlgs = Collections.unmodifiableCollection(list);
330331

331332
// Peer Supported signature algorithms
332-
i = Byte.toUnsignedInt(buf.get());
333+
i = Record.getInt8(buf);
333334
list.clear();
334335
while (i-- > 0) {
335336
list.add(SignatureScheme.valueOf(
336-
Short.toUnsignedInt(buf.getShort())));
337+
Record.getInt16(buf)));
337338
}
338339
this.peerSupportedSignAlgs = Collections.unmodifiableCollection(list);
339340

340341
// PSK
341-
byte[] b;
342-
i = Short.toUnsignedInt(buf.getShort());
343-
if (i > 0) {
344-
b = new byte[i];
345-
// Get algorithm string
346-
buf.get(b, 0, i);
347-
// Encoded length
348-
i = Short.toUnsignedInt(buf.getShort());
349-
// Encoded SecretKey
350-
b = new byte[i];
351-
buf.get(b);
342+
byte[] b = Record.getBytes16(buf);
343+
if (b.length > 0) {
344+
b = Record.getBytes16(buf);
352345
this.preSharedKey = new SecretKeySpec(b, "TlsMasterSecret");
353346
} else {
354347
this.preSharedKey = null;
355348
}
356349

357350
// PSK identity
358-
i = buf.get();
359-
if (i > 0) {
360-
b = new byte[i];
361-
buf.get(b);
351+
b = Record.getBytes8(buf);
352+
if (b.length > 0) {
362353
this.pskIdentity = b;
363354
} else {
364355
this.pskIdentity = null;
365356
}
366357

367358
// Master secret length of secret key algorithm (one byte)
368-
i = buf.get();
369-
if (i > 0) {
370-
b = new byte[i];
371-
// Get algorithm string
372-
buf.get(b, 0, i);
373-
// Encoded length
374-
i = Short.toUnsignedInt(buf.getShort());
375-
// Encoded SecretKey
376-
b = new byte[i];
377-
buf.get(b);
359+
b = Record.getBytes8(buf);
360+
if (b.length > 0) {
361+
b = Record.getBytes16(buf);
378362
this.masterSecret = new SecretKeySpec(b, "TlsMasterSecret");
379363
} else {
380364
this.masterSecret = null;
381365
}
366+
382367
// Use extended master secret
383-
this.useExtendedMasterSecret = (buf.get() != 0);
368+
this.useExtendedMasterSecret = (Record.getInt8(buf) != 0);
384369

385370
// Identification Protocol
386-
i = buf.get();
387-
if (i == 0) {
371+
b = Record.getBytes8(buf);
372+
if (b.length == 0) {
388373
identificationProtocol = null;
389374
} else {
390-
b = new byte[i];
391-
buf.get(b);
392375
identificationProtocol = new String(b);
393376
}
394377

395378
// SNI
396-
i = buf.get(); // length
397-
if (i == 0) {
379+
b = Record.getBytes8(buf);
380+
if (b.length == 0) {
398381
serverNameIndication = null;
399382
} else {
400-
b = new byte[i];
401-
buf.get(b, 0, b.length);
402383
serverNameIndication = new SNIHostName(b);
403384
}
404385

405386
// List of SNIServerName
406-
int len = Short.toUnsignedInt(buf.getShort());
387+
int len = Record.getInt16(buf);
407388
if (len == 0) {
408389
this.requestedServerNames = Collections.emptyList();
409390
} else {
410391
requestedServerNames = new ArrayList<>();
411392
while (len > 0) {
412-
int l = buf.get();
413-
b = new byte[l];
414-
buf.get(b, 0, l);
393+
b = Record.getBytes8(buf);
415394
requestedServerNames.add(new SNIHostName(new String(b)));
416395
len--;
417396
}
418397
}
419-
420398
maximumPacketSize = buf.getInt();
421399
negotiatedMaxFragLen = buf.getInt();
422400

@@ -426,31 +404,28 @@ final class SSLSessionImpl extends ExtendedSSLSession {
426404
// Get Buffer sizes
427405

428406
// Status Response
429-
len = Short.toUnsignedInt(buf.getShort());
407+
len = Record.getInt16(buf);
430408
if (len == 0) {
431409
statusResponses = Collections.emptyList();
432410
} else {
433411
statusResponses = new ArrayList<>();
434412
}
435413
while (len-- > 0) {
436-
b = new byte[Short.toUnsignedInt(buf.getShort())];
437-
buf.get(b);
414+
b = Record.getBytes16(buf);
438415
statusResponses.add(b);
439416
}
440417

441418
// Get Peer host & port
442-
i = Byte.toUnsignedInt(buf.get());
443-
if (i == 0) {
419+
b = Record.getBytes8(buf);
420+
if (b.length == 0) {
444421
this.host = "";
445422
} else {
446-
b = new byte[i];
447-
buf.get(b, 0, i);
448423
this.host = new String(b);
449424
}
450-
this.port = Short.toUnsignedInt(buf.getShort());
425+
this.port = Record.getInt16(buf);
451426

452427
// Peer certs
453-
i = buf.get();
428+
i = Record.getInt8(buf);
454429
if (i == 0) {
455430
this.peerCerts = null;
456431
} else {
@@ -469,7 +444,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
469444
}
470445

471446
// Get local certs of PSK
472-
switch (buf.get()) {
447+
switch (Record.getInt8(buf)) {
473448
case 0:
474449
break;
475450
case 1:
@@ -491,21 +466,15 @@ final class SSLSessionImpl extends ExtendedSSLSession {
491466
case 2:
492467
// pre-shared key
493468
// Length of pre-shared key algorithm (one byte)
494-
i = buf.get();
495-
b = new byte[i];
496-
buf.get(b, 0, i);
469+
b = Record.getBytes8(buf);
497470
String alg = new String(b);
498-
// Get length of encoding
499-
i = Short.toUnsignedInt(buf.getShort());
500471
// Get encoding
501-
b = new byte[i];
502-
buf.get(b);
472+
b = Record.getBytes16(buf);
503473
this.preSharedKey = new SecretKeySpec(b, alg);
504474
// Get identity len
505-
i = buf.get();
475+
i = Record.getInt8(buf);
506476
if (i > 0) {
507-
this.pskIdentity = new byte[buf.get()];
508-
buf.get(pskIdentity);
477+
this.pskIdentity = Record.getBytes8(buf);
509478
} else {
510479
this.pskIdentity = null;
511480
}
@@ -519,6 +488,7 @@ final class SSLSessionImpl extends ExtendedSSLSession {
519488
this.lastUsedTime = System.currentTimeMillis();
520489
}
521490

491+
522492
// Some situations we cannot provide a stateless ticket, but after it
523493
// has been negotiated
524494
boolean isStatelessable() {

0 commit comments

Comments
 (0)