reactphp / socket

Async, streaming plaintext TCP/IP and secure TLS socket server and client connections for ReactPHP.
https://reactphp.org/socket/
MIT License
1.21k stars 157 forks source link

How to get peer cert chain from tls handshake phase 'try to enable encryption'? #221

Closed flybyray closed 3 years ago

flybyray commented 4 years ago

How to get SSL context option capture_peer_cert_chain ...

https://github.com/Icinga/icingaweb2-module-x509/blob/3084a2d0aaceb7df668680f19ef9febf1e59fe19/library/X509/Job.php#L52

from a failing "try to enable encryption" - peer certs already presented by tls handshake initiation -

https://github.com/reactphp/socket/blob/f54040f8868431ba7ed7a82c5670a962f1689fb5/src/SecureConnector.php#L63-L66

?

Context: Assume we connect to a server which requires client certificates to establish a connection. We will get peer certificates but fail if the server closes the connection because of missing client certificates.

This issue has some relevance at https://github.com/Icinga/icingaweb2-module-x509/issues/66 . We can fix it by dirty hacking but I just want to ask what the architects of reactphp/socket have in mind how to resolve this with this library.

https://github.com/reactphp/socket/commit/7052fe2026790274aa9f11387075f0860e4583f5 https://github.com/reactphp/socket/commit/57bfe77208de36795f77a41ac4c0707fe84fac23 https://github.com/reactphp/socket/commit/10f0629ec83ea0fa22597f348623f554227e3ca0

Thanks for clearance

clue commented 4 years ago

@flybyray Thanks for reporting, this is an interesting feature request.

To recap the way I understand this: After specifying the capture_peer_cert_chain context option, we should expose the resulting peer_certificate_chain context option even if the connection fails. This is indeed not currently exposed and the underlying connection will be closed immediately before raising an Exception as documented.

I would love to see some input (PRs?) to discuss/suggest some possible APIs :+1:

flybyray commented 4 years ago

Thanks for your response.

For now I have no suggestion and essentially no php experience. I just created a workaround for our problem, by patching the bundlereact and the icingaweb2-x509 module. I raise an customexception which provides the needed data.

I spotted several other mechanisms like events in your library. Maybe some event callback would be possible. It would not introduce exception hierarchies.

lippserd commented 4 years ago

@clue I came up with an approach to solve this issue in Icinga/icingaweb2-module-x509#76. Basically we intercept the connection in a custom connector and listen for the close event:

/**
 * Connector that captures stream context options upon close of the underlying connection
 */
class StreamOptsCaptureConnector implements ConnectorInterface
{
    /** @var array|null */
    protected $capturedStreamOptions;

    /** @var ConnectorInterface */
    protected $connector;

    public function __construct(ConnectorInterface $connector)
    {
        $this->connector = $connector;
    }

    /**
     * @return array
     */
    public function getCapturedStreamOptions()
    {
        return (array) $this->capturedStreamOptions;
    }

    /**
     * @param array $capturedStreamOptions
     *
     * @return $this
     */
    public function setCapturedStreamOptions($capturedStreamOptions)
    {
        $this->capturedStreamOptions = $capturedStreamOptions;

        return $this;
    }

    public function connect($uri)
    {
        return $this->connector->connect($uri)->then(function (ConnectionInterface $conn) {
            $conn->on('close', function () use ($conn) {
                if (is_resource($conn->stream)) {
                    $this->setCapturedStreamOptions(stream_context_get_options($conn->stream));
                }
            });

            return resolve($conn);
        });
    }
}

Example usage:

$connector = new Connector($loop);
$streamCaptureConnector = new StreamOptsCaptureConnector($connector);
$secureConnector = new SecureConnector($streamCaptureConnector, $loop, [
    'verify_peer' => false,
    'verify_peer_name' => false,
    'capture_peer_cert_chain' => true,
    'SNI_enabled' => true,
    'peer_name' => $peerName
]);

$connector->connect($url)->then(
    function (ConnectionInterface $conn) use ($streamCaptureConnector) {
        // Close connection in order to capture stream context options
        $conn->close();

        $capturedStreamOptions = $streamCaptureConnector->getCapturedStreamOptions();

        ...
    },
    function (Exception $exception) use ($streamCaptureConnector) {
        $capturedStreamOptions = $streamCaptureConnector->getCapturedStreamOptions();

        if (isset($capturedStreamOptions['ssl']['peer_certificate_chain'])) {
            // The scanned target presented its certificate chain despite throwing an error
            // This is the case for targets which require client certificates for example
            ...
        }
    }
)->otherwise(function (Exception $e) {
    echo $e->getMessage() . PHP_EOL;
    echo $e->getTraceAsString() . PHP_EOL;
});

Do you see any caveats with this approach?

clue commented 4 years ago

@lippserd Nice solution! Note that the Connection::$stream property is marked as @internal and should ideally not be relied upon. There's been some debate to eventually expose this in a more permanent way (#150 and others), but at the moment there are no plans to remove this property any time soon :+1:

clue commented 3 years ago

Ping @flybyray, can you update the status here, is this still an issue to you or did you mange to resolve this in the meantime? It also looks like this could be related to #252?

flybyray commented 3 years ago

@clue I somehow solved it. I think i did something based on this: https://github.com/reactphp/socket/issues/221#issuecomment-592467920

I think the #252 is something different. But I did not tested it.

clue commented 3 years ago

@flybyray Glad to hear! Give this has been answered, I'm closing this for now. Please come back with more details if this problem persists and we can always reopen this :+1: