Make available the version of SSL/TLS protocol used in the connection

[jonjensen]

I have often used the extra response headers to troubleshoot a connection, e.g.:

Client-SSL-Cipher: ECDHE-RSA-AES128-GCM-SHA256
Client-SSL-Socket-Class: IO::Socket::SSL

but I also need to know the TLS version being used (1.2 or 1.3 in this case).

I didn't see a way to tell that, so figured adding the same kind of header for the version # would make sense.

Please let me know if there's already a way, or a better way to do this. Thanks!

[coveralls]

Coverage increased (+0.7%) to 78.125% when pulling 35e8189 on jonjensen:master into 96ba0ba on libwww-perl:master.

[skaji]

It looks like get_sslversion() may return undef. So how about

diff --git lib/LWP/Protocol/https.pm lib/LWP/Protocol/https.pm
index 42451c6..f8fe07e 100644
--- lib/LWP/Protocol/https.pm
+++ lib/LWP/Protocol/https.pm
@@ -129,7 +129,7 @@ sub _get_sock_info
     my $self = shift;
     $self->SUPER::_get_sock_info(@_);
     my($res, $sock) = @_;
-    $sock->can('get_sslversion') and $res->header("Client-SSL-Version" => $sock->get_sslversion);
+    $sock->can('get_sslversion') and $res->header("Client-SSL-Version" => $sock->get_sslversion || "Unknown");
     $res->header("Client-SSL-Cipher" => $sock->get_cipher);
     my $cert = $sock->get_peer_certificate;
     if ($cert) {

[jonjensen]

@skaji @oalders Good point. Perhaps it would be better not to add the header at all when it's unknown, as with Net::SSL? That seems better than the arbitrary "Unknown" to me. E.g. something like:

$sock->can('get_sslversion')
    and my $sslversion = $sock->get_sslversion
    and $res->header("Client-SSL-Version" => $sslversion);

[oalders]

Sure, that sounds good to me. @skaji?

[skaji]

Sounds reasonable.

[jonjensen]

@skaji @oalders Ok, new version pushed. My untested proposal above had a my scope problem, so this is slightly different.

[oalders]

@jonjensen do you think we could add a basic unit test for this? It looks like an object that can('get_sslversion') should mostly do the trick.

[jonjensen]

@oalders When I went to look at this, it seemed reasonable to add these artificial response header tests to the existing live apache.org request tests, rather than mocking a socket class. So I did that to t/apache.t and pushed a new commit.

Maybe it's overengineered or something that really would be better as a unit test rather than an integration test. Let me know what you think.

[oalders]

@jonjensen thanks for this! The live tests can be somewhat brittle, but I think the Apache tests have historically been pretty good, so this should be fine.