Closed Bug 1310201 Opened 8 years ago Closed 8 years ago

Implement TCP Fast Open

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: dragana, Assigned: dragana)

References

Details

Attachments

(1 file)

During TCP Fast Open we need to use PR_Sendto to send data. We also need to supply address for PR_Sendto.
Blocks: 1188435
Depends on: 1310197
Attachment #8801106 - Flags: review?(ekr)
This does not work well with TLS1.3 Early data, but I will fix that in a separate bug.

The reason: necko needs to call SSL_ForceHandshake to initialize NSSIOLayer and nss structures, also nss will look for cached alpn. During that call clientHello is sent. After that necko knows the application protocol and can send early-data. We need to be able to postpone clientHello, it needs to be written to the socket together with early-data.
Comment on attachment 8801106 [details] [diff] [review]
bug_1310201_nss_v1.patch

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

Hi Dragana,

I'm not sure that this is the right strategy. As I understand it, TLS should be thinking of TFO sockets abstractly as like TCP sockets, it's just that the lower level API is different. ISTM that we should be avoiding making libssl accomodate that. Rather, it should be done either in NSPR or more likely you should insert a TCP TFO layer between libssl and NSPR TCP sockets.
(In reply to Eric Rescorla (:ekr) from comment #3)
> Comment on attachment 8801106 [details] [diff] [review]
> bug_1310201_nss_v1.patch
> 
> Review of attachment 8801106 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Dragana,
> 
> I'm not sure that this is the right strategy. As I understand it, TLS should
> be thinking of TFO sockets abstractly as like TCP sockets, it's just that
> the lower level API is different. ISTM that we should be avoiding making
> libssl accomodate that. Rather, it should be done either in NSPR or more
> likely you should insert a TCP TFO layer between libssl and NSPR TCP sockets.

Thanks. 
it is your call. 
I would not put it in nspr, I prefer not making nspr even more complicated. I will make a separate layer and use set/getsockOpt to set it up.
Comment on attachment 8801106 [details] [diff] [review]
bug_1310201_nss_v1.patch

Dropping the review flag. I will leave the bug open for now and probably close it next week.
Attachment #8801106 - Flags: review?(ekr)
(In reply to Dragana Damjanovic [:dragana] from comment #4)
> (In reply to Eric Rescorla (:ekr) from comment #3)
> > Comment on attachment 8801106 [details] [diff] [review]
> > bug_1310201_nss_v1.patch
> > 
> > Review of attachment 8801106 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Hi Dragana,
> > 
> > I'm not sure that this is the right strategy. As I understand it, TLS should
> > be thinking of TFO sockets abstractly as like TCP sockets, it's just that
> > the lower level API is different. ISTM that we should be avoiding making
> > libssl accomodate that. Rather, it should be done either in NSPR or more
> > likely you should insert a TCP TFO layer between libssl and NSPR TCP sockets.
> 
> Thanks. 
> it is your call. 
> I would not put it in nspr, I prefer not making nspr even more complicated.

I agree.


> I will make a separate layer and use set/getsockOpt to set it up.

I think that a separate layer is the right approach. I don't know if you want a sockopt or just a separate handle on that layer, where it's either a passthrough or a TFO buffer
(In reply to Eric Rescorla (:ekr) from comment #6)
> (In reply to Dragana Damjanovic [:dragana] from comment #4)
> > (In reply to Eric Rescorla (:ekr) from comment #3)
> > > Comment on attachment 8801106 [details] [diff] [review]
> > > bug_1310201_nss_v1.patch
> > > 
> > > Review of attachment 8801106 [details] [diff] [review]:
> > > -----------------------------------------------------------------
> > > 
> > > Hi Dragana,
> > > 
> > > I'm not sure that this is the right strategy. As I understand it, TLS should
> > > be thinking of TFO sockets abstractly as like TCP sockets, it's just that
> > > the lower level API is different. ISTM that we should be avoiding making
> > > libssl accomodate that. Rather, it should be done either in NSPR or more
> > > likely you should insert a TCP TFO layer between libssl and NSPR TCP sockets.
> > 
> > Thanks. 
> > it is your call. 
> > I would not put it in nspr, I prefer not making nspr even more complicated.
> 
> I agree.
> 
> 
> > I will make a separate layer and use set/getsockOpt to set it up.
> 
> I think that a separate layer is the right approach. I don't know if you
> want a sockopt or just a separate handle on that layer, where it's either a
> passthrough or a TFO buffer

both is ok. Thanks.
If we do not want to do TFO we just do not add the layer, that is the easiest.
That seems like a good plan
That seems like a good plan
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: