Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.
/ jdk13u-dev Public archive

Commit

Permalink
8215712: Parsing extension failure may alert decode_error
Browse files Browse the repository at this point in the history 
Reviewed-by: dcherepanov
Backport-of: 36af90a
  • Loading branch information
Alexey Bakhtin authored and Dmitry Cherepanov committed Aug 12, 2021
1 parent 481b6f8 commit 9fe8588
Show file tree
Hide file tree
Showing 21 changed files with 313 additions and 429 deletions.
38 changes: 16 additions & 22 deletions src/java.base/share/classes/sun/security/ssl/AlpnExtension.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -89,29 +89,33 @@ private AlpnSpec(String[] applicationProtocols) {
Arrays.asList(applicationProtocols));
}

private AlpnSpec(ByteBuffer buffer) throws IOException {
private AlpnSpec(HandshakeContext hc,
ByteBuffer buffer) throws IOException {
// ProtocolName protocol_name_list<2..2^16-1>, RFC 7301.
if (buffer.remaining() < 2) {
throw new SSLProtocolException(
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid application_layer_protocol_negotiation: " +
"insufficient data (length=" + buffer.remaining() + ")");
"insufficient data (length=" + buffer.remaining() + ")"));
}

int listLen = Record.getInt16(buffer);
if (listLen < 2 || listLen != buffer.remaining()) {
throw new SSLProtocolException(
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid application_layer_protocol_negotiation: " +
"incorrect list length (length=" + listLen + ")");
"incorrect list length (length=" + listLen + ")"));
}

List<String> protocolNames = new LinkedList<>();
while (buffer.hasRemaining()) {
// opaque ProtocolName<1..2^8-1>, RFC 7301.
byte[] bytes = Record.getBytes8(buffer);
if (bytes.length == 0) {
throw new SSLProtocolException(
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid application_layer_protocol_negotiation " +
"extension: empty application protocol name");
"extension: empty application protocol name"));
}

String appProtocol = new String(bytes, alpnCharset);
Expand All @@ -130,9 +134,9 @@ public String toString() {

private static final class AlpnStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new AlpnSpec(buffer)).toString();
return (new AlpnSpec(hc, buffer)).toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand Down Expand Up @@ -301,12 +305,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
AlpnSpec spec;
try {
spec = new AlpnSpec(buffer);
} catch (IOException ioe) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
AlpnSpec spec = new AlpnSpec(shc, buffer);

