Enhance CEF Logging for Password Reset Tampering Detection

RESOLVED FIXED

Status

Cloud Services
Server: Sync
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mcoates, Assigned: telliott)

Tracking

({push-needed})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Please enhance the CEF logging for tampering of the password reset token to include both the token submitted within the HTTP request and also the valid token stored on the server.  This modification will allow us to investigate these events and determine if the user is iterating token values, inserting attacks or simply repeatedly pasting the wrong value.

The new data can be added as custom CEF fields with the names "submittedToken" and "expectedToken".
Flags: blocking-fx-sync1.4?

Updated

8 years ago
Flags: blocking-fx-sync1.4?
I saw the block for 1.4 was denied. Do you have any estimates when this change could be made?
(Assignee)

Comment 2

8 years ago
it's pretty easy - the only trick is to make sure that it only happens on the internal server, since we don't want to log that on the external box

I should be able to get it in by end of week.
1.4 was cleared, because 1.4 was two weeks ago.

We need to get some server versioning/roadmap goodness in play.
(Assignee)

Comment 4

8 years ago
Created attachment 456955 [details] [diff] [review]
Logging the submitted reset code

Logging the submitted reset code. 

Getting the expected code turns out to me trickier, since we don't actually grab it to do a compare - we just send the submitted one to MySQL and get back the username if it matches. We can get it, but it'll involve writing more queries, and after talking with mcoates, we're going to hold off until we decide we need it.
Attachment #456955 - Flags: review?(lorchard)
Attachment #456955 - Flags: review?(lorchard) → review+
(Assignee)

Comment 5

8 years ago
Code is in http://hg.mozilla.org/services/reg-server-secure/rev/f02c78032e98

Zandr - push to stage?
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

8 years ago
Server verifies the code before sending the full request back. That means the first one we logged is short-circuited
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 7

8 years ago
Created attachment 458746 [details] [diff] [review]
Adds logging of the bad reset code to the actual reset code check
Attachment #458746 - Flags: review?(lorchard)
Attachment #458746 - Flags: review?(lorchard) → review+
(Assignee)

Comment 8

8 years ago
Fixed in http://hg.mozilla.org/services/reg-server/rev/cc70a9fe3926
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Keywords: push-needed
Resolution: --- → FIXED
It looks like our CEF code is incorrect for custom strings. Each custom
string needs to be identified with cs#Label=name and then cs#=value.
(where the # is a number of course)

E.g.

cs1Label=requestClientApplication cs1=none
cs2Label=submittedToken cs2=abcFU4O-D4EG-K0K0-2UFL


Currently, we are doing this correctly for the first custom label
(requestClientApplication) but we are not doing it for the second custom
label.

This is what we are currently receiving:

CEF:0|mozilla|weave|1.3|InvalidResetCode|Invalid Reset Code
submitted|5|cs1Label=requestClientApplication cs1=none
requestMethod=POST
request=https://auth.services.mozilla.com/user/1.0/mcoatestest/password
src=63.245.208.134 dst=10.8.74.253 suser=mcoatestest
submittedToken=abcFU4O-D4EG-K0K0-2UFL


Instead, it should be this:

CEF:0|mozilla|weave|1.3|InvalidResetCode|Invalid Reset Code
submitted|5|cs1Label=requestClientApplication cs1=none
requestMethod=POST
request=https://auth.services.mozilla.com/user/1.0/mcoatestest/password
src=63.245.208.134 dst=10.8.74.253 suser=mcoatestest
cs2Label=submittedToken cs2=abcFU4O-D4EG-K0K0-2UFL



So it looks like we need to update CEF.php to correctly handle
additional custom strings in this format.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

8 years ago
Created attachment 462537 [details] [diff] [review]
Fix to label parameters according to the cs* format

This will label the parameters, but it's possible it will incorrectly flag one of the reserved-word parameters as not known and give it its own label. Please let me know if you see any.
Attachment #462537 - Flags: review?(tarek)
I double check the CEF documentation before recommending a name for a particular custom string. So we should be ok there and avoid accidentally using reserved words.

Updated

8 years ago
Attachment #462537 - Flags: review?(tarek) → review+
(Assignee)

Comment 12

8 years ago
cef labelling behavior added in 

http://hg.mozilla.org/services/reg-server/rev/9b75e64df258
http://hg.mozilla.org/services/reg-server-secure/rev/7bc9f1773e43
http://hg.mozilla.org/services/sync-server/rev/0fb58f31175a
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.