Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the issue of not being able to configure only server name in the secureSocket config #2237

Merged
merged 7 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -100,3 +106,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");
}
}
3 changes: 2 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1409,13 +1409,7 @@ public static void populateSSLConfiguration(SslConfiguration senderConfiguration
}
Object cert = secureSocket.get(HttpConstants.SECURESOCKET_CONFIG_CERT);
if (cert == null) {
BMap<BString, Object> 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);
}
senderConfiguration.useJavaDefaults();
} else {
evaluateCertField(cert, senderConfiguration);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ private SSLConfig getSSLConfigForListener() {
}

private SSLConfig getSSLConfigForSender() {
setSslParameters();
if (sslConfig.isDisableSsl() || sslConfig.useJavaDefaults()) {
return sslConfig;
}
Expand All @@ -265,7 +266,10 @@ private SSLConfig getSSLConfigForSender() {
sslConfig.setSSLProtocol(sslProtocol);
String tlsStoreType = sslConfig.getTLSStoreType() != null ? sslConfig.getTLSStoreType() : JKS;
sslConfig.setTLSStoreType(tlsStoreType);
return sslConfig;
}

private void setSslParameters() {
if (parameters != null) {
for (Parameter parameter : parameters) {
switch (parameter.getName()) {
Expand All @@ -287,6 +291,5 @@ private SSLConfig getSSLConfigForSender() {
}
}
}
return sslConfig;
}
}
Loading