Fix: correct ssl_st member offsets

[blaisewang]

PR to fix TLSv1.3-related issues.

I use ecapture to hook libssl used by an NGINX server, not the client side libssl.
Then I found that neither the written key log file nor PcapNG with DSB could decrypt the TLS 1.3 traffic.

Environment:

1. NGINX with OpenSSL 1.1.1n
2. Cipher suites: TLS_AES_256_GCM_SHA384
3. curl compiled with USE_CURL_SSLKEYLOGFILE enabled

Issues I found and fixed:

1. write SERVER_HANDSHAKE_TRAFFIC_SECRET instead of a second CLIENT_HANDSHAKE_TRAFFIC_SECRET to the key log file
2. uses Hash function found in cipher_id of the master key event in expandLabel function

After fixed the above issues, still couldn't decrypt the TLS 1.3 traffic. In key log file, client_randoms are the same for both key log file. But four values are all different.

Possible issues I found:

In OpenSSL 1.1.1d, a new pointer peer_ciphers was introduced in the ssl_st struct which may affect all offsets related to hashes and secrets?

I tried:

X-Macros from this article was used to recalculate all offsets. But the program gave different offset for client_random in the ssl3_state_st struct. The current client_random offset should be correct, as it can be verified in the Client Hello message captured by tcpdump.

Result from the X-Macros program:

ssl_offsets openssl_offset_269488367 = { 
  .ssl_session_st_master_key = 0x50,
  .ssl_session_st_master_key_length = 0x8,
  .ssl3_state_st_client_random = 0xb8,
  .ssl_st_early_secret = 0x13c,
  .ssl_st_handshake_secret = 0x17c,
  .ssl_st_master_secret = 0x1bc,
  .ssl_st_resumption_master_secret = 0x1fc,
  .ssl_st_client_finished_secret = 0x23c,
  .ssl_st_server_finished_secret = 0x27c,
  .ssl_st_server_finished_hash = 0x2bc,
  .ssl_st_handshake_traffic_hash = 0x2fc,
  .ssl_st_client_app_traffic_secret = 0x33c,
  .ssl_st_server_app_traffic_secret = 0x37c,
  .ssl_st_exporter_master_secret = 0x3bc,
}

I have no clue for now to make this function work as expected. Maybe it's strict to the environment or the designated cipher suite?

@cfc4n any suggestion?

[cfc4n]

offset is correct. I think that hkdf code has a bug . I'll debug it later, maybe next mouth.

[blaisewang]

offset is correct. I think that hkdf code has a bug . I'll debug it later, maybe next mouth.

SSL_S3_CLIENT_RANDOM_OFFSET in masterkey_kern.h is set to 0xD8 or 40 (which should be 0xB8 in OpenSSL obviously) is not used.

I was misled by this offset.

[blaisewang]

offset is correct. I think that hkdf code has a bug . I'll debug it later, maybe next mouth.

Maybe handshake_secret's offset is 0x17C instead of 0x13C? So does for the rest. I got the same offsets by offsetof(SSL, member); the macro provided by clang.

After offset correction, the function is still not working.

