Closed
Bug 578083
Opened 15 years ago
Closed 15 years ago
Enhance CEF Logging for Password Reset Tampering Detection
Categories
(Cloud Services Graveyard :: Server: Sync, defect)
Cloud Services Graveyard
Server: Sync
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mcoates, Assigned: telliott)
Details
(Keywords: push-needed)
Attachments
(3 files)
534 bytes,
patch
|
lorchard
:
review+
|
Details | Diff | Splinter Review |
1.13 KB,
patch
|
lorchard
:
review+
|
Details | Diff | Splinter Review |
1.52 KB,
patch
|
tarek
:
review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
Flags: blocking-fx-sync1.4?
Reporter | ||
Comment 1•15 years ago
|
||
I saw the block for 1.4 was denied. Do you have any estimates when this change could be made?
Assignee | ||
Comment 2•15 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.
Comment 3•15 years ago
|
||
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•15 years ago
|
||
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)
Updated•15 years ago
|
Attachment #456955 -
Flags: review?(lorchard) → review+
Assignee | ||
Comment 5•15 years ago
|
||
Code is in http://hg.mozilla.org/services/reg-server-secure/rev/f02c78032e98
Zandr - push to stage?
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 6•15 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•15 years ago
|
||
Attachment #458746 -
Flags: review?(lorchard)
Updated•15 years ago
|
Attachment #458746 -
Flags: review?(lorchard) → review+
Assignee | ||
Comment 8•15 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Keywords: push-needed
Resolution: --- → FIXED
Reporter | ||
Comment 9•15 years ago
|
||
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•15 years ago
|
||
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)
Reporter | ||
Comment 11•15 years ago
|
||
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•15 years ago
|
Attachment #462537 -
Flags: review?(tarek) → review+
Assignee | ||
Comment 12•15 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
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•