Skip to content

Conversation

@aryan-25
Copy link
Collaborator

@aryan-25 aryan-25 commented Dec 8, 2025

Motivation:

NIOSSLServerHandler allows a custom verification callback to be used for client certificate verification. This callback returns the peer's validated chain of trust, which NIOSSL then surfaces in the channel handler. Having control over certificate verification and accessing the peer's validated certificate chain can be useful for mTLS implementations.

NIOHTTPServer currently does not support propagating a custom verification callback to NIOSSLServerHandler. This change adds support for that and also surfaces the peer's validated certificate chain which becomes available as a result.

Modifications:

  • Updated the server configuration to also include a customCertificateVerificationCallback argument for the mTLS and reloadingMTLS cases of TransportSecurity.

    • Note: There are certain caveats here which I'd like to hear opinions about. We now expose NIOSSL types in the server configuration type (the arguments and return type of the callback). I'm also not sure whether custom client certificate verification is a requirement we want to impose for other server implementations, so for now, I have made the configuration type NIOHTTPServer specific and renamed HTTPServerConfiguration to NIOHTTPServerConfiguration.
  • Updated the serveSecureUpgrade method in NIOHTTPServer to:

    • Propagate the custom verification callback into the underlying NIOSSLServerHandler;
    • Extract the peer certificate chain EventLoopFuture per connection, and;
    • Expose that future in a new type in NIOHTTPServer named ConnectionContext.
      • ConnectionContext is accessible from a task-local property. Users can await the result of the promise from their route handlers by calling NIOHTTPServer. connectionContext.peerCertificateChain().
      • The name of this type, its properties, the task-local approach, etc. are all very much open for discussion.
  • Added end-to-end tests for this functionality for both HTTP/1.1 and HTTP2.

  • Other changes:

    • Added NIOSSL+X509 containing some convenience conversions between NIOSSL and X509 types.
    • Added documentation to the TransportSecurity methods.

Result:

Users can now specify a custom verification callback and access the peer's validated certificate chain from the request handler.

…allback

Motivation:
`NIOSSL` supports specifying a custom verification callback for client certificate verification. When such a callback is provided to `NIOSSLServerHandler`, it will use the callback to _replace_ its default client certificate verification logic, and surface the peer's validated chain of trust (derived and returned from the custom callback). This can be useful for mTLS implementations.

`NIOHTTPServer` currently does not support propagating a custom verification callback to `NIOSSLServerHandler`. This change adds support for that and also surfaces the peer's validated certificate chain that then becomes available as a result.

Modifications:
- Updated `HTTPServerConfiguration` to also include a `customCertificateVerificationCallback` argument for the `mTLS` and `reloadingMTLS` cases of `TransportSecurity`.
  - Note: This exposes `NIO` types at the `HTTPServerConfiguration` level (in particular, the arguments of the callback are [`NIOSSLCertificate`] and an `EventLoopPromise` that must be fulfilled within the callback). We currently do not have a `NIOHTTPServer`-specific configuration, so let us discuss in the comments what the best way to allow users to specify a custom verification callback would be.
- Updated the `serveSecureUpgrade` method in `NIOHTTPServer` to (1) propagate the custom verification callback into the underlying `NIOSSLServerHandler`, (2) extract the peer certificate chain `EventLoopFuture` per connection, and expose that future in a new type in `NIOHTTPServer` named `Context`.
  - The `Context` type is accessed from a task-local property `context`. Users can await the result of the promise from their route handlers by calling `NIOHTTPServer.context.peerCertificateChain()`.
  - The name of this type, its properties, the task-local approach, etc. are all very much open for discussion.
- Added end-to-end tests for this functionality for both HTTP/1.1 and HTTP2.
- Other changes:
  - Added `NIOSSL+X509` containing some convenience conversions between `NIOSSL` and `X509` types.
  - Added documentation to the `TransportSecurity` methods as they were missing.

Result:
Users can now specify a custom verification callback and access the peer's validated certificate chain from the request handler.
Comment on lines 15 to 16
public import NIOCore
public import NIOSSL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to discuss the larger strategy here. One goal of this package was to not expose any NIO types at all. So that in the future we could transparently change the underlying networking stack if we wanted to. @gjcairo we need to discuss this more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the only reason we need to expose NIO types there is because of the verification callback type, whose signature is: ([NIOSSLCertificate], EventLoopPromise<NIOSSLVerificationResultWithMetadata>) -> Void.

