From 605f7a4c551908bc482ff1da3a7f90628ebdf983 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Tue, 5 Jul 2022 09:25:11 +0100 Subject: [PATCH] Add defensive lifetime management for security metadata (#152) Motivation: Right now we're playing a little fast and loose with the lifetimes of the sec_protocol_metadata_t. As a practical matter it is highly likely that this is owned (and so kept alive by) the NWConnection, but rather than risk that we should tighten up the lifetime management. Modifications: Use withExtendedLifetime to extend the lifetime. Result: Better lifetime management. --- .../NIOTransportServices/NIOTSConnectionChannel.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Sources/NIOTransportServices/NIOTSConnectionChannel.swift b/Sources/NIOTransportServices/NIOTSConnectionChannel.swift index f6ea129..84a1ceb 100644 --- a/Sources/NIOTransportServices/NIOTSConnectionChannel.swift +++ b/Sources/NIOTransportServices/NIOTSConnectionChannel.swift @@ -795,9 +795,16 @@ extension NIOTSConnectionChannel { if let metadata = self.nwConnection?.metadata(definition: NWProtocolTLS.definition) as? NWProtocolTLS.Metadata { // This is a TLS connection, we may need to fire some other events. - let negotiatedProtocol = sec_protocol_metadata_get_negotiated_protocol(metadata.securityProtocolMetadata).map { - String(cString: $0) + let securityMetadata = metadata.securityProtocolMetadata + + // The pointer returned by `sec_protocol_metadata_get_negotiated_protocol` is presumably owned by it, so we need + // to confirm it's still alive while we copy the data out. + let negotiatedProtocol = withExtendedLifetime(securityMetadata) { + sec_protocol_metadata_get_negotiated_protocol(metadata.securityProtocolMetadata).map { + String(cString: $0) + } } + self.pipeline.fireUserInboundEventTriggered(TLSUserEvent.handshakeCompleted(negotiatedProtocol: negotiatedProtocol)) } }