HKDF from scapy (https://github.com/secdev/scapy/blob/master/scapy/layers/tls/crypto/hkdf.py) returned the same result as golang.org/x/crypto/hkdf.

I am starting to assume that handshake_traffic_hash from SSL struct is not ClientHello...ServerHello described in RFC 8446 7.1 (https://www.rfc-editor.org/rfc/rfc8446#section-7.1)

[cfc4n]

offset is correct. I think that hkdf code has a bug . I'll debug it later, maybe next mouth.

Maybe handshake_secret's offset is 0x17C instead of 0x13C? So does for the rest. I got the same offsets by offsetof(SSL, member); the macro provided by clang.

That is right. It's my falut.

After offset correction, the function is still not working.

maybe They are changed while eCapture capture them. I'm not an expert in TLS 1.3 handshake.

but I have an idea.

1. used eCapture capture them.
2. print they value in libssl.so source code, tls13_generate_secret function in https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/ssl/tls13_enc.c
3. compare them, try to find different. to identify the problem . is bug at eCapture hkdf or eCapture capture offset.

[blaisewang]

but I have a idea.

1. used eCapture capture them.
2. print they value in libssl.so source code, tls13_generate_secret function in https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/ssl/tls13_enc.c
3. compare them, try to find different. to identify the problem . is bug at eCapture hkdf or eCapture capture offset.

I will try to figure it out next week

[blaisewang]

Here's the code I used to log some ssl_st members' value in OpenSSL. Log first 12 char in hex format for handshake_secret, server_finished_hash, and handshake_traffic_hash.

---
 ssl/tls13_enc.c | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index b8fb07f210..e9eb1c6ae1 100644
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -251,6 +251,22 @@ int tls13_generate_secret(SSL *s, const EVP_MD *md,
     return ret == 0;
 }

+int ops_write(char *prefix, unsigned char *str) {
+    FILE *fptr = fopen("/tmp/openssl.log", "a+");
+
+    if (fptr == NULL) {
+        return -1;
+    }
+
+    fprintf(fptr, "%s %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n",
+            prefix,
+            str[0], str[1], str[2], str[3], str[4], str[5],
+            str[6], str[7], str[8], str[9], str[10], str[11]);
+
+    fclose(fptr);
+    return 0;
+}
+
 /*
  * Given an input secret |insecret| of length |insecretlen| generate the
  * handshake secret. This requires the early secret to already have been
@@ -260,9 +276,11 @@ int tls13_generate_handshake_secret(SSL *s, const unsigned char *insecret,
                                 size_t insecretlen)
 {
     /* Calls SSLfatal() if required */
-    return tls13_generate_secret(s, ssl_handshake_md(s), s->early_secret,
+    int ret = tls13_generate_secret(s, ssl_handshake_md(s), s->early_secret,
                                  insecret, insecretlen,
                                  (unsigned char *)&s->handshake_secret);
+    ops_write("handshake_secret", (unsigned char *)s->handshake_secret);
+    return ret;
 }

 /*
@@ -650,12 +668,15 @@ int tls13_change_cipher_state(SSL *s, int which)
      * Save the hash of handshakes up to now for use when we calculate the
      * client application traffic secret
      */
-    if (label == server_application_traffic)
+    if (label == server_application_traffic) {
         memcpy(s->server_finished_hash, hashval, hashlen);
+        ops_write("server_finished_hash", (unsigned char *)s->server_finished_hash);
+    }

-    if (label == server_handshake_traffic)
+    if (label == server_handshake_traffic) {
         memcpy(s->handshake_traffic_hash, hashval, hashlen);
-
+        ops_write("handshake_traffic_hash", (unsigned char *)s->handshake_traffic_hash);
+    }
     if (label == client_application_traffic) {
         /*
          * We also create the resumption master secret, but this time use the
--

I also logged the corresponding values in eCapture.
The values are only matched after offset corrections (see my changes in masterkey_kern.h). Therefore, the previous offsets are not correct.

But after the offset corrections, the decrypt still didn't work.
Now, I guess maybe we cannot obtain TLS key from only the four values.

Maybe we need to start over from the early secrets as described in this article (https://www.containiq.com/post/decrypting-ssl-at-scale-with-ebpf)?

@cfc4n You may merge my PR into a temporary branch as the fix isn't done yet.

[cfc4n]

why do not use cipher_id argument?

[blaisewang]

I would say that adding parameters to the method feels a bit redundant, especially doing cipher_id bitwise operation twice. The kind of crypto.Hash can be easily deduced from the size parameter. But I am not a Golang expert, I am not sure the best practice to do this. If you insist, I'll go with your way.

[cfc4n]

in ssl/ssl_local.h line 418 ,

/* used to hold info on the particular ciphers used */
struct ssl_cipher_st {
    uint32_t valid;
    const char *name;           /* text name */
    const char *stdname;        /* RFC name */
    uint32_t id;                /* id, 4 bytes, first is version */
    /*
     * changed in 1.0.0: these four used to be portions of a single value
     * 'algorithms'
     */
    uint32_t algorithm_mkey;    /* key exchange algorithm */

cipher_id means algorithm type. so use this method is a best way.

var transcript hash.Hash
	// test with different cipher_id
	switch cipher_id & 0x0000FFFF {
	case 0x1301:
		t.Log("TLS_AES_128_GCM_SHA256")
		transcript = crypto.SHA256.New()
	case 0x1302:
		t.Log("TLS_AES_256_GCM_SHA384")
		transcript = crypto.SHA384.New()
	case 0x1303:
		t.Log("TLS_CHACHA20_POLY1305_SHA256")
		transcript = crypto.SHA256.New()
	default:
		t.Log("Unknown cipher")
	}

[blaisewang]

I was too busy last week. I will open another PR to fix this later

[cfc4n]

thanks for your work. as you said, we need a new HOOK entry to get early secrets.

[blaisewang]

thanks for your work. as you said, we need a new HOOK entry to get early secrets.

Adding hook is very easy. But the real problem is that is the early_secrets really needed. I tried to implement the way described in that article last week, still didn't work (maybe my implementation is wrong). This leads to the previous mentioned question

Is handshake_traffic_hash from SSL struct the ClientHello...ServerHello described in RFC 8446 7.1 ?

From the comments of OpenSSL project, It does feel like the handshake_traffic_hash is the hash of ClientHello...ServerHello.

https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/ssl/tls13_enc.c#L601

Again, I am not a OpenSSL or TLS 1.3 expert either. So, I searched all over GitHub and I do found a project that written the same way you did.

https://github.com/KJTsanaktsidis/tlskeydump/blob/main/src/probes/openssl_probe.cxx#L267

After compiling and run the project in an unstable Debian as mentioned in the project manual. Nothing was written in the key log file, so no comparison.

I have no clue for now. Maybe searching an OpenSSL or TLS 1.3 expert for help is the best approach, or be an expert?

[cfc4n]

Again, I am not a OpenSSL or TLS 1.3 expert either. So, I searched all over GitHub and I do found a project that written the same way you did.

me too.

I have no clue for now. Maybe searching an OpenSSL or TLS 1.3 expert for help is the best approach, or be an expert?

I think that be an expert seems feasible.

[cfc4n]

merged