Re: TLS Next Protocol Negotiation integration for SPDY

50 views
Skip to first unread message

Patrick McManus

unread,
Aug 13, 2011, 11:44:16 AM8/13/11
to dev-tech-network
Below was the plan, the reality went somewhat similar:

instead of modifiying nsisocketprovider, which non-http and non-ssl
things use, I modified nsISSLSocketControl to take a new setNPNList
method. The HTTP code uses this during init if the SPDY pref is set and
we are on SSL. The implementation of setNPNList is in nsNSSIOLayer,
which calls into nsSSLThread to safely use the new SSL_SetNextProto NSS
function there.

So that's the 'input'. The 'output' went closer to plan.. a new
attribute was added into nsISSLStatus, that is populated through
HandshakeCallback() with very little code.

The trickiest thing is that http connection needs to wait for the
handshake to be complete before writing any data into nss - indeed it
needs to know whether to send http or spdy syntax afterall. The trick
was that the nss state machine doesn't start the handshake (much less
complete it) without the write taking place. I added a harmless
asynchronous read request when this happens (because the recv also kicks
off the handshake) even though this is a write-before-read protocol.

the code sits here:
http://hg.mozilla.org/users/mcmanus_ducksong.com/spdy

obviously, NPN is just the tip of the iceberg.

On Fri, 2011-08-05 at 12:54 -0400, Patrick McManus wrote:

>
> modify nsisocketprovider.idl methods addtosocket() and newsocket() to
> take an additional parameter which is an ordered list of acceptable
> protocol names. (we'd set it to {spdy/X.0, http/1.1} in socketprovider
> only when doing https things). Failure to negotiate one of those is ok,
> it doesn't add any overhead and we just chase the old code paths.
>
> fixup implementations of nsisocketprovider to deal with that change and
> for the ones that call into ssl make a change to nsSSLIOLayerAddToSocket
> in nsNSSIOLayer.cpp to also take the new protocol list.
>
> Inside that function (or maybe nsSSLIOLayerSetOptions()?) call
> SSL_SetNextProtoNego() using the passed protocol list (from Adam's
> patch) to kick things off.
>
> modify nsISSLStatus.idl to add a readonly negotiatedNPN attribute -
> default to "".
>
> modify HandshakeCallback() in nsNSSCallbacks.cpp to populate the new
> status attribute using SSL_GetNextProto() (from the new patch).
>
> then finally something in netwerk/http (probably nsHttpconnection or
> ConnectionMgr) can call getsecurityinfo() and QI that to an
> nsISSLStatusProvider, and call GetSSLStatus() on that and then end up
> with the nsISSLStatus and corresponding negotiatedNPN string with which
> we can tell if we should speak spdy or http going forward.
>
> Does that pass the basic sniff test?
>
> -Pat
>
>
> _______________________________________________
> Necko-devs mailing list
> Necko...@mozilla.org
> https://mail.mozilla.org/listinfo/necko-devs


Patrick McManus

unread,
Aug 13, 2011, 11:52:26 AM8/13/11
to dev-tech-network
I forgot the fun part - I should mention that using this you can connect
to https://encrypted.google.com and successfully negotiate SPDY. Of
course then it all goes kablooie :)

Brian Smith

unread,
Aug 13, 2011, 2:31:52 PM8/13/11
to Patrick McManus, dev-tech-network
Patrick McManus wrote:
> instead of modifiying nsisocketprovider, which non-http and non-ssl
> things use, I modified nsISSLSocketControl to take a new setNPNList
> method. The HTTP code uses this during init if the SPDY pref is set
> and we are on SSL. The implementation of setNPNList is in
> nsNSSIOLayer, which calls into nsSSLThread to safely use the new
> SSL_SetNextProtocol NSS function there.

The NPN extension is a useful security feature for preventing cross-protocol attacks. We should be using it for all SSL connections, even when there is a single choice. That is, eventually every user of the SSL/TLS socket transports should pass in a (often singleton) list of possible protocols when constructing the transport.

> The trickiest thing is that http connection needs to wait for the
> handshake to be complete before writing any data into nss - indeed it
> needs to know whether to send http or spdy syntax afterall. The trick
> was that the nss state machine doesn't start the handshake (much less
> complete it) without the write taking place. I added a harmless
> asynchronous read request when this happens (because the recv also
> kicks off the handshake) even though this is a write-before-read
> protocol.

There is a function SSL_ForceHandshake that does this. I would avoid trying to emulate it outside of libssl if possible, as there are a lot of subtleties. See https://bugzilla.mozilla.org/buglist.cgi?quicksearch=SSL_ForceHandshake for examples.

/* Try to make progress on an SSL handshake by attempting to read the
** next handshake from the peer, and sending any responses.
** For non-blocking sockets, returns PR_ERROR_WOULD_BLOCK if it cannot
** read the next handshake from the underlying socket.
** For SSLv2, returns when handshake is complete or fatal error occurs.
** For SSLv3, returns when handshake is complete, or application data has
** arrived that must be taken by application before handshake can continue,
** or a fatal error occurs.
** Application should use handshake completion callback to tell which.
*/

I believe the "or application data has arrived that must be taken by the application before the handshake can continue" case only occurs during renegotiations, which shouldn't be relevant to your changes.

- Brian

Patrick McManus

unread,
Aug 13, 2011, 4:21:23 PM8/13/11
to Brian Smith, dev-tech-network
On Sat, 2011-08-13 at 11:31 -0700, Brian Smith wrote:

> > The trickiest thing is that http connection needs to wait for the
> > handshake to be complete before writing any data into nss - indeed it
> > needs to know whether to send http or spdy syntax afterall. The trick
> > was that the nss state machine doesn't start the handshake (much less
> > complete it) without the write taking place. I added a harmless
> > asynchronous read request when this happens (because the recv also
> > kicks off the handshake) even though this is a write-before-read
> > protocol.
>
> There is a function SSL_ForceHandshake that does this. I would avoid trying to emulate it outside of libssl if possible, as there are a lot of subtleties. See https://bugzilla.mozilla.org/buglist.cgi?quicksearch=SSL_ForceHandshake for examples.
>

I went down that road. It blocked the thread when dealing with OSCP in a
way that send/recv did not.

Do you see a problem with initiating the recv() in order to activate the
handshake? It isn't a problem for the HTTP state machine.


Brian Smith

unread,
Aug 13, 2011, 4:41:00 PM8/13/11
to Patrick McManus, dev-tech-network
Patrick McManus wrote:

> Brian Smith wrote:
> > There is a function SSL_ForceHandshake that does this. I would avoid
> > trying to emulate it outside of libssl if possible, as there are a
> > lot of subtleties. See
> > https://bugzilla.mozilla.org/buglist.cgi?quicksearch=SSL_ForceHandshake
> > for examples.
>
> I went down that road. It blocked the thread when dealing with OSCP in
> a way that send/recv did not.

I will investigate this. Keep doing things the way you are for now. My guess is that the call to SSL_ForceHandshake must be made on the SSL thread.

> Do you see a problem with initiating the recv() in order to activate
> the handshake? It isn't a problem for the HTTP state machine.

For now, I'd just make a note in your code that calls revc() that we are doing this because SSL_ForceHandshake would block. I will fix things so that SSL_ForceHandshake will work correctly here.

- Brian

Reply all
Reply to author
Forward
0 new messages