From 78934325167c808139fdadfa2a895550a5e3edba Mon Sep 17 00:00:00 2001 From: Andreas Kostuch Date: Fri, 29 Nov 2019 15:44:08 +0100 Subject: [PATCH 1/3] Bugfix HTTPS SNI and IP Address Motivation: Solving the SNI Bug Modifications: Added an internal extension on String for checking if the hostname is an IP Address -- see the private extension on SNI. Additionally using the IPv4Address and IPv6Address Function from Network above 10.14 as protecting with #availabe. Adding the test for HTTPS and IP in as hostname Result: We get results with an IP as Hostname --- Sources/AsyncHTTPClient/HTTPClient.swift | 14 +++---- Sources/AsyncHTTPClient/Utils.swift | 37 +++++++++++++++++++ .../HTTPClientTests+XCTest.swift | 1 + .../HTTPClientTests.swift | 13 +++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index b78a4195b..ca2d9af7e 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -275,7 +275,7 @@ public class HTTPClient { let taskHandler = TaskHandler(task: task, delegate: delegate, redirectHandler: redirectHandler, ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown) return channel.pipeline.addHandler(taskHandler) } - } + } if let timeout = self.resolve(timeout: self.configuration.timeout.connect, deadline: deadline) { bootstrap = bootstrap.connectTimeout(timeout) @@ -285,11 +285,11 @@ public class HTTPClient { bootstrap.connect(host: address.host, port: address.port) .map { channel in task.setChannel(channel) - } - .flatMap { channel in - channel.writeAndFlush(request) - } - .cascadeFailure(to: task.promise) + } + .flatMap { channel in + channel.writeAndFlush(request) + } + .cascadeFailure(to: task.promise) return task } @@ -501,7 +501,7 @@ private extension ChannelPipeline { do { let tlsConfiguration = tlsConfiguration ?? TLSConfiguration.forClient() let context = try NIOSSLContext(configuration: tlsConfiguration) - return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: request.host), + return self.addHandler(try NIOSSLClientHandler(context: context, serverHostname: request.host.isIPAddress ? nil : request.host), position: .first) } catch { return self.eventLoop.makeFailedFuture(error) diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index 7fbea9c38..ec4fd204c 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -14,6 +14,10 @@ import NIO import NIOHTTP1 +#if os(Linux) +#else +import Network +#endif public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate { public typealias Response = Void @@ -32,3 +36,36 @@ public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate { return () } } + +#if os(Linux) +internal extension String { + var isIPAddress: Bool { + var ipv4Addr = in_addr() + var ipv6Addr = in6_addr() + + return self.withCString { ptr in + return inet_pton(AF_INET, ptr, &ipv4Addr) == 1 || + inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 + } + } +} +#else +internal extension String { + var isIPAddress: Bool { + if #available(OSX 10.14, *) { + if IPv4Address(self) != nil || IPv6Address(self) != nil { + return true + } + } else { + var ipv4Addr = in_addr() + var ipv6Addr = in6_addr() + + return self.withCString { ptr in + return inet_pton(AF_INET, ptr, &ipv4Addr) == 1 || + inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 + } + } + return false + } +} +#endif diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift index a2189df40..d2ffdd688 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests+XCTest.swift @@ -32,6 +32,7 @@ extension HTTPClientTests { ("testGetWithDifferentEventLoopBackpressure", testGetWithDifferentEventLoopBackpressure), ("testPost", testPost), ("testGetHttps", testGetHttps), + ("testGetHttpsWithIP", testGetHttpsWithIP), ("testPostHttps", testPostHttps), ("testHttpRedirect", testHttpRedirect), ("testHttpHostRedirect", testHttpHostRedirect), diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index b7e9aa382..c93d8045e 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -122,6 +122,19 @@ class HTTPClientTests: XCTestCase { XCTAssertEqual(.ok, response.status) } + func testGetHttpsWithIP() throws { + let httpBin = HTTPBin(ssl: true) + let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, + configuration: HTTPClient.Configuration(certificateVerification: .none)) + defer { + XCTAssertNoThrow(try httpClient.syncShutdown()) + XCTAssertNoThrow(try httpBin.shutdown()) + } + + let response = try httpClient.get(url: "https://127.0.0.1:\(httpBin.port)/get").wait() + XCTAssertEqual(.ok, response.status) + } + func testPostHttps() throws { let httpBin = HTTPBin(ssl: true) let httpClient = HTTPClient(eventLoopGroupProvider: .createNew, From 0b77f944a390b5791ea7fe59534f30b06108606e Mon Sep 17 00:00:00 2001 From: Andreas Kostuch Date: Fri, 29 Nov 2019 21:02:31 +0100 Subject: [PATCH 2/3] swiftformat, rework isIPAddress and import regarding pull request comments --- Sources/AsyncHTTPClient/HTTPClient.swift | 13 ++-- Sources/AsyncHTTPClient/Utils.swift | 59 ++++++++----------- .../HTTPClientTests.swift | 2 +- 3 files changed, 31 insertions(+), 43 deletions(-) diff --git a/Sources/AsyncHTTPClient/HTTPClient.swift b/Sources/AsyncHTTPClient/HTTPClient.swift index ca2d9af7e..9c4cb2b20 100644 --- a/Sources/AsyncHTTPClient/HTTPClient.swift +++ b/Sources/AsyncHTTPClient/HTTPClient.swift @@ -275,7 +275,7 @@ public class HTTPClient { let taskHandler = TaskHandler(task: task, delegate: delegate, redirectHandler: redirectHandler, ignoreUncleanSSLShutdown: self.configuration.ignoreUncleanSSLShutdown) return channel.pipeline.addHandler(taskHandler) } - } + } if let timeout = self.resolve(timeout: self.configuration.timeout.connect, deadline: deadline) { bootstrap = bootstrap.connectTimeout(timeout) @@ -285,12 +285,11 @@ public class HTTPClient { bootstrap.connect(host: address.host, port: address.port) .map { channel in task.setChannel(channel) - } - .flatMap { channel in - channel.writeAndFlush(request) - } - .cascadeFailure(to: task.promise) - + } + .flatMap { channel in + channel.writeAndFlush(request) + } + .cascadeFailure(to: task.promise) return task } diff --git a/Sources/AsyncHTTPClient/Utils.swift b/Sources/AsyncHTTPClient/Utils.swift index ec4fd204c..398720b43 100644 --- a/Sources/AsyncHTTPClient/Utils.swift +++ b/Sources/AsyncHTTPClient/Utils.swift @@ -14,9 +14,31 @@ import NIO import NIOHTTP1 -#if os(Linux) + +#if canImport(Network) + import Network + + internal extension String { + var isIPAddress: Bool { + if IPv4Address(self) != nil || IPv6Address(self) != nil { + return true + } + return false + } + } + #else -import Network + internal extension String { + var isIPAddress: Bool { + var ipv4Addr = in_addr() + var ipv6Addr = in6_addr() + + return self.withCString { ptr in + inet_pton(AF_INET, ptr, &ipv4Addr) == 1 || + inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 + } + } + } #endif public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate { @@ -36,36 +58,3 @@ public final class HTTPClientCopyingDelegate: HTTPClientResponseDelegate { return () } } - -#if os(Linux) -internal extension String { - var isIPAddress: Bool { - var ipv4Addr = in_addr() - var ipv6Addr = in6_addr() - - return self.withCString { ptr in - return inet_pton(AF_INET, ptr, &ipv4Addr) == 1 || - inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 - } - } -} -#else -internal extension String { - var isIPAddress: Bool { - if #available(OSX 10.14, *) { - if IPv4Address(self) != nil || IPv6Address(self) != nil { - return true - } - } else { - var ipv4Addr = in_addr() - var ipv6Addr = in6_addr() - - return self.withCString { ptr in - return inet_pton(AF_INET, ptr, &ipv4Addr) == 1 || - inet_pton(AF_INET6, ptr, &ipv6Addr) == 1 - } - } - return false - } -} -#endif diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index c93d8045e..ce21a8b77 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -844,7 +844,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(try web.writeOutbound(.head(.init(version: .init(major: 1, minor: 0), status: .ok, headers: HTTPHeaders([("connection", - i % 2 == 0 ? "close" : "keep-alive")]))))) + i % 2 == 0 ? "close" : "keep-alive")]))))) XCTAssertNoThrow(try web.writeOutbound(.end(nil))) var response: HTTPClient.Response? From ef404bfa20ca286e91da9a9db0b4353a07df0216 Mon Sep 17 00:00:00 2001 From: Johannes Weiss Date: Mon, 16 Dec 2019 22:12:35 +0000 Subject: [PATCH 3/3] Update HTTPClientTests.swift --- Tests/AsyncHTTPClientTests/HTTPClientTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift index 935878c1d..33c529330 100644 --- a/Tests/AsyncHTTPClientTests/HTTPClientTests.swift +++ b/Tests/AsyncHTTPClientTests/HTTPClientTests.swift @@ -904,7 +904,7 @@ class HTTPClientTests: XCTestCase { XCTAssertNoThrow(try web.writeOutbound(.head(.init(version: .init(major: 1, minor: 0), status: .ok, headers: HTTPHeaders([("connection", - i % 2 == 0 ? "close" : "keep-alive")]))))) + i % 2 == 0 ? "close" : "keep-alive")]))))) XCTAssertNoThrow(try web.writeOutbound(.end(nil))) var response: HTTPClient.Response?