845934 - Telemetry for TLS session resumption rate

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement)

Description

[Patrick McManus [:mcmanus]]

Let's find out what our TLS session resumption rate is.

brian points out that maximizing this number at all costs isn't the right way to interpret it (i.e. dropping pconns for servers that issue session tickets would do that - but it wouldn't be a good idea :)).

We might also want to know more nuanced things like what portion of servers issue session tickets, and what portion honor them (i've already got one which pretty clearly does not) which might be interesting inputs into 695789. And, as brian suggests, the number of non resumed sessions we make per host.

But I don't see a reason to block getting the basic piece of information because of those other nice to haves. Using it can help us decide how relevant strategies that accelerate non-resumed handshakes are on the web.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: patch 0

I changed one if(p) p->foo() call to be just p->foo() because the line after it was already unconditionally doing p->bar() and I wanted to add a line before it as p->baz().

borderline gratuitous, but I thought it was justified for clarity.

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 719109 [details] [diff] [review]
patch 0

Honza, brian suggested you could take this review (and one other) to unblock it.

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 719109 [details] [diff] [review]
patch 0

Review of attachment 719109 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab, but I haven't tested the patch personally.

::: security/manager/ssl/src/nsNSSCallbacks.cpp
@@ +833,5 @@
>    nsresult rv;
>    int32_t encryptBits;
>  
>    nsNSSSocketInfo* infoObject = (nsNSSSocketInfo*) fd->higher->secret;
> +  bool isFirstCallback = !(infoObject->GetFirstServerHelloReceived());

To make it more clear what the value of this flag actually holds, better name can be isResumedSession and provide a comment why infoObject->GetFirstServerHelloReceived() is giving you this information here.

@@ +838,3 @@
>  
> +  // This is the first callback on resumption handshakes
> +  infoObject->SetFirstServerHelloReceived();

Please leave the non-null check when accessing infoObject.

::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ +188,5 @@
>    }
>  }
>  
>  void
> +nsNSSSocketInfo::SetHandshakeCompleted(bool aFromFirstCallback)

s/aFromFirstCallback/aResumedSession/ ?

Comment 4

[Patrick McManus [:mcmanus]]

in email bsmith gave go ahead to push telemetry PSM patches without separate sr

https://hg.mozilla.org/integration/mozilla-inbound/rev/43a90cebe6e7

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/43a90cebe6e7