We could just expose a callback of type ([X509.Certificate]) async throws -> VerificationResult (where VerificationResult is just a wrapper over NIOSSLVerificationResultWithMetadata) and then bridge to NIOSSL like below:

NIOSSLServerHandler(
    ...
    customVerificationCallbackWithMetadata: { certificates, promise in
        promise.completeWithTask {
            // Convert input [NIOSSLCertificate] to [X509.Certificate]
            let x509Certs = try certificates.map { try Certificate($0) }

            let callbackResult = try await customVerificationCallback(x509Certs)

            switch callbackResult {
            case .certificateVerified(let verificationMetadata):
                guard let peerChain = verificationMetadata.validatedCertificateChain else {
                    return .certificateVerified(.init(nil))
                }

                // Convert the result back into [NIOSSLCertificate]
                let nioSSLCerts = try peerChain.map { try NIOSSLCertificate($0) }
                return .certificateVerified(.init(.init(nioSSLCerts)))

            case .failed:
                return .failed
            }
        }
    }
)

The tradeoff we make here is the conversion cost from [NIOSSLCertificate] to [X509.Certificate] and vice-versa.

But a broader question: should we have an abstract server configuration type? If so, having the custom certificate verification callback as part of the abstract configuration may not be the right choice because that would require different server implementations to support it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One goal of this package was to not expose any NIO types at all.

This implementation is a NIO implementation. The abstract APIs don't use any configuration types right now, which means we don't currently need an HTTPServerConfiguration type that's completely independent of the specific implementation details. For this new callback specifically, I am not convinced it would even make sense to expose it in a more generic (i.e. non-NIO-specific) context, since each implementation might handle this differently, not with a closure of this exact shape.

We could have an HTTPServerConfiguration with an implementationSpecific property, similar to what I'd done for the RequestContext originally, and put this callback there if you think that's better. But that brings me to the second point...

So that in the future we could transparently change the underlying networking stack if we wanted to.

I think we need to decide then whether this package should be a NIO implementation of the server transport, or a... default (for lack of a better word) implementation of a server? If we're saying we intend to, in the future, reuse the same package but change the underlying implementation, then we need to find a way to do so in a way that can be evolved without breaking people. I'm not convinced the future NIO replacement will need this callback for example, or maybe something similar but not with this exact shape. Other aspects of the configuration may be available in the new implementation that aren't available in NIO, or viceversa. I think some changes will for sure be necessary at the config level if we change the underlying implementation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is a NIO implementation. The abstract APIs don't use any configuration types right now, which means we don't currently need an HTTPServerConfiguration type that's completely independent of the specific implementation details. For this new callback specifically, I am not convinced it would even make sense to expose it in a more generic (i.e. non-NIO-specific) context, since each implementation might handle this differently, not with a closure of this exact shape.

I think we are mixing two things up here. We always had the goal of the HTTPServer implementation to also be decoupled from the NIO. The fact that we also want to have a high level HTTP server protocol that is decoupled as well is related but orthogonal. The reason why we want the concrete HTTP server implementation to also be decoupled is because we expect more packages to build on top of it. If we require NIO in our API here then these packages also need to use NIO. This was one goal when the SSWG started this repo and I personally don't think we should drop it.

I think we need to decide then whether this package should be a NIO implementation of the server transport, or a... default (for lack of a better word) implementation of a server?

In my opinion and the initial goal of this package was that this is a default implementation of a server and it uses NIO to start with.

Yeah, the only reason we need to expose NIO types there is because of the verification callback type, whose signature is: ([NIOSSLCertificate], EventLoopPromise) -> Void.
We could just expose a callback of type ([X509.Certificate]) async throws -> VerificationResult (where VerificationResult is just a wrapper over NIOSSLVerificationResultWithMetadata) and then bridge to NIOSSL like below:

I would really prefer that approach. The wrapping isn't too bad and it keeps us flexible. We can certainly introduce changes on the NIO side to reduce the perf overhead if we wanted to but going this way keeps us flexible.

