diff --git a/CHANGELOG.md b/CHANGELOG.md index 69fc079..96082fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed + +- relaxed SNI hostname resolution + ## 0.6.5 - 2025-09-16 ### Fixed diff --git a/gradle.properties b/gradle.properties index a2e2a3f..da96b92 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=0.6.5 +version=0.6.6 group=com.coder.toolbox name=coder-toolbox \ No newline at end of file diff --git a/src/main/kotlin/com/coder/toolbox/util/TLS.kt b/src/main/kotlin/com/coder/toolbox/util/TLS.kt index dac816e..0a4b72f 100644 --- a/src/main/kotlin/com/coder/toolbox/util/TLS.kt +++ b/src/main/kotlin/com/coder/toolbox/util/TLS.kt @@ -4,8 +4,10 @@ import com.coder.toolbox.settings.ReadOnlyTLSSettings import okhttp3.internal.tls.OkHostnameVerifier import java.io.File import java.io.FileInputStream +import java.net.IDN import java.net.InetAddress import java.net.Socket +import java.nio.charset.StandardCharsets import java.security.KeyFactory import java.security.KeyStore import java.security.cert.CertificateException @@ -18,11 +20,12 @@ import java.util.Locale import javax.net.ssl.HostnameVerifier import javax.net.ssl.KeyManager import javax.net.ssl.KeyManagerFactory -import javax.net.ssl.SNIHostName +import javax.net.ssl.SNIServerName import javax.net.ssl.SSLContext import javax.net.ssl.SSLSession import javax.net.ssl.SSLSocket import javax.net.ssl.SSLSocketFactory +import javax.net.ssl.StandardConstants import javax.net.ssl.TrustManager import javax.net.ssl.TrustManagerFactory import javax.net.ssl.X509TrustManager @@ -83,11 +86,13 @@ fun sslContextFromPEMs( fun coderSocketFactory(settings: ReadOnlyTLSSettings): SSLSocketFactory { val sslContext = sslContextFromPEMs(settings.certPath, settings.keyPath, settings.caPath) - if (settings.altHostname.isNullOrBlank()) { + + val altHostname = settings.altHostname + if (altHostname.isNullOrBlank()) { return sslContext.socketFactory } - return AlternateNameSSLSocketFactory(sslContext.socketFactory, settings.altHostname) + return AlternateNameSSLSocketFactory(sslContext.socketFactory, altHostname) } fun coderTrustManagers(tlsCAPath: String?): Array { @@ -111,7 +116,7 @@ fun coderTrustManagers(tlsCAPath: String?): Array { return trustManagerFactory.trustManagers.map { MergedSystemTrustManger(it as X509TrustManager) }.toTypedArray() } -class AlternateNameSSLSocketFactory(private val delegate: SSLSocketFactory, private val alternateName: String?) : +class AlternateNameSSLSocketFactory(private val delegate: SSLSocketFactory, private val alternateName: String) : SSLSocketFactory() { override fun getDefaultCipherSuites(): Array = delegate.defaultCipherSuites @@ -176,12 +181,19 @@ class AlternateNameSSLSocketFactory(private val delegate: SSLSocketFactory, priv private fun customizeSocket(socket: SSLSocket) { val params = socket.sslParameters - params.serverNames = listOf(SNIHostName(alternateName)) + + params.serverNames = listOf(RelaxedSNIHostname(alternateName)) socket.sslParameters = params } } +private class RelaxedSNIHostname(hostname: String) : SNIServerName( + StandardConstants.SNI_HOST_NAME, + IDN.toASCII(hostname, 0).toByteArray(StandardCharsets.UTF_8) +) + class CoderHostnameVerifier(private val alternateName: String?) : HostnameVerifier { + override fun verify( host: String, session: SSLSession, diff --git a/src/test/kotlin/com/coder/toolbox/util/AlternateNameSSLSocketFactoryTest.kt b/src/test/kotlin/com/coder/toolbox/util/AlternateNameSSLSocketFactoryTest.kt new file mode 100644 index 0000000..1b5460f --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/AlternateNameSSLSocketFactoryTest.kt @@ -0,0 +1,237 @@ +package com.coder.toolbox.util + +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.verify +import java.net.InetAddress +import java.net.Socket +import javax.net.ssl.SSLParameters +import javax.net.ssl.SSLSocket +import javax.net.ssl.SSLSocketFactory +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertSame + + +class AlternateNameSSLSocketFactoryTest { + + @Test + fun `createSocket with no parameters should customize socket with alternate name`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + every { mockFactory.createSocket() } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket() + + // Then + verify { mockSocket.sslParameters = any() } + assertSame(mockSocket, result) + } + + @Test + fun `createSocket with host and port should customize socket with alternate name`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + every { mockFactory.createSocket("original.com", 443) } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket("original.com", 443) + + // Then + verify { mockSocket.sslParameters = any() } + assertSame(mockSocket, result) + } + + @Test + fun `createSocket with host port and local address should customize socket`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + val localHost = mockk() + + every { mockFactory.createSocket("original.com", 443, localHost, 8080) } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket("original.com", 443, localHost, 8080) + + // Then + verify { mockSocket.sslParameters = any() } + assertSame(mockSocket, result) + } + + @Test + fun `createSocket with InetAddress should customize socket with alternate name`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + val address = mockk() + + every { mockFactory.createSocket(address, 443) } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket(address, 443) + + // Then + verify { mockSocket.sslParameters = any() } + assertSame(mockSocket, result) + } + + @Test + fun `createSocket with InetAddress and local address should customize socket`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + val address = mockk() + val localAddress = mockk() + + every { mockFactory.createSocket(address, 443, localAddress, 8080) } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket(address, 443, localAddress, 8080) + + // Then + verify { mockSocket.sslParameters = any() } + assertSame(mockSocket, result) + } + + @Test + fun `createSocket with existing socket should customize socket with alternate name`() { + // Given + val mockFactory = mockk() + val mockSSLSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + val existingSocket = mockk() + + every { mockFactory.createSocket(existingSocket, "original.com", 443, true) } returns mockSSLSocket + every { mockSSLSocket.sslParameters } returns mockParams + every { mockSSLSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "alternate.example.com") + + // When + val result = alternateFactory.createSocket(existingSocket, "original.com", 443, true) + + // Then + verify { mockSSLSocket.sslParameters = any() } + assertSame(mockSSLSocket, result) + } + + @Test + fun `customizeSocket should set SNI hostname to alternate name for valid hostname`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + every { mockFactory.createSocket() } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "valid-hostname.example.com") + + // When & Then - This should work without throwing an exception + assertNotNull(alternateFactory.createSocket()) + verify { mockSocket.sslParameters = any() } + } + + @Test + fun `customizeSocket should NOT throw IllegalArgumentException for hostname with underscore`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + every { mockFactory.createSocket() } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "non_compliant_hostname.example.com") + + // When & Then - This should work without throwing an exception + assertNotNull(alternateFactory.createSocket()) + verify { mockSocket.sslParameters = any() } + assertEquals(0, mockSocket.sslParameters.serverNames.size) + } + + @Test + fun `createSocket should work with valid international domain names`() { + // Given + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + every { mockFactory.createSocket() } returns mockSocket + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + val alternateFactory = AlternateNameSSLSocketFactory(mockFactory, "test-server.example.com") + + // When & Then - This should work as hyphens are valid + assertNotNull(alternateFactory.createSocket()) + verify { mockSocket.sslParameters = any() } + } + + private fun createMockSSLSocketFactory(): SSLSocketFactory { + val mockFactory = mockk() + val mockSocket = mockk(relaxed = true) + val mockParams = mockk(relaxed = true) + + // Setup default behavior + every { mockFactory.defaultCipherSuites } returns arrayOf("TLS_AES_256_GCM_SHA384") + every { mockFactory.supportedCipherSuites } returns arrayOf("TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256") + + // Make all createSocket methods return our mock socket + every { mockFactory.createSocket() } returns mockSocket + every { mockFactory.createSocket(any(), any()) } returns mockSocket + every { mockFactory.createSocket(any(), any(), any(), any()) } returns mockSocket + every { mockFactory.createSocket(any(), any()) } returns mockSocket + every { + mockFactory.createSocket( + any(), + any(), + any(), + any() + ) + } returns mockSocket + every { mockFactory.createSocket(any(), any(), any(), any()) } returns mockSocket + + // Setup SSL parameters + every { mockSocket.sslParameters } returns mockParams + every { mockSocket.sslParameters = any() } just Runs + + return mockFactory + } +} \ No newline at end of file diff --git a/src/test/kotlin/com/coder/toolbox/util/CoderHostnameVerifierTest.kt b/src/test/kotlin/com/coder/toolbox/util/CoderHostnameVerifierTest.kt new file mode 100644 index 0000000..f2bb0d2 --- /dev/null +++ b/src/test/kotlin/com/coder/toolbox/util/CoderHostnameVerifierTest.kt @@ -0,0 +1,238 @@ +package com.coder.toolbox.util + +import io.mockk.every +import io.mockk.mockk +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.slf4j.Logger +import java.security.cert.Certificate +import java.security.cert.X509Certificate +import javax.net.ssl.SSLSession +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class CoderHostnameVerifierTest { + + private lateinit var sslSession: SSLSession + private lateinit var x509Certificate: X509Certificate + private lateinit var logger: Logger + private lateinit var verifier: CoderHostnameVerifier + + @BeforeEach + fun setUp() { + sslSession = mockk() + x509Certificate = mockk() + logger = mockk(relaxed = true) + } + + @Test + fun `should return false when no certificates are present`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + every { sslSession.peerCertificates } returns null + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertFalse(result) + } + + @Test + fun `should return false when certificates array is empty`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + every { sslSession.peerCertificates } returns arrayOf() + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertFalse(result) + } + + @Test + fun `should return true when SAN contains matching alternate name with underscore`() { + // Given + val alternateNameWithUnderscore = "test_server.internal.com" + verifier = CoderHostnameVerifier(alternateNameWithUnderscore) + + // Mock certificate with SAN containing underscore + val sanEntries = listOf( + listOf(2, "example.com"), // Standard DNS name + listOf(2, "test_server.internal.com"), // SAN with underscore + listOf(2, "api.example.com") // Another DNS name + ) + + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + every { x509Certificate.subjectAlternativeNames } returns sanEntries + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertTrue(result, "Should return true when SAN contains matching alternate name with underscore") + } + + @Test + fun `should return false when SAN does not contain matching alternate name`() { + // Given + verifier = CoderHostnameVerifier("missing_host.example.com") + + // Mock certificate without matching SAN + val sanEntries = listOf( + listOf(2, "example.com"), + listOf(2, "api.example.com"), + listOf(2, "different_host.example.com") + ) + + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + every { x509Certificate.subjectAlternativeNames } returns sanEntries + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertFalse(result, "Should return false when SAN does not contain matching alternate name") + } + + @Test + fun `should ignore non-DNS SAN entries`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + + // Mock certificate with various SAN types + val sanEntries = listOf( + listOf(1, "user@example.com"), // Email (type 1) + listOf(6, "http://example.com"), // URI (type 6) + listOf(7, "192.168.1.1"), // IP Address (type 7) + listOf(2, "test_host.example.com") // DNS Name (type 2) - this should match + ) + + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + every { x509Certificate.subjectAlternativeNames } returns sanEntries + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertTrue(result, "Should ignore non-DNS SAN entries and find the matching DNS entry") + } + + @Test + fun `should return false when certificate has no SAN extension`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + every { x509Certificate.subjectAlternativeNames } returns null + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertFalse(result, "Should return false when certificate has no SAN extension") + } + + @Test + fun `should handle multiple certificates and find match in second certificate`() { + // Given + verifier = CoderHostnameVerifier("api_server.internal.com") + + val cert1Mock = mockk() + val cert2Mock = mockk() + + // First certificate has no matching SAN + val sanEntries1 = listOf( + listOf(2, "example.com"), + listOf(2, "www.example.com") + ) + + // Second certificate has matching SAN with underscore + val sanEntries2 = listOf( + listOf(2, "internal.com"), + listOf(2, "api_server.internal.com") + ) + + every { sslSession.peerCertificates } returns arrayOf(cert1Mock, cert2Mock) + every { cert1Mock.subjectAlternativeNames } returns sanEntries1 + every { cert2Mock.subjectAlternativeNames } returns sanEntries2 + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertTrue(result, "Should find match in second certificate") + } + + @Test + fun `should handle non-X509 certificates gracefully`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + + val nonX509Cert = mockk() // Not an X509Certificate + every { sslSession.peerCertificates } returns arrayOf(nonX509Cert, x509Certificate) + + val sanEntries = listOf( + listOf(2, "test_host.example.com") + ) + every { x509Certificate.subjectAlternativeNames } returns sanEntries + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertTrue(result, "Should skip non-X509 certificates and process X509 certificates") + } + + @Test + fun `should reproduce the underscore bug scenario`() { + // Given - This test reproduces the exact scenario from the bug report + val problematicHostname = "coder_instance.dev.company.com" + verifier = CoderHostnameVerifier(problematicHostname) + + // Mock a certificate that would be valid but contains underscore in SAN + val sanEntries = listOf( + listOf(2, "dev.company.com"), + listOf(2, "coder_instance.dev.company.com"), // This contains underscore + listOf(2, "*.dev.company.com") + ) + + every { x509Certificate.subjectAlternativeNames } returns sanEntries + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + + // When + val result = verifier.verify("dev.company.com", sslSession) + + // Then + assertTrue(result, "Should successfully verify hostname with underscore in SAN") + + // Additional verification that the problematic hostname would be found + val foundHostnames = mutableListOf() + sanEntries.forEach { entry -> + if (entry[0] == 2) { // DNS name type + foundHostnames.add(entry[1] as String) + } + } + + assertTrue( + foundHostnames.any { it.equals(problematicHostname, ignoreCase = true) }, + "Certificate should contain the problematic hostname with underscore" + ) + } + + @Test + fun `should handle edge case with empty SAN list`() { + // Given + verifier = CoderHostnameVerifier("test_host.example.com") + + every { sslSession.peerCertificates } returns arrayOf(x509Certificate) + every { x509Certificate.subjectAlternativeNames } returns emptyList() + + // When + val result = verifier.verify("example.com", sslSession) + + // Then + assertFalse(result, "Should return false when SAN list is empty") + } +} \ No newline at end of file