#47945 Add SSL/TLS version info to the access log
Closed: Fixed None Opened 4 years ago by nhosoi.

Access log reports SSL/TLS info per connection as follows.
[..] conn=11 fd=66 slot=66 SSL connection from 192.168.122.159 to 192.168.122.1
[..] conn=11 SSL 128-bit AES

It should report the protocolVersion, as well.


Description: Added the currently used SSL library version info per
connection to the access log.
Sample output:
SSL
[..] conn=3 fd=64 slot=64 SSL connection from ::1 to ::1
[..] conn=3 TLS1.2 128-bit AES-GCM

startTLS
[..] conn=4 op=0 EXT oid="1.3.6.1.4.1.1466.20037" name="startTLS"
[..] conn=4 op=0 RESULT err=0 tag=120 nentries=0 etime=0
[..] conn=4 TLS1.2 128-bit AES-GCM

I think the code that maps version numbers to strings should be in one place. We already have such a table in ssl.c - NSSVersion_list.

Also, for TLS, it seems that there could be a way to convert from the protocolVersion directly to a TLS version, so that we do not have to maintain (and update with new versions :P) a string to table mapping. I think we may have to for SSL, but maybe not for TLS.

{{{
FPRINTF(stderr,
"strsclnt: SSL version %d.%d using %d-bit %s with %d-bit %s MAC\n",
channel.protocolVersion >> 8, channel.protocolVersion & 0xff,
suite.effectiveKeyBits, suite.symCipherName,
suite.macBits, suite.macAlgorithmName);
}}}

So it appears that protocolVersion is some sort of bitfield that encodes the version.

It is unfortunate that NSS doesn't provide an int-to-string (and vice versa) mapping . . .

Would it also be possible to log what version is used when a connection fails?

[07/Nov/2014:09:55:10 -0500] conn=1 fd=64 slot=64 connection from 127.0.0.1 to 127.0.0.1
[07/Nov/2014:09:55:10 -0500] conn=1 op=0 EXT oid="1.3.6.1.4.1.1466.20037" name="startTLS"
[07/Nov/2014:09:55:10 -0500] conn=1 op=0 RESULT err=0 tag=120 nentries=0 etime=0
[07/Nov/2014:09:55:10 -0500] conn=1 op=-1 fd=64 closed - Cannot communicate securely with peer: no common encryption algorithm(s).

[07/Nov/2014:09:57:47 -0500] conn=3 fd=64 slot=64 SSL connection from 127.0.0.1 to 127.0.0.1
[07/Nov/2014:09:57:47 -0500] conn=3 op=-1 fd=64 closed - Cannot communicate securely with peer: no common encryption algorithm(s).

Would be nice to see what was attempted.

Replying to [comment:3 mreynolds]:

Would it also be possible to log what version is used when a connection fails?

[07/Nov/2014:09:55:10 -0500] conn=1 fd=64 slot=64 connection from 127.0.0.1 to 127.0.0.1
[07/Nov/2014:09:55:10 -0500] conn=1 op=0 EXT oid="1.3.6.1.4.1.1466.20037" name="startTLS"
[07/Nov/2014:09:55:10 -0500] conn=1 op=0 RESULT err=0 tag=120 nentries=0 etime=0
[07/Nov/2014:09:55:10 -0500] conn=1 op=-1 fd=64 closed - Cannot communicate securely with peer: no common encryption algorithm(s).

[07/Nov/2014:09:57:47 -0500] conn=3 fd=64 slot=64 SSL connection from 127.0.0.1 to 127.0.0.1
[07/Nov/2014:09:57:47 -0500] conn=3 op=-1 fd=64 closed - Cannot communicate securely with peer: no common encryption algorithm(s).

Would be nice to see what was attempted.

I also thought it's an excellent idea, but it seems NSS does not return the channel info including the version number if the SSL handshake is not successful... :(

This is the channel info when "TLS error -12190:Peer reports incompatible or unsupported protocol version." is returned. protocolVersion is supposed to be 0x2, 0x300, or above. But no such meaningful value is returned.
{{{
(gdb) p channelInfo
$3 = {length = 80, protocolVersion = 0, cipherSuite = 0, authKeyBits = 0, keaKeyBits = 0, creationTime = 0, lastAccessTime = 0,
expirationTime = 0, sessionIDLength = 0, sessionID = '\000' <repeats 31="" times="">, compressionMethodName = 0x0,
compressionMethod = ssl_compression_null}
}}}

Ok. Please file a bug against NSS for this. In the meantime, we'll just have to do without.

git patch file (master) -- To convert the SSL version number to string, instead of maintaining a mapping table, calculates the number and generates the version string
0001-Ticket-47945-Add-SSL-TLS-version-info-to-the-access-.2.patch

Note: I filed 2 RFEs against NSS.
Bug 1161807 - [RFE] API to convert SSL version number to SSL version string
Bug 1161808 - [RFE] method to get the SSL version info from the client regardless of the handshake result

One more optimization - give the function the ability to write the string to a given buffer + length e.g. slapi_getNSSVersion_str(protocolnum, char *buf, size_t bufsize)

If buf is NULL or bufsize is 0, do the smprintf, otherwise, snprintf(buf, bufsize, ...)

git patch file (master) -- slapi_getSSLVersion_str takes buf to return the SSL version string.
0001-Ticket-47945-Add-SSL-TLS-version-info-to-the-access-.3.patch

Replying to [comment:7 rmeggins]:

One more optimization - give the function the ability to write the string to a given buffer + length e.g. slapi_getNSSVersion_str(protocolnum, char *buf, size_t bufsize)

If buf is NULL or bufsize is 0, do the smprintf, otherwise, snprintf(buf, bufsize, ...)

