From d1335283dfa1b28bb04d9b309a785bd3bb2425ff Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Tue, 11 Jun 2024 13:07:50 +0200 Subject: [PATCH 1/2] Respond with TLS alert "bad_certificate" if certificate parsing fails --- .../java/org/bouncycastle/tls/TlsUtils.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java b/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java index 69b8930203..6a53a69e74 100644 --- a/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java +++ b/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java @@ -5188,7 +5188,14 @@ static TlsAuthentication receiveServerCertificate(TlsClientContext clientContext .setCertificateType(securityParameters.getServerCertificateType()) .setMaxChainLength(client.getMaxCertificateChainLength()); - Certificate serverCertificate = Certificate.parse(options, clientContext, buf, endPointHash); + Certificate serverCertificate; + + try + { + serverCertificate = Certificate.parse(options, clientContext, buf, endPointHash); + } catch (IOException e) { + throw new TlsFatalAlert(AlertDescription.bad_certificate, e); + } TlsProtocol.assertEmpty(buf); @@ -5229,7 +5236,13 @@ static TlsAuthentication receive13ServerCertificate(TlsClientContext clientConte .setCertificateType(securityParameters.getServerCertificateType()) .setMaxChainLength(client.getMaxCertificateChainLength()); - Certificate serverCertificate = Certificate.parse(options, clientContext, buf, null); + Certificate serverCertificate; + + try { + serverCertificate = Certificate.parse(options, clientContext, buf, null); + } catch (IOException e) { + throw new TlsFatalAlert(AlertDescription.bad_certificate, e); + } TlsProtocol.assertEmpty(buf); From 744637bfadce556d47bf16058881af2ac297f197 Mon Sep 17 00:00:00 2001 From: Markus Jung Date: Wed, 19 Jun 2024 14:13:20 +0200 Subject: [PATCH 2/2] Refine response to broken certificates received during TLS handshake - Refactor response into common static method - Apply response to certificates received by server, too - Only throw a new TlsFatalAlert in case of an unspecific IOException --- .../bouncycastle/tls/TlsServerProtocol.java | 4 +-- .../java/org/bouncycastle/tls/TlsUtils.java | 34 +++++++++++-------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java b/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java index 3ace602565..37b0dad37d 100644 --- a/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java +++ b/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java @@ -1459,7 +1459,7 @@ protected void receive13ClientCertificate(ByteArrayInputStream buf) .setCertificateType(tlsServerContext.getSecurityParametersHandshake().getClientCertificateType()) .setMaxChainLength(tlsServer.getMaxCertificateChainLength()); - Certificate clientCertificate = Certificate.parse(options, tlsServerContext, buf, null); + Certificate clientCertificate = TlsUtils.parseCertificate(options, tlsServerContext, buf, null); assertEmpty(buf); @@ -1499,7 +1499,7 @@ protected void receiveCertificateMessage(ByteArrayInputStream buf) .setCertificateType(tlsServerContext.getSecurityParametersHandshake().getClientCertificateType()) .setMaxChainLength(tlsServer.getMaxCertificateChainLength()); - Certificate clientCertificate = Certificate.parse(options, tlsServerContext, buf, null); + Certificate clientCertificate = TlsUtils.parseCertificate(options, tlsServerContext, buf, null); assertEmpty(buf); diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java b/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java index 6a53a69e74..0803cea3e7 100644 --- a/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java +++ b/tls/src/main/java/org/bouncycastle/tls/TlsUtils.java @@ -28,6 +28,7 @@ import org.bouncycastle.asn1.rosstandart.RosstandartObjectIdentifiers; import org.bouncycastle.asn1.x509.X509ObjectIdentifiers; import org.bouncycastle.asn1.x9.X9ObjectIdentifiers; +import org.bouncycastle.tls.Certificate.ParseOptions; import org.bouncycastle.tls.crypto.Tls13Verifier; import org.bouncycastle.tls.crypto.TlsAgreement; import org.bouncycastle.tls.crypto.TlsCertificate; @@ -5172,6 +5173,22 @@ private static boolean isSafeRenegotiationServerCertificate(TlsClientContext cli return false; } + static Certificate parseCertificate(ParseOptions options, TlsContext context, ByteArrayInputStream buf, + ByteArrayOutputStream endPointHashOutput) throws TlsFatalAlert { + try + { + return Certificate.parse(options, context, buf, endPointHashOutput); + } + catch (TlsFatalAlert e) + { + throw e; + } + catch (IOException e) + { + throw new TlsFatalAlert(AlertDescription.bad_certificate, e); + } + } + static TlsAuthentication receiveServerCertificate(TlsClientContext clientContext, TlsClient client, ByteArrayInputStream buf, Hashtable serverExtensions) throws IOException { @@ -5188,14 +5205,7 @@ static TlsAuthentication receiveServerCertificate(TlsClientContext clientContext .setCertificateType(securityParameters.getServerCertificateType()) .setMaxChainLength(client.getMaxCertificateChainLength()); - Certificate serverCertificate; - - try - { - serverCertificate = Certificate.parse(options, clientContext, buf, endPointHash); - } catch (IOException e) { - throw new TlsFatalAlert(AlertDescription.bad_certificate, e); - } + Certificate serverCertificate = parseCertificate(options, clientContext, buf, endPointHash); TlsProtocol.assertEmpty(buf); @@ -5236,13 +5246,7 @@ static TlsAuthentication receive13ServerCertificate(TlsClientContext clientConte .setCertificateType(securityParameters.getServerCertificateType()) .setMaxChainLength(client.getMaxCertificateChainLength()); - Certificate serverCertificate; - - try { - serverCertificate = Certificate.parse(options, clientContext, buf, null); - } catch (IOException e) { - throw new TlsFatalAlert(AlertDescription.bad_certificate, e); - } + Certificate serverCertificate = parseCertificate(options, clientContext, buf, null); TlsProtocol.assertEmpty(buf);