Closed Bug 1929622 Opened 4 months ago Closed 4 months ago

Potential crash in Microsoft Entra SSO (macOS).

Categories

(Core :: Networking: HTTP, defect, P2)

Product:
Core
Component:
Networking: HTTP
Platform:
Unspecified
macOS
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
134 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- unaffected
firefox-esr128 133+ fixed
firefox132 + fixed
firefox133 + fixed
firefox134 + fixed

People

(Reporter: sekim, Assigned: sekim)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: crash, regression, Whiteboard: [necko-triaged][necko-priority-queue])

Crash Data

Attachments

(4 files)

Bug 1929622 - Handle missing device_headers in Microsoft Entra SSO cookies r=kershaw
48 bytes, text/x-phabricator-request
Details | Review
Bug 1929622 - Improve error handling in SSO Cookie Parsing r=kershaw
48 bytes, text/x-phabricator-request
Details | Review
Bug 1929622 - Improve error handling in SSO Cookie Parsing
48 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-release+
phab-bot
: approval-mozilla-esr128+
Details | Review
Bug 1929622 - Improve error handling in SSO Cookie Parsing
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review

A user shared that Firefox crashes for two users as soon as they access MS services like portal.office.com. As soon as they exclude the users from the SSO configuration, it works again.

I think the issue might be from the fact that we neglect cases when device_headers is not present.

I managed to reproduce this issue by manually popping the device_headers header from the SSO cookie (Based on the log, I noticed that the device_headers header isn't present in the cookie).

Severity: -- → S2
Keywords: regression
Priority: -- → P2
Regressed by: 1768724
Whiteboard: [necko-triaged][necko-priority-queue]
OS: Unspecified → macOS
Summary: Potential crash from Microsoft Entra SSO → Potential crash in Microsoft Entra SSO (macOS).
Assignee: nobody → sekim
Status: NEW → ASSIGNED

Set release status flags based on info from the regressing bug 1768724

status-firefox132: --- → affected
status-firefox133: --- → affected
status-firefox134: --- → affected
status-firefox-esr128: --- → affected
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a44743cb5d3e Handle missing device_headers in Microsoft Entra SSO cookies r=kershaw,necko-reviewers
Keywords: leave-open

We might need to check if the patch fixes the issue before closing the bug (however, this patch was necessary).

(In reply to Sean Kim from comment #4)

We might need to check if the patch fixes the issue before closing the bug (however, this patch was necessary).

Sean, adding a need-info as a reminder for comment 4.
This will also need uplift requests for beta, release, and esr128.
The uplift requests for release need to be in by eod tomorrow (2024-11-08) to make the Fx132 planned dot release.

Flags: needinfo?(sekim)
Flags: needinfo?(sekim)
Attachment #9436191 - Attachment description: Bug 1929622 - Check if header exists before adding the header r=kershaw → Bug 1929622 - Improve error handling in SSO Cookie Parsing r=kershaw
Pushed by sekim@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ec149762ecaa Improve error handling in SSO Cookie Parsing r=kershaw,necko-reviewers
Attachment #9436270 - Flags: approval-mozilla-release?
Attachment #9436270 - Attachment description: WIP: Bug 1929622 - Improve error handling in SSO Cookie Parsing → Bug 1929622 - Improve error handling in SSO Cookie Parsing
Attachment #9436270 - Flags: approval-mozilla-esr128?

Created a combined one for uplifting two patches.

Please request Beta approval on this as well and fill out the approval request form.

Flags: needinfo?(sekim)
Attachment #9436281 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: Some users might experience a crash with Microsoft Entra SSO (macOS) if Microsoft's broker sends a cookie without a device header.
  • Code covered by automated testing: no
  • Fix verified in Nightly: yes
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: N/A
  • Risk associated with taking this patch: Low
  • Explanation of risk level: We are only checking if a variable is null. We have already implemented this change on Nightly to confirm it works
  • String changes made/needed: N/A
  • Is Android affected?: no

esr128 Uplift Approval Request

  • User impact if declined: Some users might experience a crash with Microsoft Entra SSO (macOS) if Microsoft's broker sends a cookie without a device header.
  • Code covered by automated testing: no
  • Fix verified in Nightly: yes
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: N/A
  • Risk associated with taking this patch: Low
  • Explanation of risk level: We are only checking if a variable is null. We have already implemented this change on Nightly to confirm it works
  • String changes made/needed: N/A
  • Is Android affected?: no
Flags: needinfo?(sekim)

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

Status: ASSIGNED → RESOLVED
Closed: 4 months ago
status-firefox134: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → 134 Branch
Attachment #9436281 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9436270 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+

Comment on attachment 9436270 [details]
Bug 1929622 - Improve error handling in SSO Cookie Parsing

Approved for 132.0.2.

Attachment #9436270 - Flags: approval-mozilla-release? → approval-mozilla-release+
Blocks: microsoft-sso
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: