Make nsNSSSocketInfo::IsAcceptableForHost work asynchronously on the socket process

NEW
Unassigned

Status

()

enhancement
P5
normal
9 months ago
9 months ago

People

(Reporter: mayhemer, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 affected)

Details

(Whiteboard: [necko-triaged] maybe WONTFIX)

Reporter

Description

9 months ago
This is used for h2 conn joining an performs the cert chain verification, synchronously on the socket thread.

This method is going to be called on the socket process.  Certificate validation can only be done on the parent process, hence we need to IPC it, ideally asynchronously, in the worst case synchronously.

This will have two parts, if done async:
- PSM, should be fairly easy
- Necko, must wait for the result asynchronously

The Necko async part is going to be quite complicated.  Usage:

nsHttpConnectionMgr::TryDispatchTransaction,
nsHttpConnectionMgr::ProcessSpdyPendingQ,
nsHttpConnectionMgr::GetOrCreateConnectionEntry,
nsHttpConnectionMgr::nsConnectionEntry::AvailableForDispatchNow

all call it via:

nsHttpConnectionMgr::GetSpdyActiveConn -> nsHttpConnectionMgr::FindCoalescableConnection -> nsHttpConnectionMgr::FindCoalescableConnectionByHashKey -> nsHttpConnection -> ASpdySession -> nsISSLSocketControl.  


Patrick, Nick, do you happen to know about some way around it?  Note that right now we are already doing the verification on the socket process in a blocking way.  If we do a sync call to the parent and do it to a dedicated thread that can't be blocked on any sync calls to the socket process, we are probably safe.
Flags: needinfo?(mcmanus)
Flags: needinfo?(hurley)

Comment 1

9 months ago
Can you elaborate what you mean by "some way around it"? Do you mean some way of getting around the call into nsISSLSocketControl? Unfortunately, I don't believe so - all that info is required to know if we can merge the connections. We already coalesce more than other browsers do (though within the bounds of what's allowed by the spec), which leads to periodic confusion on the part of server implementors (and bug reports that I have to close). If we try to get clever, I'm afraid it will be far too easy to fall into a situation where we are improperly coalescing, and then we'll have real bugs to fix.

But, perhaps I'm misunderstanding your question.

(FTR, I think what you propose in your last sentence will work fine.)
Flags: needinfo?(hurley)
Reporter

Comment 2

9 months ago
What I would prefer is to turn the synchronous bool getter "IsAcceptableForHost" to be async.  But returning the same result as we do now.  I.e. to change the API, not the decision.

This is probably going to be very hard as the stack is deep and the code complicated.

Hence, doing this in a sync way is probably the way to go.  If there are issues, we will deal with them later.

I'm moving this to necko since the hard work would happen there.
Component: Security: PSM → Networking: HTTP
Flags: needinfo?(mcmanus)
Priority: -- → P5
Whiteboard: [necko-triaged] maybe WONTFIX
Reporter

Comment 3

9 months ago
(In reply to Honza Bambas (:mayhemer) from comment #2)
> Hence, doing this in a sync way is probably the way to go.  If there are

I mean through *sync IPC*
You need to log in before you can comment on or make changes to this bug.