// Update the context.
if (noAPSelector) { // noAlpnProtocols is false
Expand Down Expand Up @@ -482,12 +481,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
AlpnSpec spec;
try {
spec = new AlpnSpec(buffer);
} catch (IOException ioe) {
throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
AlpnSpec spec = new AlpnSpec(chc, buffer);

// Only one application protocol is allowed.
if (spec.applicationProtocols.size() != 1) {
Expand Down
21 changes: 6 additions & 15 deletions src/java.base/share/classes/sun/security/ssl/CertSignAlgsExtension.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -56,9 +56,10 @@ final class CertSignAlgsExtension {
private static final
class CertSignatureSchemesStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new SignatureSchemesSpec(buffer)).toString();
return (new SignatureSchemesSpec(hc, buffer))
.toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand Down Expand Up @@ -150,12 +151,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
SignatureSchemesSpec spec;
try {
spec = new SignatureSchemesSpec(buffer);
} catch (IOException ioe) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
SignatureSchemesSpec spec = new SignatureSchemesSpec(shc, buffer);

// Update the context.
shc.handshakeExtensions.put(
Expand Down Expand Up @@ -295,12 +291,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
SignatureSchemesSpec spec;
try {
spec = new SignatureSchemesSpec(buffer);
} catch (IOException ioe) {
throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
SignatureSchemesSpec spec = new SignatureSchemesSpec(chc, buffer);

// Update the context.
chc.handshakeExtensions.put(
Expand Down
74 changes: 35 additions & 39 deletions src/java.base/share/classes/sun/security/ssl/CertStatusExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ private CertStatusRequestSpec(CertStatusRequest statusRequest) {
this.statusRequest = statusRequest;
}

private CertStatusRequestSpec(ByteBuffer buffer) throws IOException {
private CertStatusRequestSpec(HandshakeContext hc,
ByteBuffer buffer) throws IOException {
// Is it a empty extension_data?
if (buffer.remaining() == 0) {
// server response
Expand All @@ -130,8 +131,9 @@ private CertStatusRequestSpec(ByteBuffer buffer) throws IOException {
}

if (buffer.remaining() < 1) {
throw new SSLProtocolException(
"Invalid status_request extension: insufficient data");
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid status_request extension: insufficient data"));
}

byte statusType = (byte)Record.getInt8(buffer);
Expand Down Expand Up @@ -178,10 +180,12 @@ private CertStatusResponseSpec(CertStatusResponse resp) {
this.statusResponse = resp;
}

private CertStatusResponseSpec(ByteBuffer buffer) throws IOException {
private CertStatusResponseSpec(HandshakeContext hc,
ByteBuffer buffer) throws IOException {
if (buffer.remaining() < 2) {
throw new SSLProtocolException(
"Invalid status_request extension: insufficient data");
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid status_request extension: insufficient data"));
}

// Get the status type (1 byte) and response data (vector)
Expand Down Expand Up @@ -212,9 +216,9 @@ public String toString() {
private static final
class CertStatusRequestStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new CertStatusRequestSpec(buffer)).toString();
return (new CertStatusRequestSpec(hc, buffer)).toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand All @@ -225,9 +229,9 @@ public String toString(ByteBuffer buffer) {
private static final
class CertStatusRespStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new CertStatusResponseSpec(buffer)).toString();
return (new CertStatusResponseSpec(hc, buffer)).toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand Down Expand Up @@ -599,12 +603,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
CertStatusRequestSpec spec;
try {
spec = new CertStatusRequestSpec(buffer);
} catch (IOException ioe) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
CertStatusRequestSpec spec = new CertStatusRequestSpec(shc, buffer);

// Update the context.
shc.handshakeExtensions.put(SSLExtension.CH_STATUS_REQUEST, spec);
Expand Down Expand Up @@ -776,7 +775,8 @@ private CertStatusRequestV2Spec(CertStatusRequest[] certStatusRequests) {
this.certStatusRequests = certStatusRequests;
}

private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
private CertStatusRequestV2Spec(HandshakeContext hc,
ByteBuffer message) throws IOException {
// Is it a empty extension_data?
if (message.remaining() == 0) {
// server response
Expand All @@ -787,15 +787,17 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
if (message.remaining() < 5) { // 2: certificate_status_req_list
// +1: status_type
// +2: request_length
throw new SSLProtocolException(
"Invalid status_request_v2 extension: insufficient data");
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid status_request_v2 extension: insufficient data"));
}

int listLen = Record.getInt16(message);
if (listLen <= 0) {
throw new SSLProtocolException(
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"certificate_status_req_list length must be positive " +
"(received length: " + listLen + ")");
"(received length: " + listLen + ")"));
}

int remaining = listLen;
Expand All @@ -805,10 +807,12 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
int requestLen = Record.getInt16(message);

if (message.remaining() < requestLen) {
throw new SSLProtocolException(
throw hc.conContext.fatal(
Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid status_request_v2 extension: " +
"insufficient data (request_length=" + requestLen +
", remining=" + message.remaining() + ")");
", remining=" + message.remaining() + ")"));
}

byte[] encoded = new byte[requestLen];
Expand All @@ -823,9 +827,11 @@ private CertStatusRequestV2Spec(ByteBuffer message) throws IOException {
if (encoded.length < 4) {
// 2: length of responder_id_list
// +2: length of request_extensions
throw new SSLProtocolException(
throw hc.conContext.fatal(
Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid status_request_v2 extension: " +
"insufficient data");
"insufficient data"));
}
statusRequests.add(
new OCSPStatusRequest(statusType, encoded));
Expand Down Expand Up @@ -874,9 +880,9 @@ public String toString() {
private static final
class CertStatusRequestsStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new CertStatusRequestV2Spec(buffer)).toString();
return (new CertStatusRequestV2Spec(hc, buffer)).toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand Down Expand Up @@ -957,12 +963,7 @@ public void consume(ConnectionContext context,
}

// Parse the extension.
CertStatusRequestV2Spec spec;
try {
spec = new CertStatusRequestV2Spec(buffer);
} catch (IOException ioe) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}
CertStatusRequestV2Spec spec = new CertStatusRequestV2Spec(shc, buffer);

// Update the context.
shc.handshakeExtensions.put(SSLExtension.CH_STATUS_REQUEST_V2,
Expand Down Expand Up @@ -1185,12 +1186,7 @@ public void consume(ConnectionContext context,
ClientHandshakeContext chc = (ClientHandshakeContext)context;

// Parse the extension.
CertStatusResponseSpec spec;
try {
spec = new CertStatusResponseSpec(buffer);
} catch (IOException ioe) {
throw chc.conContext.fatal(Alert.DECODE_ERROR, ioe);
}
CertStatusResponseSpec spec = new CertStatusResponseSpec(chc, buffer);

if (chc.sslContext.isStaplingEnabled(true)) {
// Activate stapling
Expand Down
30 changes: 10 additions & 20 deletions src/java.base/share/classes/sun/security/ssl/CookieExtension.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -63,11 +63,13 @@ public class CookieExtension {
static class CookieSpec implements SSLExtensionSpec {
final byte[] cookie;

private CookieSpec(ByteBuffer m) throws IOException {
private CookieSpec(HandshakeContext hc,
ByteBuffer m) throws IOException {
// opaque cookie<1..2^16-1>;
if (m.remaining() < 3) {
throw new SSLProtocolException(
"Invalid cookie extension: insufficient data");
throw hc.conContext.fatal(Alert.DECODE_ERROR,
new SSLProtocolException(
"Invalid cookie extension: insufficient data"));
}

this.cookie = Record.getBytes16(m);
Expand All @@ -90,9 +92,9 @@ public String toString() {

private static final class CookieStringizer implements SSLStringizer {
@Override
public String toString(ByteBuffer buffer) {
public String toString(HandshakeContext hc, ByteBuffer buffer) {
try {
return (new CookieSpec(buffer)).toString();
return (new CookieSpec(hc, buffer)).toString();
} catch (IOException ioe) {
// For debug logging only, so please swallow exceptions.
return ioe.getMessage();
Expand Down Expand Up @@ -159,13 +161,7 @@ public void consume(ConnectionContext context,
return; // ignore the extension
}

CookieSpec spec;
try {
spec = new CookieSpec(buffer);
} catch (IOException ioe) {
throw shc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}

CookieSpec spec = new CookieSpec(shc, buffer);
shc.handshakeExtensions.put(SSLExtension.CH_COOKIE, spec);

// No impact on session resumption.
Expand Down Expand Up @@ -264,13 +260,7 @@ public void consume(ConnectionContext context,
return; // ignore the extension
}

CookieSpec spec;
try {
spec = new CookieSpec(buffer);
} catch (IOException ioe) {
throw chc.conContext.fatal(Alert.UNEXPECTED_MESSAGE, ioe);
}

CookieSpec spec = new CookieSpec(chc, buffer);
chc.handshakeExtensions.put(SSLExtension.HRR_COOKIE, spec);
}
}
Expand Down
Loading

1 comment on commit 9fe8588

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

Please sign in to comment.