Use Separate SSL Session Cache in OTR Mode

Use Separate SSL Session Cache in OTR Mode

Currently Chromium maintains a persistent TLS session cache between OTR and
non-OTR mode. This means that a user can go to https://gmail.com in ordinary
mode, open an incognito window, go to https://gmail.com and Chromium will 
use the TLS Session ID from ordinary mode to resume that TLS session.

This patch changes the behaviour as follows:

- TLS Session IDs generated in non-OTR mode are only used in non-OTR mode.
- TLS Session IDs generated in OTR mode are only used in OTR mode.

The patch implements this behavour for Linux, Mac and Windows. 

Chromium's OTR profile now has it's own copy of the SSL configuration settings.
If the profile is OTR, a new member of SSLConfig called otr_mode is
set to true. By default, otr_mode is set to false.

On Mac and Linux, if otr_mode is true the phrase "-OTR" is added to the hostname
and port information when constructing the peer_id used to calculate and store
the SSL connection's session ID. This results in a distinct session cache for
SSL connections for each mode.

On Windows an extra bit is added to SSL_VERSION_MASKS. This bitmask defines the
number of CredHandles stored in a CredHandle lookup table. When the SSL connection
belongs to a request made in OTR mode the CredHandle for that connection will be
stored and retrieved from the CredHandle members of the array reserved for OTR
mode.


See also:
https://groups.google.com/group/chromium-extensions/msg/e83920020719a6b2?hl=en