Thank you, Rich!! Updated the patch.

one small coding problem
{{{
(void) slapi_getSSLVersion_str(enabledNSSVersions.max, emax, sizeof(emin));
}}}
should be sizeof(emax)

Also, in restrict_SSLVersionRange() - I think you can just format all of the strings once at the top of the function. e.g. something like this:
{{{
char mymin[VERSION_STR_LENGTH], mymax[VERSION_STR_LENGTH];
char emin[VERSION_STR_LENGTH], emax[VERSION_STR_LENGTH];
char recommendedmin[VERSION_STR_LENGTH];

(void) slapi_getSSLVersion_str(slapdNSSVersions.min, mymin, sizeof(mymin)); 
(void) slapi_getSSLVersion_str(slapdNSSVersions.max, mymax, sizeof(mymax)); 
(void) slapi_getSSLVersion_str(enabledNSSVersions.min, emin, sizeof(emin)); 
(void) slapi_getSSLVersion_str(enabledNSSVersions.max, emax, sizeof(emax));
(void) slapi_getSSLVersion_str(SSL_LIBRARY_VERSION_TLS_1_1, recommendedmin, sizeof(recommendedmin));

}}}
It would save a lot of code below.

Finally, we don't have to hardcode the "1" in "TLS1". Take a look at the definitions:
{{{
#define SSL_LIBRARY_VERSION_TLS_1_0 0x0301
#define SSL_LIBRARY_VERSION_TLS_1_1 0x0302
}}}
The major version ("1") is just (value >> 8) - 2. So for "TLS" we could format the string version like this:
{{{
char
slapi_getSSLVersion_str(PRUint16 vnum, char
buf, size_t bufsize)
{
...
if (vnum >= SSL_LIBRARY_VERSION_3_0) { / e.g. TLSv1.x, TLSv2.x, etc. /
if (vnum & 0xff) { / TLS /
if (buf && bufsize) {
PR_snprintf(buf, bufsize, "TLS%d.%d", (vnum >> 8) - 2, (vnum & 0xff) - 1);
} else {
vstr = slapi_ch_smprintf("TLS%d.%d", (vnum >> 8) - 2, (vnum & 0xff) - 1);
}
...
}}}
That should give us a few years of future-proofing if TLS 2.x comes out

git patch file (master) -- updated the patch following comment:9 by Rich
0001-Ticket-47945-Add-SSL-TLS-version-info-to-the-access-.4.patch

Thanks. Almost there
{{{
if ((vnum & SSL_LIBRARY_VERSION_3_0) == SSL_LIBRARY_VERSION_3_0) {
...
}}}
This will only work for TLSv1.x. I would like to see support for TLSv2.x and later, something like this:
{{{
if (vnum >= SSL_LIBRARY_VERSION_3_0) {
if (vnum == SSL_LIBRARY_VERSION_3_0) { / SSL3 /
if (buf && bufsize) {
PR_snprintf(buf, bufsize, "SSL3");
} else {
vstr = slapi_ch_smprintf("SSL3");
}
} else { / TLS v X.Y /
const char TLSFMT = "TLS%d.%d";
int minor_offset = 0; /
e.g. 0x0401 -> TLS v 2.1, not 2.0 */

        if ((vnum & SSL_LIBRARY_VERSION_3_0) == SSL_LIBRARY_VERSION_3_0) {
            minor_offset = 1; /* e.g. 0x0301 -> TLS v 1.0, not 1.1 */
        }
        if (buf && bufsize) { 
            PR_snprintf(buf, bufsize, TLSFMT, (vnum >> 8) - 2, (vnum & 0xff) - minor_offset); 
        } else { 
            vstr = slapi_ch_smprintf(TLSFMT, (vnum >> 8) - 2, (vnum & 0xff) - minor_offset); 
        }
    }
} else { /* SSL2 or unknown */
    ...
}

}}}
That way, if vnum > SSL_LIBRARY_VERSION_3_0 (e.g. vnum == 0x0400 e.g. TLS v2.0) our code will support it with no changes.

git patch file (master) -- applied the change in comment:11 by Rich.
0001-Ticket-47945-Add-SSL-TLS-version-info-to-the-access-.5.patch

One more thing - please open a ticket for logconv.pl to support parsing/showing/reporting different protocol versions

Replying to [comment:13 rmeggins]:

One more thing - please open a ticket for logconv.pl to support parsing/showing/reporting different protocol versions

Done!
Ticket #47949 (new enhancement)
logconv.pl -- support parsing/showing/reporting different protocol versions

Reviewed by Rich (Thank you!!)

Pushed to master:
8f540a6..47868d3 master -> master
commit a2e0de3

Pushed to 389-ds-base-1.3.3:
d87202a..cb4f0cb 389-ds-base-1.3.3 -> 389-ds-base-1.3.3
commit 0d1087d

https://fedorahosted.org/389/ticket/47949 - addresses the logconv.pl changes for the new access logging.

Pushed to 389-ds-base-1.2.11:
099d1ce..8550aaf 389-ds-base-1.2.11 -> 389-ds-base-1.2.11
commit d62b281

Pushed to 389-ds-base-1.3.1:
d4d34b2..0680446 389-ds-base-1.3.1 -> 389-ds-base-1.3.1
commit 0680446be988316e26715a2c3dbeb3163f7f6e85

Pushed to 389-ds-base-1.3.2:
f7ae1e8..712d8eb 389-ds-base-1.3.2 -> 389-ds-base-1.3.2
commit 712d8eb

Metadata Update from @rmeggins:
- Issue assigned to nhosoi
- Issue set to the milestone: 1.3.3 backlog

2 years ago

Login to comment on this ticket.

Metadata