From 1efb93e26b6e2b1afcfe3f2e0682d57d451367ce Mon Sep 17 00:00:00 2001 From: bhashinee Date: Wed, 4 Dec 2024 20:52:10 +0530 Subject: [PATCH 1/4] Fix the SNI for default certs --- .../tests/ssl_disable_ssl_test.bal | 16 -------- .../ballerina/stdlib/http/api/HttpUtil.java | 8 ++-- .../contract/config/SslConfiguration.java | 41 +++++++++---------- 3 files changed, 25 insertions(+), 40 deletions(-) diff --git a/ballerina-tests/http-security-tests/tests/ssl_disable_ssl_test.bal b/ballerina-tests/http-security-tests/tests/ssl_disable_ssl_test.bal index 21bf5b34f6..f1b003b61c 100644 --- a/ballerina-tests/http-security-tests/tests/ssl_disable_ssl_test.bal +++ b/ballerina-tests/http-security-tests/tests/ssl_disable_ssl_test.bal @@ -61,19 +61,3 @@ public function testSslDisabledClient1() returns error? { test:assertFail(msg = "Found unexpected output: " + resp.message()); } } - -http:ClientConfiguration disableSslClientConf2 = { - secureSocket: { - } -}; - -@test:Config {} -public function testSslDisabledClient2() { - http:Client|error httpClient = new ("https://localhost:9238", disableSslClientConf2); - string expectedErrMsg = "Need to configure cert with client SSL certificates file"; - if (httpClient is error) { - test:assertEquals(httpClient.message(), expectedErrMsg); - } else { - test:assertFail(msg = "Expected mutual SSL error not found"); - } -} diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java b/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java index e3710ad818..5eba6a537d 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java @@ -1409,13 +1409,15 @@ public static void populateSSLConfiguration(SslConfiguration senderConfiguration } Object cert = secureSocket.get(HttpConstants.SECURESOCKET_CONFIG_CERT); if (cert == null) { + senderConfiguration.useJavaDefaults(); BMap key = getBMapValueIfPresent(secureSocket, HttpConstants.SECURESOCKET_CONFIG_KEY); if (key != null) { senderConfiguration.useJavaDefaults(); - } else { - throw createHttpError("Need to configure cert with client SSL certificates file", - HttpErrorType.SSL_ERROR); } +// else { +// throw createHttpError("Need to configure cert with client SSL certificates file", +// HttpErrorType.SSL_ERROR); +// } } else { evaluateCertField(cert, senderConfiguration); } diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java index 0db8ffdeb8..47cb4b5659 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java @@ -245,27 +245,6 @@ private SSLConfig getSSLConfigForListener() { } private SSLConfig getSSLConfigForSender() { - if (sslConfig.isDisableSsl() || sslConfig.useJavaDefaults()) { - return sslConfig; - } - if ((sslConfig.getTrustStore() == null || sslConfig.getTrustStorePass() == null) && ( - sslConfig.getClientTrustCertificates() == null)) { - throw new IllegalArgumentException("TrustStoreFile or TrustStorePassword not defined for HTTPS/WS scheme"); - } - if (sslConfig.getTrustStore() != null) { - if (!sslConfig.getTrustStore().exists()) { - throw new IllegalArgumentException("TrustStore File " + sslConfig.getTrustStore() + " not found"); - } - sslConfig.setCertPass(sslConfig.getKeyStorePass()); - } else if (!sslConfig.getClientTrustCertificates().exists()) { - throw new IllegalArgumentException("Key file or server certificates file not found"); - } - sslConfig.setTrustStore(sslConfig.getTrustStore()).setTrustStorePass(sslConfig.getTrustStorePass()); - String sslProtocol = sslConfig.getSSLProtocol() != null ? sslConfig.getSSLProtocol() : TLS_PROTOCOL; - sslConfig.setSSLProtocol(sslProtocol); - String tlsStoreType = sslConfig.getTLSStoreType() != null ? sslConfig.getTLSStoreType() : JKS; - sslConfig.setTLSStoreType(tlsStoreType); - if (parameters != null) { for (Parameter parameter : parameters) { switch (parameter.getName()) { @@ -287,6 +266,26 @@ private SSLConfig getSSLConfigForSender() { } } } + if (sslConfig.isDisableSsl() || sslConfig.useJavaDefaults()) { + return sslConfig; + } + if ((sslConfig.getTrustStore() == null || sslConfig.getTrustStorePass() == null) && ( + sslConfig.getClientTrustCertificates() == null)) { + throw new IllegalArgumentException("TrustStoreFile or TrustStorePassword not defined for HTTPS/WS scheme"); + } + if (sslConfig.getTrustStore() != null) { + if (!sslConfig.getTrustStore().exists()) { + throw new IllegalArgumentException("TrustStore File " + sslConfig.getTrustStore() + " not found"); + } + sslConfig.setCertPass(sslConfig.getKeyStorePass()); + } else if (!sslConfig.getClientTrustCertificates().exists()) { + throw new IllegalArgumentException("Key file or server certificates file not found"); + } + sslConfig.setTrustStore(sslConfig.getTrustStore()).setTrustStorePass(sslConfig.getTrustStorePass()); + String sslProtocol = sslConfig.getSSLProtocol() != null ? sslConfig.getSSLProtocol() : TLS_PROTOCOL; + sslConfig.setSSLProtocol(sslProtocol); + String tlsStoreType = sslConfig.getTLSStoreType() != null ? sslConfig.getTLSStoreType() : JKS; + sslConfig.setTLSStoreType(tlsStoreType); return sslConfig; } } From ccb2db64f334c2236be8b3e85072d198e3771bbb Mon Sep 17 00:00:00 2001 From: bhashinee Date: Wed, 4 Dec 2024 21:42:59 +0530 Subject: [PATCH 2/4] Add test cases for default certs with SNI --- .../tests/ssl_sni_host_name_test.bal | 22 +++++++++ .../ballerina/stdlib/http/api/HttpUtil.java | 8 ---- .../contract/config/SslConfiguration.java | 46 ++++++++++--------- 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal b/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal index 46eb64b1cb..f64c588ff0 100644 --- a/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal +++ b/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal @@ -100,3 +100,25 @@ public function testSniFailure() returns error? { test:assertFail("Test `testSniFailure` is expecting an error. But received a success response"); } } + +@test:Config {} +public function testSniWhenUsingDefaultCerts() returns error? { + http:Client httpClient = check new("https://www.google.com", http2SniClientConf3); + string|error resp = httpClient->get("/"); + // This response is success because even though we send a wrong server name, google.com sends the default cert which + // is valid and trusted by the client. + if resp is error { + test:assertFail("Found unexpected output: " + resp.message()); + } +} + +@test:Config {} +public function testSniFailureWhenUsingDefaultCerts() returns error? { + http:Client clientEP = check new ("https://127.0.0.1:9208", http2SniClientConf3); + string|error resp = clientEP->get("/http1SniService/"); + if resp is error { + common:assertTrueTextPayload(resp.message(), "SSL connection failed:javax.net.ssl.SSLHandshakeException:"); + } else { + test:assertFail("Test `testSniFailureWhenUsingDefaultCerts` is expecting an error. But received a success response"); + } +} diff --git a/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java b/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java index 5eba6a537d..b15cd28180 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/http/api/HttpUtil.java @@ -1410,14 +1410,6 @@ public static void populateSSLConfiguration(SslConfiguration senderConfiguration Object cert = secureSocket.get(HttpConstants.SECURESOCKET_CONFIG_CERT); if (cert == null) { senderConfiguration.useJavaDefaults(); - BMap key = getBMapValueIfPresent(secureSocket, HttpConstants.SECURESOCKET_CONFIG_KEY); - if (key != null) { - senderConfiguration.useJavaDefaults(); - } -// else { -// throw createHttpError("Need to configure cert with client SSL certificates file", -// HttpErrorType.SSL_ERROR); -// } } else { evaluateCertField(cert, senderConfiguration); } diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java index 47cb4b5659..23975cbc82 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contract/config/SslConfiguration.java @@ -245,27 +245,7 @@ private SSLConfig getSSLConfigForListener() { } private SSLConfig getSSLConfigForSender() { - if (parameters != null) { - for (Parameter parameter : parameters) { - switch (parameter.getName()) { - case CLIENT_SUPPORT_CIPHERS: - sslConfig.setCipherSuites(parameter.getValue()); - break; - case CLIENT_SUPPORT_SSL_PROTOCOLS: - sslConfig.setEnableProtocols(parameter.getValue()); - break; - case CLIENT_ENABLE_SESSION_CREATION: - sslConfig.setEnableSessionCreation(Boolean.parseBoolean(parameter.getValue())); - break; - case SNI_SERVER_NAME: - sslConfig.setSniHostName(parameter.getValue()); - break; - default: - //do nothing - break; - } - } - } + setSslParameters(); if (sslConfig.isDisableSsl() || sslConfig.useJavaDefaults()) { return sslConfig; } @@ -288,4 +268,28 @@ private SSLConfig getSSLConfigForSender() { sslConfig.setTLSStoreType(tlsStoreType); return sslConfig; } + + private void setSslParameters() { + if (parameters != null) { + for (Parameter parameter : parameters) { + switch (parameter.getName()) { + case CLIENT_SUPPORT_CIPHERS: + sslConfig.setCipherSuites(parameter.getValue()); + break; + case CLIENT_SUPPORT_SSL_PROTOCOLS: + sslConfig.setEnableProtocols(parameter.getValue()); + break; + case CLIENT_ENABLE_SESSION_CREATION: + sslConfig.setEnableSessionCreation(Boolean.parseBoolean(parameter.getValue())); + break; + case SNI_SERVER_NAME: + sslConfig.setSniHostName(parameter.getValue()); + break; + default: + //do nothing + break; + } + } + } + } } From b92b340e6a74ec1b1b498cc30352b777611098ee Mon Sep 17 00:00:00 2001 From: bhashinee Date: Wed, 4 Dec 2024 21:56:12 +0530 Subject: [PATCH 3/4] Add the missing client config --- .../http-security-tests/tests/ssl_sni_host_name_test.bal | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal b/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal index f64c588ff0..7eaa52b7ea 100644 --- a/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal +++ b/ballerina-tests/http-security-tests/tests/ssl_sni_host_name_test.bal @@ -76,6 +76,12 @@ http:ClientConfiguration http1SniClientConf2 = { } }; +http:ClientConfiguration http2SniClientConf3 = { + secureSocket: { + serverName: "localhost" + } +}; + @test:Config {} public function testHttp2WithSni() returns error? { http:Client clientEP = check new ("https://127.0.0.1:9207", http2SniClientConf); From 60c18aff9a83df6b6119b2f999b6f2b49fba28cf Mon Sep 17 00:00:00 2001 From: bhashinee Date: Thu, 5 Dec 2024 06:53:40 +0530 Subject: [PATCH 4/4] Update the change log --- changelog.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/changelog.md b/changelog.md index 6b36b464ef..d262664c18 100644 --- a/changelog.md +++ b/changelog.md @@ -6,7 +6,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] -- [Add support for configuring server name to be used in the SSL SNI extension](https://github.com/ballerina-platform/ballerina-library/issues/7435) ### Added @@ -18,12 +17,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Migrate client and service data binding lang utils usage into data.jsondata module utils `toJson` and `parserAsType`](https://github.com/ballerina-platform/ballerina-library/issues/6747) - [Add static code rules](https://github.com/ballerina-platform/ballerina-library/issues/7283) - [Add relax data binding support for service and client data binding](https://github.com/ballerina-platform/ballerina-library/issues/7366) +- [Add support for configuring server name to be used in the SSL SNI extension](https://github.com/ballerina-platform/ballerina-library/issues/7435) ### Fixed - [Address CVE-2024-7254 vulnerability](https://github.com/ballerina-platform/ballerina-library/issues/7013) - [Fix duplicating `Content-Type` header via the `addHeader` method](https://github.com/ballerina-platform/ballerina-library/issues/7268) - [Update netty version](https://github.com/ballerina-platform/ballerina-library/issues/7358) +- [Fix the issue of not being able to configure only server name in the secureSocket config](https://github.com/ballerina-platform/ballerina-library/issues/7443) ## [2.12.0] - 2024-08-20