1116428 - Add security warnings to the Network Monitor

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Sami Jaktholm]

Bug 932179 brings per-request security state to the Network Monitor but cases where security is considered to be weak are poorly handled.

It should at least be able separate weakly secured requests from plaintext and tell why the request is weakly secured.

Comment 1

[Sami Jaktholm]

Attachment: netmonitor-security-warnings-1-backend.patch

Here's a patch that exposes security warnings in the actor.

The presence of minor security issues is signaled by STATE_IS_BROKEN flag. Flags STATE_USES_SSL_3 and STATE_USES_WEAK_CRYPTO specify the nature of those issues (see [1]).

This patch looks for STATE_IS_BROKEN in the state and if present, the state is set to "weak" and a list of reasons is attached to the info object. Currently reasons are "sslv3" for STATE_USES_SSL_3 and "cipher" for STATE_USES_WEAK_CRYPTO.

[1] https://hg.mozilla.org/mozilla-central/file/c0f88b376e33/security/manager/ssl/src/nsNSSCallbacks.cpp#l1233

Comment 2

[Sami Jaktholm]

Attachment: netmonitor-security-warnings-2-frontend.patch

Here's a patch that exposes the warnings in the UI.

If the security state of a request is "weak", the passive mixed content icon (grey triangle) is displayed next to the domain name in the request list.

The security details tab adds an alert icon (same as in inspector ruleview when inserting an invalid rule) next to the problematic property. For example a request that uses rc4 will show the alert icon at the "Cipher suite" line which has a tooltip specifying the problem.

The icon is aligned to the right and thus it's a bit hard to identify the problematic property. I tried to place the icon immediately after the value but I haven't been able to figure out how to tell the value label to take the remaining space in the container but not expand to fill if it the label does not require it.

Making the label flex=1 crops it correctly but the icon is pushed to the right as the label fills the remaining space. Without flexing a static width is required for cropping but I can't figure out how to make the max-width to be the remaining space in the container. Ideas are welcome.

But that's just a minor nuance and shouldn't be worried too much.

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

Comment 3

[Sami Jaktholm]

Attachment: netmonitor-warnings-ui-screenshot.png

Here's a screenshot with the warnings shown in netmonitor.

Comment 4

[Sami Jaktholm]

Attachment: netmonitor-security-warnings-1-backend.patch

The test in previous version contained an incorrect comment. Here's a fixed version.

Comment 5

[Victor Porof [:vporof][:vp]]

Comment on attachment 8554155 [details] [diff] [review]
netmonitor-security-warnings-2-frontend.patch

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

Very nice

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8554158 [details] [diff] [review]
netmonitor-security-warnings-1-backend.patch

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

Nice!

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/046c7d482f36
https://hg.mozilla.org/integration/fx-team/rev/06e5cde2c6fc

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/046c7d482f36
https://hg.mozilla.org/mozilla-central/rev/06e5cde2c6fc