From fa59e890867d5999a0b801304f8d25a6bea09b8e Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Thu, 17 Dec 2020 18:36:44 +0000 Subject: [PATCH 1/6] Add HSTSRedirection Middleware --- README.md | 11 ++++ .../HSTSRedirectMiddleware.swift | 23 +++++++++ .../SecurityHeadersFactory.swift | 1 + .../RedirectionTest.swift | 50 +++++++++++++++++++ 4 files changed, 85 insertions(+) create mode 100644 Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift create mode 100644 Tests/VaporSecurityHeadersTests/RedirectionTest.swift diff --git a/README.md b/README.md index 6120819..1905a1d 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ Easily add headers to all your responses for improving the security of your site * X-Frame-Options * X-Content-Type-Options * Strict-Transport-Security (HSTS) +* Redirect HTTP to HTTPS * Server * Referrer Policy @@ -436,6 +437,16 @@ let securityHeadersFactory = SecurityHeadersFactory().with(strictTransportSecuri strict-transport-security: max-age=31536000; includeSubDomains; preload ``` +## Redirect HTTP to HTTPS + +If Strict-Transport-Security is not enough to accomplish a forwarding connection to HTTPS from the browsers, you can opt to add an additional middleware who provides this redirection if clients try to reach your site with an HTTP connection. + +To use the HSTS Redirect Middleware, you can add the following line in your middlewares, preferably before securityHeadersFactory.build(): + +```swift +app.middleware.use(securityHeadersFactory.redirectMiddleware) +``` + ## Server The Server header is usually hidden from responses in order to not give away what type of server you are running and what version you are using. This is to stop attackers from scanning your site and using known vulnerabilities against it easily. By default Vapor does not show the server header in responses for this reason. diff --git a/Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift b/Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift new file mode 100644 index 0000000..d683c85 --- /dev/null +++ b/Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift @@ -0,0 +1,23 @@ +import Vapor + +public class HSTSRedirectMiddleware: Middleware { + + public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture { + if request.application.environment == .development { + return next.respond(to: request) + } + + let proto = request.headers.first(name: "X-Forwarded-Proto") + ?? request.url.scheme + ?? "http" + + guard proto == "https" else { + guard let host = request.headers.first(name: .host) else { + return request.eventLoop.makeFailedFuture(Abort(.badRequest)) + } + let httpsURL = "https://" + host + "\(request.url)" + return request.redirect(to: "\(httpsURL)", type: .permanent).encodeResponse(for: request) + } + return next.respond(to: request) + } +} diff --git a/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift b/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift index 9d883b8..bf00c9d 100644 --- a/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift +++ b/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift @@ -9,6 +9,7 @@ public class SecurityHeadersFactory { var server: ServerConfiguration? var referrerPolicy: ReferrerPolicyConfiguration? var contentSecurityPolicyReportOnly: ContentSecurityPolicyReportOnlyConfiguration? + public var redirectMiddleware = HSTSRedirectMiddleware() public init() {} diff --git a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift new file mode 100644 index 0000000..d1541ce --- /dev/null +++ b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift @@ -0,0 +1,50 @@ +import XCTest + +@testable import Vapor + +import VaporSecurityHeaders + +class RedirectionTest: XCTestCase { + + // MARK: - Properties + + private var application: Application! + private var eventLoopGroup: EventLoopGroup! + private var request: Request! + + override func setUp() { + eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) + application = Application(.testing, .shared(eventLoopGroup)) + } + + override func tearDownWithError() throws { + application.shutdown() + try eventLoopGroup.syncShutdownGracefully() + } + + func testRedirectionMiddleware() throws { + let expectedRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 301, reasonPhrase: "Moved permanently") + let expectedNoRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") + let requestURL = Request(application: application, method: .GET, url: URI(string: "/testRedirection"), on: eventLoopGroup.next()) + requestURL.headers.add(name: .host, value: "localhost:8080") + let responseRedirected = try makeTestResponse(for: requestURL, withRedirection: true) + let response = try makeTestResponse(for: requestURL, withRedirection: false) + XCTAssertEqual(expectedRedirectStatus, responseRedirected.status) + XCTAssertEqual(expectedNoRedirectStatus, response.status) + } + + private func makeTestResponse(for request: Request, withRedirection: Bool, routeHandler: ((Request) throws -> String)? = nil) throws -> Response { + + application.middleware = Middlewares() + + if withRedirection == true { + application.middleware.use(SecurityHeadersFactory().redirectMiddleware) + } + + application.routes.get("testRedirection") { req in + return "TESTREDIRECTION" + } + + return try application.responder.respond(to: request).wait() + } +} From 56341368333c679fea4d31006c36a85949fd119f Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Sat, 19 Dec 2020 22:58:20 +0000 Subject: [PATCH 2/6] Fix route overriding --- .../RedirectionTest.swift | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift index d1541ce..08a097b 100644 --- a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift +++ b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift @@ -15,6 +15,7 @@ class RedirectionTest: XCTestCase { override func setUp() { eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) application = Application(.testing, .shared(eventLoopGroup)) + request = Request(application: application, method: .GET, url: URI(string: "/"), on: eventLoopGroup.next()) } override func tearDownWithError() throws { @@ -25,26 +26,37 @@ class RedirectionTest: XCTestCase { func testRedirectionMiddleware() throws { let expectedRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 301, reasonPhrase: "Moved permanently") let expectedNoRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") - let requestURL = Request(application: application, method: .GET, url: URI(string: "/testRedirection"), on: eventLoopGroup.next()) - requestURL.headers.add(name: .host, value: "localhost:8080") - let responseRedirected = try makeTestResponse(for: requestURL, withRedirection: true) - let response = try makeTestResponse(for: requestURL, withRedirection: false) + request.headers.add(name: .host, value: "localhost:8080") + let responseRedirected = try makeTestResponse(for: request, withRedirection: true) + let response = try makeTestResponse(for: request, withRedirection: false) XCTAssertEqual(expectedRedirectStatus, responseRedirected.status) XCTAssertEqual(expectedNoRedirectStatus, response.status) } private func makeTestResponse(for request: Request, withRedirection: Bool, routeHandler: ((Request) throws -> String)? = nil) throws -> Response { - + application.middleware = Middlewares() if withRedirection == true { application.middleware.use(SecurityHeadersFactory().redirectMiddleware) - } - application.routes.get("testRedirection") { req in - return "TESTREDIRECTION" } - + try routes(application) + return try application.responder.respond(to: request).wait() } + + func routes(_ app: Application) throws { + try app.register(collection: RouteController()) + } + + struct RouteController: RouteCollection { + func boot(routes: RoutesBuilder) throws { + routes.get(use: testing) + } + + func testing(req: Request) throws -> String { + return "Test" + } + } } From 0fa4649c958b882d5305f3c8d9f24d6a6220d681 Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Sun, 20 Dec 2020 12:53:56 +0000 Subject: [PATCH 3/6] Make two separate tests --- .../RedirectionTest.swift | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift index 08a097b..e4bd337 100644 --- a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift +++ b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift @@ -15,34 +15,32 @@ class RedirectionTest: XCTestCase { override func setUp() { eventLoopGroup = MultiThreadedEventLoopGroup(numberOfThreads: 1) application = Application(.testing, .shared(eventLoopGroup)) - request = Request(application: application, method: .GET, url: URI(string: "/"), on: eventLoopGroup.next()) + request = Request(application: application, method: .GET, on: eventLoopGroup.next()) } override func tearDownWithError() throws { application.shutdown() try eventLoopGroup.syncShutdownGracefully() } - - func testRedirectionMiddleware() throws { + + func testWithRedirectionMiddleware() throws { let expectedRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 301, reasonPhrase: "Moved permanently") - let expectedNoRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") request.headers.add(name: .host, value: "localhost:8080") let responseRedirected = try makeTestResponse(for: request, withRedirection: true) - let response = try makeTestResponse(for: request, withRedirection: false) XCTAssertEqual(expectedRedirectStatus, responseRedirected.status) + } + func testWithoutRedirectionMiddleware() throws { + let expectedNoRedirectStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") + request.headers.add(name: .host, value: "localhost:8080") + let response = try makeTestResponse(for: request, withRedirection: false) XCTAssertEqual(expectedNoRedirectStatus, response.status) } - private func makeTestResponse(for request: Request, withRedirection: Bool, routeHandler: ((Request) throws -> String)? = nil) throws -> Response { - application.middleware = Middlewares() - if withRedirection == true { application.middleware.use(SecurityHeadersFactory().redirectMiddleware) - } try routes(application) - return try application.responder.respond(to: request).wait() } @@ -54,7 +52,6 @@ class RedirectionTest: XCTestCase { func boot(routes: RoutesBuilder) throws { routes.get(use: testing) } - func testing(req: Request) throws -> String { return "Test" } From 4bacaa6a78abe895065409d32795640b1e1070b7 Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Mon, 21 Dec 2020 16:45:13 +0000 Subject: [PATCH 4/6] Fix packages installation on CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 700a27f..a51fd1e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,7 +19,7 @@ jobs: - name: Run Bionic Tests run: swift test --enable-test-discovery --enable-code-coverage --sanitize=thread - name: Setup container for codecov upload - run: apt-get update && apt-get install curl + run: apt-get update && apt-get install curl -y - name: Process coverage file run: llvm-cov show .build/x86_64-unknown-linux-gnu/debug/VaporSecurityHeadersPackageTests.xctest -instr-profile=.build/debug/codecov/default.profdata > coverage.txt - name: Upload code coverage From ba0e18cb106173bf76a3199ebb9efa7e9a54b583 Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Tue, 22 Dec 2020 17:40:28 +0000 Subject: [PATCH 5/6] Add missing tests --- .../RedirectionTest.swift | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift index e4bd337..42a0bfc 100644 --- a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift +++ b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift @@ -35,8 +35,35 @@ class RedirectionTest: XCTestCase { let response = try makeTestResponse(for: request, withRedirection: false) XCTAssertEqual(expectedNoRedirectStatus, response.status) } - private func makeTestResponse(for request: Request, withRedirection: Bool, routeHandler: ((Request) throws -> String)? = nil) throws -> Response { + + func testOnDevelopmentEnvironment() throws { + let expectedStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") + request.headers.add(name: .host, value: "localhost:8080") + let response = try makeTestResponse(for: request, withRedirection: true, environment: .development) + XCTAssertEqual(expectedStatus, response.status) + } + + func testWithoutHost() throws { + let expectedOutcome: String = "Abort.400: Bad Request" + do { + _ = try makeTestResponse(for: request, withRedirection: true) + } catch (let error) { + XCTAssertEqual(expectedOutcome, error.localizedDescription) + } + } + + func testWithProtoSet() throws { + let expectedStatus: HTTPStatus = HTTPResponseStatus(statusCode: 200, reasonPhrase: "Ok") + request.headers.add(name: .xForwardedProto, value: "https") + let response = try makeTestResponse(for: request, withRedirection: true) + XCTAssertEqual(expectedStatus, response.status) + } + + private func makeTestResponse(for request: Request, withRedirection: Bool, environment: Environment? = nil) throws -> Response { application.middleware = Middlewares() + if let environment = environment { + application.environment = environment + } if withRedirection == true { application.middleware.use(SecurityHeadersFactory().redirectMiddleware) } From ed920b4a6d53d87960c86c52004be485fb1ff744 Mon Sep 17 00:00:00 2001 From: Mattiav8 <42816898+Mattiav8@users.noreply.github.com> Date: Tue, 5 Jan 2021 18:54:08 +0000 Subject: [PATCH 6/6] Clean code and required changes made --- README.md | 4 ++-- ...RedirectMiddleware.swift => HTTPSRedirectMiddleware.swift} | 4 +++- Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift | 1 - Tests/VaporSecurityHeadersTests/RedirectionTest.swift | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) rename Sources/VaporSecurityHeaders/Configurations/{HSTSRedirectMiddleware.swift => HTTPSRedirectMiddleware.swift} (91%) diff --git a/README.md b/README.md index 1905a1d..e4e26dd 100644 --- a/README.md +++ b/README.md @@ -441,10 +441,10 @@ strict-transport-security: max-age=31536000; includeSubDomains; preload If Strict-Transport-Security is not enough to accomplish a forwarding connection to HTTPS from the browsers, you can opt to add an additional middleware who provides this redirection if clients try to reach your site with an HTTP connection. -To use the HSTS Redirect Middleware, you can add the following line in your middlewares, preferably before securityHeadersFactory.build(): +To use the HTTPS Redirect Middleware, you can add the following line in **configure.swift** to enable the middleware. This must be done before `securityHeadersFactory.build()` to ensure HSTS works: ```swift -app.middleware.use(securityHeadersFactory.redirectMiddleware) +app.middleware.use(HTTPSRedirectMiddleware()) ``` ## Server diff --git a/Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift b/Sources/VaporSecurityHeaders/Configurations/HTTPSRedirectMiddleware.swift similarity index 91% rename from Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift rename to Sources/VaporSecurityHeaders/Configurations/HTTPSRedirectMiddleware.swift index d683c85..8c06047 100644 --- a/Sources/VaporSecurityHeaders/Configurations/HSTSRedirectMiddleware.swift +++ b/Sources/VaporSecurityHeaders/Configurations/HTTPSRedirectMiddleware.swift @@ -1,7 +1,9 @@ import Vapor -public class HSTSRedirectMiddleware: Middleware { +public class HTTPSRedirectMiddleware: Middleware { + public init() {} + public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture { if request.application.environment == .development { return next.respond(to: request) diff --git a/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift b/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift index bf00c9d..9d883b8 100644 --- a/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift +++ b/Sources/VaporSecurityHeaders/SecurityHeadersFactory.swift @@ -9,7 +9,6 @@ public class SecurityHeadersFactory { var server: ServerConfiguration? var referrerPolicy: ReferrerPolicyConfiguration? var contentSecurityPolicyReportOnly: ContentSecurityPolicyReportOnlyConfiguration? - public var redirectMiddleware = HSTSRedirectMiddleware() public init() {} diff --git a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift index 42a0bfc..bb2ad34 100644 --- a/Tests/VaporSecurityHeadersTests/RedirectionTest.swift +++ b/Tests/VaporSecurityHeadersTests/RedirectionTest.swift @@ -65,7 +65,7 @@ class RedirectionTest: XCTestCase { application.environment = environment } if withRedirection == true { - application.middleware.use(SecurityHeadersFactory().redirectMiddleware) + application.middleware.use(HTTPSRedirectMiddleware()) } try routes(application) return try application.responder.respond(to: request).wait()