Closed Bug 1716082 (CVE-2022-45419) Opened 3 years ago Closed 2 years ago

Deleting security exception doesn't restore warning

Categories

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

Firefox 89
x86_64
Windows 10
defect

Tracking

()

VERIFIED FIXED
107 Branch
Tracking Status
firefox-esr102 - wontfix
firefox105 - wontfix
firefox106 - wontfix
firefox107 + verified

People

(Reporter: mozillabugs, Assigned: keeler)

Details

(Keywords: sec-low, Whiteboard: [psm-assigned][post-critsmash-triage][adv-main107+])

Attachments

(2 files)

Adding a security exception for a site with a mismatched cert should allow access without a warning only as long as the exception exists. However, if you delete an exception, then visit the site again, FF shows no warning. You have to restart FF for the warning to appear again.

STR:

  1. Visit https://download.windowsupdate.com/d/upgr/2021/03/windows10.0-kb5000736-x64_880844224a175033802b3d7a1f40ec304c0548dd.msu .
  2. Notice the warning [1]
  3. Add a security exception for the site.
  4. Cancel the download.
  5. Remove the security exception at tools/settings/certificates/view certificates/servers.
  6. Open a new tab and visit the site again. Notice that FF offers to download the file without giving a warning. Cancel the download.
  7. Restart FF.
  8. Visit the site again.
  9. Notice the warning.

[1] Hey Microsoft! Why isn't your cert correct?!?!

Flags: sec-bounty?

BTW FF allows you to download the file without a warning if you continue the download, rather than cancelling it, at step 6.

This is probably expected behavior. For perf reasons sites negotiate a TLS session token with clients to avoid the overhead of doing the full handshake for each new resource a browser is going to request. Browsers make a lot of separate requests for a typical web page with lots of sub-resources (scripts, styles, images, etc) and you don't want to do the handshake for each one. The certificate isn't checked again until the TLS session expires (a day is typical? or until you shut down) so nothing would trigger the need for the deleted exception in this scenario.

Is that right, Dana?

Group: core-security → crypto-core-security
Component: Security → Security: PSM
Flags: needinfo?(dkeeler)

That's all true, but from my reading of the implementation, we should be clearing the TLS session cache when exceptions get removed, here: https://searchfox.org/mozilla-central/rev/294f10eff7398d6b05beac6aa256d86ac3cb7113/security/manager/ssl/nsCertOverrideService.cpp#704
Perhaps this is due to the BF cache (or maybe network cache)?
At step 6, what happens if you shift-refresh when viewing the page?

Flags: needinfo?(dkeeler) → needinfo?(mozillabugs)

Opening a new tab and visiting the site again shouldn't hit the BF cache, FWIW.

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

That's all true, but from my reading of the implementation, we should be clearing the TLS session cache when exceptions get removed, here: https://searchfox.org/mozilla-central/rev/294f10eff7398d6b05beac6aa256d86ac3cb7113/security/manager/ssl/nsCertOverrideService.cpp#704
Perhaps this is due to the BF cache (or maybe network cache)?
At step 6, what happens if you shift-refresh when viewing the page?

If I complete step 6 as writ, and do shift/refresh afterward, FF clears the address bar. If I then paste the URL back in the address bar and hit "enter", FF offers to download it without showing the scare-screen. Same thing if, after doing the shift/refresh thing, I open a new tab and open the URL. And the same thing if I open a new (non-private) window and open the URL. However, if I open a new private window and go to the URL, it shows the scare screen.

If I don't open a new tab at step 6, but instead shift/refresh in the same tab I used for steps 1-5, FF shows the scare-screen.

Flags: needinfo?(mozillabugs)

Shift-refresh clears the cache for the current site you're visiting, so if you open a new tab and don't visit a site, that won't do anything. It sounds like you're hitting the network cache. Try clearing it before step 6 with history -> clear recent history -> cache?

Flags: needinfo?(mozillabugs)

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

Shift-refresh clears the cache for the current site you're visiting, so if you open a new tab and don't visit a site, that won't do anything. It sounds like you're hitting the network cache. Try clearing it before step 6 with history -> clear recent history -> cache?

When I do that immediately after step 5, FF doesn't show the scare-screen at step 6; it just offers to download the file.

Flags: needinfo?(mozillabugs)

Passing needinfo back to Dana to continue figuring out what's happening here. :-)

Flags: needinfo?(dkeeler)

Unfortunately the link in comment 0 doesn't work any longer (which is why this fell off my radar the first time). I tried reproducing locally, but I couldn't get the same behavior. I don't suppose there's another link you have that behaves the same way?

Flags: needinfo?(dkeeler) → needinfo?(mozillabugs)

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

Unfortunately the link in comment 0 doesn't work any longer (which is why this fell off my radar the first time). I tried reproducing locally, but I couldn't get the same behavior. I don't suppose there's another link you have that behaves the same way?

The link worked for me just a few seconds ago. What error do you get?

Odd - it was refusing connections for me, but now it seems to be working again. In any case, it looks like what's happening is the first connection opens a connection with keep-alive that gets reused after the exception is removed. There's no new (or even resumed) TLS connection.

Dragana, is there a way to reset/clear connections by hostname? Is that even something we would want to do? (context is removing certificate error exceptions)

Flags: needinfo?(mozillabugs) → needinfo?(dd.mozilla)

It is unfortunate that our UX is confusing. I see that it's odd we require a restart when removing an exception while adding an override is instant. But we don't think it's a risk for remote attackers and therefore does not qualify as sec-moderate and does not qualify for a bounty.

Flags: sec-bounty? → sec-bounty-
Keywords: sec-moderatesec-low

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

Odd - it was refusing connections for me, but now it seems to be working again. In any case, it looks like what's happening is the first connection opens a connection with keep-alive that gets reused after the exception is removed. There's no new (or even resumed) TLS connection.

Dragana, is there a way to reset/clear connections by hostname? Is that even something we would want to do? (context is removing certificate error exceptions)

We should post a "net:cancel-all-connections" notification. That will close, actually make non-reusable, all connection. I think that is fine for this use case.

Flags: needinfo?(dd.mozilla)
Assignee: nobody → dkeeler
Severity: -- → S4
Priority: -- → P1
Whiteboard: [psm-assigned]
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 107 Branch
Group: crypto-core-security → core-security-release
Flags: qe-verify+
Whiteboard: [psm-assigned] → [psm-assigned][post-critsmash-triage]

Reproduced the issue on Windows 10x64 with Firefox 107.0a1 (2022-10-03) by following the STR from comment 0. Visiting the webpage on a new tab after the security exception was removed will download the file again.
The issue is verified fixed with Firefox 107.0a1 (2022-10-12) on Windows 10x64 and macOS 10.15.7. Visiting the webpage on a new tab after the security exception was removed will display the warning again.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [psm-assigned][post-critsmash-triage] → [psm-assigned][post-critsmash-triage][adv-main107+]
Alias: CVE-2022-45419
Flags: sec-bounty-hof+
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: