1425032 - reimplement PKCS#11 logout connection resetting using new "cancel all connections" event in necko

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement, P1)

Description

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Bug 1411316 added the event "net:cancel-all-connections", which as I understand it does what it says on the tin. Currently PSM tracks all sockets it creates just in case the user does a PKCS#11 logout (which very few users will ever do). We can get rid of this tracking if we reimplement this functionality on top of the new notification.

Comment 1

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Attachment: bug 1425032 - use new "cancel all connections" notification for PKCS#11 logout

When the user performs a PKCS#11 logout, we need to cancel all in-progress
network connections. Before this patch, PSM would track all the sockets it
created to implement this feature. However, bug 1411316 added the ability to
cancel these connections by sending the notification
"net:cancel-all-connections". This patch removes the now-unnecessary tracking
machinery in favor of delegating this to necko.

Review commit: https://reviewboard.mozilla.org/r/207540/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/207540/

Comment 2

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8936842 [details]
bug 1425032 - use new "cancel all connections" notification for PKCS#11 logout

https://reviewboard.mozilla.org/r/207540/#review213722

::: security/manager/ssl/tests/unit/test_logoutAndTeardown.js:136
(Diff revision 1)
> +      observer.stop();
> +    });
> +  }
> -    }
> +}
> +
> +function startServer(cert) {

I might prefer getStartedServer or something that communicates that it returns the server - not a strong opinion though.

Comment 3

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8936842 [details]
bug 1425032 - use new "cancel all connections" notification for PKCS#11 logout

https://reviewboard.mozilla.org/r/207540/#review213722

Thanks!

> I might prefer getStartedServer or something that communicates that it returns the server - not a strong opinion though.

Ok - sounds good.

Comment 4

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8936842 [details]
bug 1425032 - use new "cancel all connections" notification for PKCS#11 logout

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/207540/diff/1-2/

Comment 5

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3482d5edde05

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/746eeb7be002
use new "cancel all connections" notification for PKCS#11 logout r=mgoodwin

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/746eeb7be002