1580272 - When running tests, ssl external cache is cleared

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P2)

Description

[Kershaw Chang [:kershaw]]

I found this problem when I tried to run this test with turning network.ssl_tokens_cache_enabled on.
We set the resumption token at here, but the session id is cleared in head_psm.js by calling proxyStartSSL.
Since we got a null session if in ssl3_SendClientHello, we can't resume the session successfully.

Comment 1

[Dana Keeler (she/her) [:keeler]]

Seems like maybe NSS is not treating the external cache like the internal cache? Or maybe I'm misunderstanding what this bug is.

Comment 2

[Kershaw Chang [:kershaw]]

NSS clears the cache no matter it's internal or external in ssl_ResetSecurityInfo, which is triggered by calling proxyStartSSL.
I think the problem is why do we have to call sslSocketControl.proxyStartSSL() at here? Could we remove it or move it to somewhere else?

Comment 3

[Dana Keeler (she/her) [:keeler]]

proxyStartSSL is how we start the TLS handshake. Maybe driveHandshake would work? In any case, I'm still confused why the test would work with the NSS cache but not work with the external cache.

Comment 4

[Kershaw Chang [:kershaw]]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #3)

proxyStartSSL is how we start the TLS handshake. Maybe driveHandshake would work? In any case, I'm still confused why the test would work with the NSS cache but not work with the external cache.

Because the sid is assigned at here, which is happened after proxyStartSSL. If we use external cache, sid is assigned before proxyStartSSL, so it will be cleared.

FWIW, all unit tests under security/manager/ssl/tests are passed without proxyStartSSL. Maybe we can just remove these lines.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=496f5285b4f700bb7584cf02099178018ca7f6b6&selectedJob=267274402

Comment 5

[Dana Keeler (she/her) [:keeler]]

Huh - ok. Maybe those lines were never necessary.

Comment 6

[Dana Keeler (she/her) [:keeler]]

Kershaw - it sounds like you have a patch with the fix we'll need (removing the call to proxyStartSSL) - do you want to post that and I can review it?

Comment 7

[Kershaw Chang [:kershaw]]

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #6)

Kershaw - it sounds like you have a patch with the fix we'll need (removing the call to proxyStartSSL) - do you want to post that and I can review it?

Sure. I can do this.

Kevin, I am sorry for stealing this bug from you. Feel free to take this back if you have another approach.

Comment 8

[Kershaw Chang [:kershaw]]

Attachment: Bug 1580272 - Remove unnecessary call to proxyStartSSL

Comment 9

[Kershaw Chang [:kershaw]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b5faba078f97a630d7e054050047cb755a18394d

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aad903e8e61e
Remove unnecessary call to proxyStartSSL r=keeler

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/aad903e8e61e