BUG=30877
TEST=https sites perform identically (in particular, https://test-ssev.verisign.com/ and the three pages linked from there)

[abarth-chromium, 2010-02-03 19:58:02]

Sorry for missing this review!  I think wtc is your best bet.

[Matt Perry, 2010-02-03 20:01:04]

Wan-Teh, are you the right person to do this review?

[wtc, 2010-02-03 20:20:23]

Matt: do you want to completely turn off TLS session
resumption in OTR mode?  Can the OTR mode use its own TLS
session ID cache?

Turning off TLS session resumption makes TLS connection
setup slower.

[Matt Perry, 2010-02-10 19:15:23]

On 2010/02/03 20:20:23, wtc wrote:
> Matt: do you want to completely turn off TLS session
> resumption in OTR mode?  Can the OTR mode use its own TLS
> session ID cache?
> 
> Turning off TLS session resumption makes TLS connection
> setup slower.

Robert is the patch author, so he'll have to weigh in on this. But from my
understanding, the goal is simply to enforce separate TLS session IDs between
incognito and regular profiles. Turning off the TLS session cache completely is
perhaps too extreme a way to accomplish this.

[mwenge, 2010-02-12 15:52:24]

On Wednesday 10 February 2010 19:15:23 mpcomplete@chromium.org wrote:
> On 2010/02/03 20:20:23, wtc wrote:
> > Matt: do you want to completely turn off TLS session
> > resumption in OTR mode?  Can the OTR mode use its own TLS
> > session ID cache?
> >
> > Turning off TLS session resumption makes TLS connection
> > setup slower.
> 
> Robert is the patch author, so he'll have to weigh in on this. But from
>  my understanding, the goal is simply to enforce separate TLS session
>  IDs between
> incognito and regular profiles. Turning off the TLS session cache
> completely is
> perhaps too extreme a way to accomplish this.
> 
> http://codereview.chromium.org/502087
> 

I did it this way because I thought maintaining separate caches for OTR 
would be harder than it actually is. 

Just to clarify the initial patch, the options available for controlling 
use of the ssl session cache are quite different for each of Linux, Windows 
and OSX and in fact my patch does one thing for Linux, a slightly different 
thing for Windows while it can't do anything for OSX.

Linux: Use NSS's SSL_NO_CACHE option to ignore the session cache when in 
OTR mode.

Windows: If in OTR mode re-initialize the credentials handle for a given 
ssl connection so that the ssl cache is cleared for that host and 
configure the handle so that future sessions are only cached for 1 
milisecond. (I don't have the setup to compile and test this particular 
code so it is written on the basis of windows docs. It also has a typo 
which needs to be corrected.)

OSX: Session caching isn't currently supported by chromium because a new 
SSL context is created for every socket, and OSX ties its session cache to 
SSL contexts.

So to implement separate SSL caches for OTR mode:

Linux: Create and use a separate NSS singleton when using/creating SSL 
sockets in OTR mode. (I take it modifying NSS directly is not an option). 

Windows: Create a separate table of CredHandles for OTR mode in 
ssl_client_socket_win.cc.

OSX: Unchanged.

So appears to be quite do-able.

Apologies for the delay in responding - I was away. I'll have a go at the 
above.

[wtc, 2010-02-12 23:16:02]

On 2010/02/12 15:52:24, mwenge wrote:
>
> OSX: Session caching isn't currently supported by chromium because a new 
> SSL context is created for every socket, and OSX ties its session cache to 
> SSL contexts.

Are you sure?

We are calling SSLSetPeerID on Mac OS X to allow sessions to
be resumed.  I just did an experiment and saw Chrome resume
sessions on Mac OS X.

If Chrome is not resuming SSL sessions, that's a P1 bug.

Before you write more code, could you state what your goal is?
Anonymous browsing, or distinct identity for incognito mode?
Our goal for OTR (off the record) mode is to not leave behind
browsing history on the disk.

[mwenge, 2010-02-13 12:49:21]

On Friday 12 February 2010 23:16:03 wtc@chromium.org wrote:
> On 2010/02/12 15:52:24, mwenge wrote:
> > OSX: Session caching isn't currently supported by chromium because a
> > new SSL context is created for every socket, and OSX ties its session
> > cache to SSL contexts.
> 
> Are you sure?
> 
> We are calling SSLSetPeerID on Mac OS X to allow sessions to
> be resumed.  I just did an experiment and saw Chrome resume
> sessions on Mac OS X.
> 
> If Chrome is not resuming SSL sessions, that's a P1 bug.

I didn't mean to sound quite so certain! In the note to the patch I said: 
"As far as I can tell Chromium Mac does not fully support TLS Session 
Resumption because a new SSL context is created for every socket, and OSX 
ties its session cache to SSL contexts. So whereas Win/Linux can support 
session resumption across socket creation, Chromium Mac cannot - and my 
sense is that a new socket will be created for most new SSL connections."

From the sound of it I'm wrong, so will look more closely at the OSX code. 

> 
> Before you write more code, could you state what your goal is?
> Anonymous browsing, or distinct identity for incognito mode?

In narrow terms, ensure non-OTR mode does not use session ids created in 
OTR mode and vice versa. This complements the requirements for anonymous 
browsing (such as with Tor, I am working with Mike Perry from the Tor team 
on the API requirements for a chromium plugin for Tor). It is also intended 
to be consistent with the notion that locally cached information, such as 
browsing history, is not shared between OTR and non-OTR modes.

The patch was prompted by the discussion at:

https://groups.google.com/group/chromium-
extensions/msg/e83920020719a6b2?hl=en&pli=1

> Our goal for OTR (off the record) mode is to not leave behind
> browsing history on the disk.
> 

> http://codereview.chromium.org/502087
> 

[mwenge, 2010-07-21 09:56:55]

On 2010/02/12 23:16:02, wtc wrote:
> Before you write more code, could you state what your goal is?
> Anonymous browsing, or distinct identity for incognito mode?
> Our goal for OTR (off the record) mode is to not leave behind
> browsing history on the disk.

Hi wtc,

The patch now implements separate session ID caches for OTR and non-OTR mode.
This means if you close an OTR session it won't be possible to test recent use
of a site in OTR mode by fetching it and watching the SSL handshake. This is a
very elaborate way of compromising Incognito mode but is certainly practical.

I was only able to test the patch on Linux.

Robert
I think the behaviour implemented by this patch complements OTR's requirement to
leave no trace behind

[davidben, 2010-07-26 21:31:33]

I'm not sure the approach taken here is the right one. The networking layer and
all these other pieces should not depend on the existence of an OTR mode. It
will also break should Chromium decide to create another profile for any other
reason.

It looks like the nuisance here comes from NSS (and it seems OS X as well?)
using a global session cache. I gather from your approach on Win32 that making a
separate handle is sufficient there. See my comments on your implementation of
that in-line below.

Probably the proper way to deal with this would be to create a cache name or
something to identify a "session cache context" or so. It... possibly wants to
be attached to the SSLConfig, so that URLRequestContexts may choose whether
share them or not.

Stringifying the profiles is likely to be hairy, so it's probably best to use a
shared counter maintained in the network side of things as the cache name.

http://codereview.chromium.org/502087/diff/21001/22004
File net/socket/ssl_client_socket_mac.cc (right):

http://codereview.chromium.org/502087/diff/21001/22004#newcode815
net/socket/ssl_client_socket_mac.cc:815: peer_id += std::string("OTR");
Unless hostname also includes the port (and even then, this feels poor), you'd
want to integrate the uniquifier with proper separators. You're probably saved
at the last minute from collisions by the capitalization, but it's not something
immediately apparent to someone reading the code.

http://codereview.chromium.org/502087/diff/21001/22005
File net/socket/ssl_client_socket_nss.cc (right):

http://codereview.chromium.org/502087/diff/21001/22005#newcode526
net/socket/ssl_client_socket_nss.cc:526: peer_id += std::string("OTR");
An explicit separator here (another colon?) would be nice, even though there's
no chance of collision.

http://codereview.chromium.org/502087/diff/21001/22006
File net/socket/ssl_client_socket_win.cc (right):

http://codereview.chromium.org/502087/diff/21001/22006#newcode80
net/socket/ssl_client_socket_win.cc:80: OTR_MODE = 1 << 3,
OTR_MODE doesn't make much sense as a member of SSL version mask.

It looks like your bit only ends up being useful as a uniquifier for the
CredHandle maps in CredHandleTable. I think it would be cleaner just to key by
something else there. The optimization in anonymous_creds_ can probably be
preserved by making CredHandleTable not a singleton and putting the name lookup
at a higher level. (And I believe we now use NSS for sockets without client
certs on all platforms anyway.)

[jochen (gone - plz use gerrit), 2010-07-27 18:40:06]

Just a random note: the issue described in the description of this issue is not
breaking the contract of chrome's otr mode. we only promise not to leave local
traces, and not to hinder a remote server to trace you.

[mwenge, 2010-07-27 19:28:06]

On 2010/07/27 18:40:06, jochen wrote:
> Just a random note: the issue described in the description of this issue is
not
> breaking the contract of chrome's otr mode. we only promise not to leave local
> traces, and not to hinder a remote server to trace you.

Hi Jochen,

There is a local trace in this case - a local user can open a packet sniffer and
try the site. The use of session IDs would indicate you have visited it in a
recent OTR session. A bit of an elaborate problem case I grant you, but a real
one I think.

[davidben, 2010-07-27 20:35:14]

This comment from Robert seemed to only make it to email and not the page
itself. Reproducing below:
----

Hi Ben,

Thanks for the feedback!

On Monday 26 July 2010 22:31:33 davidben@chromium.org wrote:
>
> It looks like the nuisance here comes from NSS (and it seems OS X as
> well?) using a global session cache.

Exactly!

>
> Probably the proper way to deal with this would be to create a cache
> name or something to identify a "session cache context" or so. It...
> possibly wants to
> be attached to the SSLConfig, so that URLRequestContexts may choose
> whether share them or not.
>
I don't think I follow you here.

My patch introduces a separate SSLConfig for each profile. Are you saying
that instead of assuming the only 'other' profile will ever be OTR I should
maintain a unique reference in the SSLConfig, such as a counter, and allow
that to uniquify the entry in the session id cache instead of just
appending OTR to the host/port?

I don't know the code well enough to know where URLRequestContexts come
into it in this situation.

[davidben, 2010-07-27 20:58:32]

> Hi Ben,

Hi. (Actually, I'm David. :-D But I get called Ben often enough that I mostly
answer to it.)

> I don't think I follow you here.
> 
> My patch introduces a separate SSLConfig for each profile. Are you saying
> that instead of assuming the only 'other' profile will ever be OTR I should
> maintain a unique reference in the SSLConfig, such as a counter, and allow
> that to uniquify the entry in the session id cache instead of just
> appending OTR to the host/port?
> 
> I don't know the code well enough to know where URLRequestContexts come
> into it in this situation.

Right, a separate SSLConfigService for each profile is certainly be part of the
solution. But your patch only uniquifies the peer ID by an OTR bit. The network
stack is not intended to depend on the browser components, so a more generic
solution would be better. So yeah; I'm saying the uniquifying should be
independent of OTR mode, possibly by some kind of counter.

The URLRequestContext is an class that users of the network stack subclass and
pass in with requests. State shared between requests hangs off there (cookies,
SSLConfigService, etc.). As "which SSL session" cache to use is such state, it
should ultimately be determined by that class, probably by way of SSLConfig and
SSLConfigService.

I'm not sure just sticking the uniquifier in the SSLConfig will work. It's
conceivable that we instantiate lots of duplicates, or that we may create
variants of the same URLRequestContext, so there should be a mechanism for them
to share caches. I'm not familiar with that code. Others probably could provide
more authoritative comments as to where would be best place. I'm imagining some
kind of a net::SSLSessionCache handle which wraps an internal counter, with the
handle the context chooses selecting the cache.