Closed Bug 1116428 Opened 5 years ago Closed 5 years ago

Add security warnings to the Network Monitor

Categories

(DevTools :: Netmonitor, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 38

People

(Reporter: sjakthol, Assigned: sjakthol)

References

Details

Attachments

(3 files, 1 obsolete file)

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.
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
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8554154 - Flags: review?(past)
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
Attachment #8554155 - Flags: review?(vporof)
Here's a screenshot with the warnings shown in netmonitor.
The test in previous version contained an incorrect comment. Here's a fixed version.
Attachment #8554154 - Attachment is obsolete: true
Attachment #8554154 - Flags: review?(past)
Attachment #8554158 - Flags: review?(past)
Comment on attachment 8554155 [details] [diff] [review]
netmonitor-security-warnings-2-frontend.patch

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

Very nice
Attachment #8554155 - Flags: review?(vporof) → review+
Comment on attachment 8554158 [details] [diff] [review]
netmonitor-security-warnings-1-backend.patch

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

Nice!
Attachment #8554158 - Flags: review?(past) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/046c7d482f36
https://hg.mozilla.org/mozilla-central/rev/06e5cde2c6fc
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.