But a broader question: should we have an abstract server configuration type? If so, having the custom certificate verification callback as part of the abstract configuration may not be the right choice because that would require different server implementations to support it

I don't think we should have an abstract configuration type. At least yet. Having said that the API you proposed

([X509.Certificate]) async throws -> VerificationResult

Looks like a good API that can stand the battle of time since it uses just ecosystem wide currency types i.e. Certificate. The only question that I have is if VerificationResult should be a type in swift-certificates?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We always had the goal of the HTTPServer implementation to also be decoupled from the NIO. The fact that we also want to have a high level HTTP server protocol that is decoupled as well is related but orthogonal.

I actually didn't realize this, but if that's the case, then I think this PR should be pre-empted by another PR that splits the targets, and puts the NIO-dependent code into a separate one from the "generic" one. That way, the build system ensures we don't depend on things incorrectly, rather than something that needs to be caught by human review.

Copy link
Collaborator Author

@aryan-25 aryan-25 Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go down the wrapping route:

The only question that I have is if VerificationResult should be a type in swift-certificates?

Just remembered that a CertificateValidationResult type was recently introduced in swift-certificates (see PR apple/swift-certificates#272). This type is returned by Verifier's validate method.

If user's don't use swift-certificates' Verifier within the callback, then CertificateValidationResult may not be an ergonomic choice:

  • The associated value of the validCertificate case is ValidatedCertificateChain, which intentionally does not have a nice public initializer (to discourage users from initializing this type with certificates that haven't been validated).
  • Also, the other couldNotValidate case has PolicyFailure as an associated value, which aside from being tied with the Verifier and VerifierPolicy relation, can only be initialized with a UnverifiedCertificateChain---UnverifiedCertificateChain is the input to VerifierPolicy, and has no public initializer (again, this makes sense for swift-certificates because users shouldn't be able to use VerifierPolicys in isolation without Verifier).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @gjcairo:

The idea is for this repo to depend on swift-http-api-proposal and remove all the abstract pieces once both repos are public. Leaving the abstract pieces aside, I don't think there's any code that doesn't depend on NIO.

From @FranzBusch:

I think we are mixing two things up here. We always had the goal of the HTTPServer implementation to also be decoupled from the NIO. The fact that we also want to have a high level HTTP server protocol that is decoupled as well is related but orthogonal.

My read of this is that Franz implied that this repo should have two things, which are merged right now: a NIO-less HTTP server "implementation", and a NIO-specific "transport". If that's the case, let's separate these layers early, as it only gets more difficult as more code is added.

This will also simplify discussing where each configuration should go. Otherwise we might spend time creating various wrappers that aren't needed, if actually we meant to configure the transport (which can totally use NIO types in configuration).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should drop this requirement either, and we're not using NIO in our APIs broadly. It's just the configuration that's required to create an instance of a NIOHTTPServer exposes a NIO type - which doesn't seem like such a far fetched thing in my eyes.

I think we should avoid it in the configuration as well. This will make the entire package a lot more future proof. Especially since we do have the types already and X509.Certificate is a lot nicer to use.

If we do that though, why would we keep the config as a NIOHTTPServer-only type instead of in the abstract package?

I think we should bikeshed the type name a bit more. Maybe NIOHTTPServer is not the right thing after all.

Just remembered that a CertificateValidationResult type was recently introduced in swift-certificates (see PR apple/swift-certificates#272). This type is returned by Verifier's validate method.

I think this type is good. Regarding your two comments:

  1. I think the uncheckedCertificateChain is great. It is publicly available but nudges the user into the right direction.
  2. I think we should discuss making the init on UnverifiedCertificateChain public.

In the end, we need something to provide the currency types in the ecosystem. You can see that Go does the same. If these types aren't in swift-certificates then we are going to need them to be somewhere different.

Copy link
Collaborator

@gjcairo gjcairo Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My read of this is that Franz implied that this repo should have two things, which are merged right now: a NIO-less HTTP server "implementation", and a NIO-specific "transport".

There are no two layers here. We discussed this before: having a lower level transport protocol is kinda pointless, there's really nothing separating each layer. If we would move the NIO implementation into a transport layer, the "NIO-less server implementation" would literally be just forwarding calls to the NIO layer.

What we might want though is what I suggested in one of my previous comments: a layered configuration, with some implementation-specific stuff.


I think we should bikeshed the type name a bit more. Maybe NIOHTTPServer is not the right thing after all.

This kinda missed the point of my question, which was: if we're using only swift-certificate types (or more broadly, currency types), then should the config type just live alongside the abstract pieces?

Regarding naming, I'm not sure. It is a NIO implementation. gRPC or OpenAPI transports do disclose the underlying implementation in their names (grpc-swift-nio-transport, swift-openapi-urlsession).

You shouldn't touch this type as a library developer, you'd just use the abstract pieces (e.g. HTTPServer). I understand your point about evolving this into an implementation that doesn't use NIO underneath, but I'm not convinced that requires changing the underlying implementation of this module transparently. Doing so, IMO, would require having a transport protocol layer which we'd discussed seems superfluous.

I don't see what the issue would be with having a new package or module that provides a different implementation not using NIO in the future. Library authors would not require any changes in their packages since they'd be using the abstract pieces anyways. For users using the NIO implementation directly, I don't think it's wrong they'd have to manually switch to the new thing. The only other scenario I can envision is libraries like Vapor or HB bootstrapping a default implementation as a utility, but then they could simply change this bootstrap and everyone using NIO will start using whatever replaces it.


Regarding CertificateValidationResult:

I think this type is good.

I was just chatting with Aryan and Raphael offline, who worked on adding the verifier APIs in gRPC and Vapor, and we're not sure this type is actually good. It is if you're using the Verifier in swift-certificates. But you could be using some custom verifier or Security.framework, and in this case, the verification results may not map easily to CertificateValidationResult cases. For example, PolicyFailures are a swift-certificates thing, and nothing from Security.framework can be mapped easily into that.

To be clear, I'm not saying a validation result type shouldn't exist. I'm just saying that I am not sure reusing this type works outside the context of swift-certificates. If we want to support other use cases (I assume that particularly Security.framework should be supported) then we probably need a new type without the associated values.

Tangentially related question: the client doesn't currently provide functionality like this, does it? Should it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This kinda missed the point of my question, which was: if we're using only swift-certificate types (or more broadly, currency types), then should the config type just live alongside the abstract pieces?

Not yet would be my argument. Hopefully in the future yes. I think there is a lot of stuff to figure out.

Regarding naming, I'm not sure. It is a NIO implementation. gRPC or OpenAPI transports do disclose the underlying implementation in their names (grpc-swift-nio-transport, swift-openapi-urlsession).

You shouldn't touch this type as a library developer, you'd just use the abstract pieces (e.g. HTTPServer). I understand your point about evolving this into an implementation that doesn't use NIO underneath, but I'm not convinced that requires changing the underlying implementation of this module transparently. Doing so, IMO, would require having a transport protocol layer which we'd discussed seems superfluous.

I don't see what the issue would be with having a new package or module that provides a different implementation not using NIO in the future. Library authors would not require any changes in their packages since they'd be using the abstract pieces anyways. For users using the NIO implementation directly, I don't think it's wrong they'd have to manually switch to the new thing. The only other scenario I can envision is libraries like Vapor or HB bootstrapping a default implementation as a utility, but then they could simply change this bootstrap and everyone using NIO will start using whatever replaces it.

I think you bring up good points and I am definitely okay with keeping the name NIOHTTPServer for now. I think we still wanna discuss which types we use in the public API. Even if it is NIO based I would still strongly argue that we should use swift-certifactes and async so that the APIs are nice to use.

All kinds of users will hold this type both library and applications authors and I would like them to not touch NIO bits for the basic things which I consider mTLS to be.

To be clear, I'm not saying a validation result type shouldn't exist. I'm just saying that I am not sure reusing this type works outside the context of swift-certificates. If we want to support other use cases (I assume that particularly Security.framework should be supported) then we probably need a new type without the associated values.

That's fair. I am happy if we introduce a new type as in the scope of this project but I think certificates should be expressed as X509.Certificate.

Tangentially related question: the client doesn't currently provide functionality like this, does it? Should it?

This is one open problem of the client. We probably want to spell tls/mtls settings on a per-request basis which means we gotta solve the TLS currency type problem.

Copy link
Contributor

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss the strategy here. We shouldn't expose NIO types since this is going to couple us to NIO where as right now NIO is just an implementation detail.

import SwiftASN1
import X509

/// Some convenience helpers for converting between NIOSSL and X509 certificate and private key types.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth filing an issue on swift-nio-ssl to provide this, and link the issue from here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've filed the issue here: apple/swift-nio-ssl#568

Copy link
Contributor

@FranzBusch FranzBusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great. A few minor comments

public enum CertificateVerificationResult: Sendable, Hashable {
/// An error representing certificate verification failure.
public struct VerificationError: Swift.Error, Hashable {
let description: String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be public


/// Creates a verification error with a description of the failure.
/// - Parameter description: A description of why certificate verification failed.
public init(description: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Maybe reason is better for this?

/// to provide access to it in the request handler through ``NIOHTTPServer/ConnectionContext/peerCertificateChain``,
/// accessed via the task-local ``NIOHTTPServer/connectionContext`` property. Otherwise, return
/// ``CertificateVerificationResult/failed(_:)`` if verification fails.
public typealias CustomCertificateVerificationCallback =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need this type alias? We normally try not to use type aliases because they are imposing some problems for the developer experience.

@aryan-25
Copy link
Collaborator Author

I've added support for also allowing optional client certificate verification, i.e. clients can connect without presenting certificates, but if they do present certificates, they are validated like normal (see the NIOSSL PR for context: apple/swift-nio-ssl#565).

This required adding a new type named CertificateVerificationMode. It essentially mirrors NIOSSL's CertificateVerification type, but doesn't have the none and fullVerification case as they aren't relevant from a server context.


let backing: Backing

public static let plaintext: Self = Self(backing: .plaintext)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a doc comment to this?

//
//===----------------------------------------------------------------------===//

public import NIOCertificateReloading
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I just realized this is another NIO type. Let's open an issue we should really be able to spell this without NIO types if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where things start getting a bit tricky though IMO. Unless certificate reloads also become part of the currency TLS types I think it will be hard to expose this in an abstract way. Anyways, we should have this conversation on the issue.


/// Task-local storage for connection-specific information accessible from request handlers.
///
/// Use this to access data such as the peer's validated certificate chain.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this line and just link to the docs for ConnectionContext instead (you can use - SeeAlso:

/// Connection-specific information available during request handling.
///
/// Provides access to data such as the peer's validated certificate chain.
public struct ConnectionContext: Sendable {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this type to an extension on its own file?

channel: http2StreamChannel,
handler: handler
)
Self.$connectionContext.withValue(ConnectionContext(chainFuture)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should move this outside of the for...await loop - I don't think new streams would trigger the chain to change, right?

return .certificateVerified(.init(.init(nioSSLCerts)))

case .failed(let error):
self.logger.error("Custom certificate verification failed: \(error)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add this error as metadata instead of interpolating it?


// MARK: X509 to NIOSSL

@available(macOS 26.0, iOS 26.0, watchOS 26.0, tvOS 26.0, visionOS 26.0, *)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit but I don't think we need these availability guards.

//
//===----------------------------------------------------------------------===//

public import NIOCertificateReloading
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where things start getting a bit tricky though IMO. Unless certificate reloads also become part of the currency TLS types I think it will be hard to expose this in an abstract way. Anyways, we should have this conversation on the issue.

Comment on lines 155 to 158
/// - customCertificateVerificationCallback: If specified, this callback *overrides* the default NIOSSL client
/// certificate verification logic. Refer to the documentation for this argument in
/// ``mTLS(certificateChain:privateKey:trustRoots:certificateVerification:customCertificateVerificationCallback:)``
/// for more details.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we copy the docs here instead of referring to a different method's docs? I think if you're using this from an IDE it's a bit annoying to not be able to see the corresponding docs in place.

try await responseBodySender.produceAndConclude { responseBodyWriter in
var responseBodyWriter = responseBodyWriter
try await responseBodyWriter.write([1, 2].span)
return [.trailer: "test_trailer"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Self.trailer here?

@aryan-25 aryan-25 merged commit eea3f4e into swift-server:main Dec 16, 2025
4 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants