Closed Bug 1719964 Opened 5 years ago Closed 4 years ago

Incorrect hash validation in hawk library effecting Firefox Sync

Categories

(Cloud Services :: Server: Firefox Accounts, defect)

Firefox 89
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: chris, Assigned: dcoates)

References

()

Details

(Keywords: reporter-external)

Attachments

(1 file)

129.85 KB, application/octet-stream
Details
Attached file mitm-hawk-files.zip

+++ This bug was initially created as a clone of Bug #1719614 +++

Attachment mitm-hawk-0001 throught to mitm-hawk-0010 shows a MITM that modified the JSON payload and left all attributes of Authorization header as-is.

A new mac was not generated to sign the new modified payload by the mitm, the mitm simply modified the JSON payload and the server accepted the Authorization header and failed to detect the mismatch between the signature provided by the client before the mitm and what the server should have done is sign using the elements it is processing i.e. the server should sign the modified JSON it received (unknowingly at this stage) and the resulting signature would not match the mac provided by the client (thus detect the mitm changes).

The mitm may chance the JSON and the server should reject the entire HTTP request.

in this example the server accepted the Hawk signed HTTP request, and processed the given payload. The element that failed is not related to the vulnerability in Hawk, the fact that any elements were accepted and processed (8 in the mitm-hawk-0001 example were successfully processed) shows that Hawk Authentication was by-passed.

These show there is an Authentication bypass in the Sync service.

mitm-hawk-0011 through to 0013 are OAuth related and affects Firefox Accounts.
i.e. api.accounts.firefox.com/v1/oauth
Similar to the Sync issue, the payload was modified (added an unknown key but potentially can modify grant_type, scope, ttl which are all of auth security impacts)

I also managed to send bogus data to accounts.firefox.com/metrics using the same vulnerability, you may want to look at preventing payload modification to help keep metric data clean.

The root cause of these issues is more technically described in https://github.com/mozilla-services/hawkauthlib/issues/6
And this has a corresponding PR to fix the issues and then patches to effected systems that can resolve the issues in Firefox Sync and Account services. I imagine many other effected services exist, as detailed in the original Bug #1719614

Where this was attempted and the vulnerability was not present:

  • api.accounts.firefox.com/v1/session/verify_code
  • /v1/account/device
  • /v1/emails/reminders/cad

Anything not mentioned above was not tested and may be vulnerable.

Priority: -- → P1

Thanks for taking the time to investigate these!

Attachment mitm-hawk-0001 throught to mitm-hawk-0010 shows a MITM that modified the JSON payload and left all attributes of Authorization header as-is.

As noted in Bug 1719614 Comment 5, we're aware that the sync storage servers don't check the payload hash, and I believe we're OK with that. I'm adding Phil and JR for visibility just in case.

mitm-hawk-0011 through to 0013 are OAuth related and affects Firefox Accounts.

The Firefox Accounts servers use a different Hawk library which does not suffer the issue from hawkauthlib#6. However, even that library defaults to having payload verification off by default, and we opt in to it on routes where it has been deemed important, such as here for the /certificate/sign route.

It seems this opt-in is missing from the /v1/oauth/token and /v1/account/scoped-key-data routes, and it's probably worth adding it there for consistency (although given that we rely more on TLS than Hawk for MitM protection, I don't think it's particularly urgent). Danny, thoughts?

Severity: S1 → S3
Priority: P1 → --
Component: Security → Sync
Group: firefox-core-security → cloud-services-security
Component: Sync → Server: Firefox Accounts
Product: Firefox → Cloud Services

This bug report is not specific to the hawkauthlib, please respect the provided exploits as proof of the vulnerability in the services (Sync, Accounts, OAuth, and possible others) that will encompase the Hawk Authentication in many languages implementing the spec.

Also the spec is clear that if a hash is provided (the exploits show this is true) then the optional becomes requirement. The hash must be validated on the server if provided, specifically the signature calculation should not trust a client supplied hash and rather calculate the has and signature on the server.

Ryan, the provided proof shows these exploited paths have opted in.

Therefore, this bug report is seeking confirmation that the intention present in the services is to opt in to hash validation, and the HTTP Signature (Hawk Authentication) being the extension of the initial authentication mechanism , the use of Hawk avoids auth challenges for subsequent requests. i.e. the attacker who performs alterations to requests that Hawk fails to detect negates the purpose of the HTTP Signature (it may as well not be present at all, it no longer serves any security characteristic).

If there is no Signed Request / Hawk Authentication (due to it not performing it's security characteristics) the server trusts inherently without verification and effectively processes any request without checking if the identity is who they claim they are, i.e. the attacker does not need to have the secret used for signing the request and yet the server processes the request as though the attacker had used the secret.

If the authentication, security characteristic of signing requests with Hawk, if this is ineffective - these are effectively unauthenticated endpoints masquerading as protected by an identity that holds a secret used in authentication challenges that should occur for these endpoints.

Flags: needinfo?(rfkelly)

This bug report is not specific to the hawkauthlib, please respect the provided exploits as proof of the vulnerability in the services

Yes, I agree that these endpoints are not checking the Hawk payload hash and this is independent of the issue you filed in hawkauthlib.

Also the spec is clear that if a hash is provided (the exploits show this is true) then the optional becomes requirement.

Is it? The spec specifically says "It is up to the server if and when it validates the payload for any given request, based solely on its security policy and the nature of the data included".

the provided proof shows these exploited paths have opted in.

Sorry, I don't understand what you mean by this, could you say more?

Therefore, this bug report is seeking confirmation that the intention present in the services is to opt in to hash validation

The sync storage service deliberately does not perform payload validation.

I agree that the /v1/oauth/token and /v1/account/scoped-key-data routes should be doing payload validation, for consistency with other FxA server routes. But its absence doesn't render these endpoints completely insecure, it reduces the security to something approaching that of an OAuth bearer token (of which we use plenty in other parts of the system).

If the authentication, security characteristic of signing requests with Hawk, if this is ineffective - these are
effectively unauthenticated endpoints masquerading as protected by an identity that holds a secret used in
authentication challenges that should occur for these endpoints.

With respect, I strongly disagree with "these are effectively unauthenticated endpoints". Actually capturing one of these requests in practice requires the ability to act as a TLS MitM, and if an attacker does succeed in capturing one, they are not able to change e.g. the user identity or the request path. It would be roughly equivalent to MitM-ing my connection to Bugzilla, stealing my login cookie, and using it to access this and other security-sensitive bugs - does the presence of such an attack make Bugzilla security bugs an "effectively unauthenticated endpoint"?

I don't want to sound dismissive here, because I really do appreciate you digging in to these details. This is a good find and the FxA routes are indeed not behaving as one might expect them to.

However, I'm trying to emphasize that the practical implications for the security of the system as a whole are small - not because there isn't a bug here, but because the value that this system gets from using Hawk over e.g. a plain auth cookie or bearer token is pretty small in practice.

Quoting from the Hawk spec:

The primary design goals of this scheme are to:

   * simplify and improve HTTP authentication for services that are unwilling or unable to deploy TLS for all resources,
   * secure credentials against leakage (e.g., when the client uses some form of dynamic configuration to determine where to send an authenticated request), and
   * avoid the exposure of credentials sent to a malicious server over an unauthenticated secure channel due to client failure to validate the server's identity as part of its TLS handshake.

None of those reasons are a significant threat to Firefox Accounts or Firefox Sync as they are deployed in practice. I don't think we can reasonably call this a "vulnerability" in and of itself. It's a defense-in-depth measure that is not working quite as thoroughly as it could.

It is up to the server if and when it validates the payload for any given request, based solely on its security policy and the nature of the data included

I read this also, and wouldn't you agree that it is inauthentic if the spec algorithm states;
"includes the Hawk key identifier, timestamp, nonce, payload hash, application specific data"
directly following the 'normalized request string' to explain it.

Then in context to your own quote, it clarifies why the hash is calculated on the server "after" the mac, your quote is followed by; "If the MAC is valid, the server calculates the payload hash and compares the value with the provided payload hash in the header. In many cases, checking the MAC first is faster than calculating the payload hash."

Obviously it is inauthentic to take a quote out of context. I acknowledge the first calculation is done using the client provided value, but only because it is faster. The spec continues to state that it should continue to use the server-side calculated verification - notice it is independent of payload verification, it is just the basic specification choosing to be fast first and secure last.

Next, if the optional hash is present, the server may chose to do payload verification - this is the intent of the quote you provided.
Therefore, it appears that given the Mozilla services provided the hash - you might still choose to ignore it on the server? If that is correct, you control the implementation of both client and server, why would you have an implementation where a client sends the optional hash but the server chooses to not apply payload verification? Seems to be a charade, security smoke and mirrors.

If payload verification is not used, and TLS is insufficiently mitigating MitM attacks occuring out-of-band to TLS - as demonstrated. The bug report remains valid as long as the exploit 'works'.

Given the exploits 'work' malicious actors can take advantage of them and there is a bug bounty for reporting the vulnerability. I am in the process of submitting a CVE and want to be clear i intend to follow the agreed rules of your bounty program.

By the way, it is an out-of-band to TLS, so we are clear you mention TLS as mitigating but offer no evidence. I offer exploit payloads that show it is not requiring anything to "act as a TLS MitM" and mitigating TLS is not involved at all.

Don't just take my word for it. In the words of ‌‌Nate Lawson;
(cryptographer and software engineer who has contributed to the protocols since SSL3.0)

Data within the session should not be recoverable by anyone except the endpoints
It is that simple.

‌‌Whether you have good or bad intentions, all you need is to be either the requester or the 'receiver'. TLS is designed to let you see the 'protected' data. With zero authentication at the protocol level there are no real way to control the identity of a requester or a receiver, effectively this means in plain speak that if you have the encrypted data you can read it if you are able to perform the procedure a requester or a receiver would.

The modification of payloads occur in JavaScript, via CSRF/XSS or similar compromises of the client.
Alternatively, they may occur when the client is not a browser by SSRF or infected host by any other means.

The point is, that TLS is not in play here at all. These are exploits 'because' Hawk is ineffective in providing any of the claimed security characteristics and you have pointed out yourself that you rely on TLS alone to do something TLS is not capable of providing.. there is a misunderstanding of TLS and Hawk security characteristics in this thread.

and TLS is insufficiently mitigating MitM attacks occuring out-of-band to TLS - as demonstrated

I don't understand what you mean here, are you suggesting that something in the TLS configuration of Firefox Accounts or Firefox Sync is not sufficiently protecting against MitM attacks? This would be a very serious (but entirely separate) bug.

Obviously it is inauthentic to take a quote out of context.
[...]
I am in the process of submitting a CVE and want to be clear i intend to follow the agreed rules of your bounty program.

You seem to be imputing bad faith to my comments, perhaps on the theory that I'm trying to weasel us out of a bug bounty payment. I am simply trying to explain my perspective on the system-level security implications of the issue you describe, and I have no affiliation with or control over our bug bounty program.

I'm confident that we correctly understand the security properties of the system here, and that absent some more fundamental client or server compromise that introduces a MitM situation the safety of user data is not compromised by the missing Hawk payload verification. I will not be engaging further with this issue.

This would be a very serious (but entirely separate) bug.

No, i'm saying you are the only one mentioning TLS because you believe it mitigates this exploit.
What you are missing is TLS is not at all relevent to this discussion. Even mentioning TLS is a mitigation is misunderstanding both TLS and Hawk

I will not be engaging further with this issue.

That is an 'absolute'

Unfair because you have not shown in this thread that you have yet to even address the exploit directly - you seem to be jumping to conclusions about TLS and confusing this exploit as being somehow bound to hawkauthlib which is it completely independent from.

Can you or someone simply just look at the exploit? actually look at it and start fresh without jumping to conclusion about irrelevant TLS and python libs?

Disagree with me (my reputation) if you like, but you may not realise that you are also disagreeing with Nate Lawson and clearly have much to learn about SSL/TLS because they are not relevant when the discussion is about the security characteristics HTTP request signing (i.e. Hawk Auth).

A good read is https://tools.ietf.org/id/draft-cavage-http-signatures-08.html
Which is an attempt to standardise the same security characteristics of Hawk, it is draft so maybe you can take learning from Hawk and provide them guidance?

Essentially, HTTP request signing is not solving security concerns addressed by TLS, it solves for security concerns in spite of TLS limitations.

Apologies, this is the expired IETF used by a past employer, see the active/current iteration here https://datatracker.ietf.org/doc/draft-ietf-httpbis-message-signatures/

I quickly wanted to jump in and comment, before this gets any busier. It seems to me that there's a disconnect in acceptable and unacceptable threats that we will need to detangle.

My main focus of work is currently on Firefox and not on our services, so I might be getting things wrong.
What is our intent for using Hawk at all?
It seems to me that Chris was assuming we use Hawk to prevent & detect bugs that may occur in TLS, as unlikely as they are.
But maybe our threat model is less strong and we use Hawk solely as a session identifier?
How so, if we never check the cryptographic signatures that the user is in possession of the key?

Chris: We really appreciate the extra work you put in to understand hawk and the systems we have deployed. I think there's a disconnect in the attackers that the sync & services team is willing to accept as part of their threat model and those that you are seeing? Let's take a moment and wait for an answer, before we end up commenting over and over. Please remember that this is an asynchronous communication medium and we do not expect or even provide instant replies. I also want to remind you of our Etiquette and Contributor Guidelines.

I really believe that we can get to the bottom of it together, so I'm really curious to hear more from you @Ryan. Thanks all!

Can you or someone simply just look at the exploit? actually look at it

I looked at it in some detail before posting any replies here.

Let me walk through what I believe to be the most severe of the cases you describe, the lack of payload hash checking on the /v1/oauth/token route of the FxA server. Here's what I understand from your report:

  • In its normal operation, a Firefox Accounts client can use its sessionToken to make a Hawk-signed request to /v1/oauth/token, creating a new OAuth token grant for the account owning that sessionToken. This is a pretty powerful API, it can give access to basically any account-connected data via OAuth.
  • You instrumented your browser to intercept this request and:
    • Modified the JSON request body, inserting a new key "mitm": true for demonstration purposes. You could have equivalently modified the client_id or scope field to change the parameters of the OAuth grant.
    • Left the existing Authorization header intact, notably including hash="aAbmvEvszQ3E4xnqFEiobDx+Bwx2ffHZbKIYVLXAOrI=" which does not match the hash of the modified payload body.
  • You forwarded the modified request to the FxA server and the FxA server processed it despite the fact that the provided hash parameter did not match the hash of the modified payload body.
  • You expected that instead, the FxA server should have rejected the request as having an invalid signature.

Chris, does that accurately represent the mechanics of what you are reporting? If it does not, please provide more details, because all of my comments here are based on the above interpretation.

--

Proceeding on the assumption that the above is accurate...

I agree that the FxA server should have rejected the request as having an invalid signature, especially because we reject on invalid payload hash for other endpoints such as /v1/certificate/sign. I said as much in my initial comment back in Comment 1.

I do not agree that this is somehow equivalent to having no security on the endpoint at all. Put more concretely: I would not page any FxA devs for an emergency hotfix deploy to add this payload verification.

It seems to me that Chris was assuming we use Hawk to prevent & detect bugs that may occur in TLS, as unlikely as they are.
But maybe our threat model is less strong and we use Hawk solely as a session identifier?

This sounds broadly right, yes.

Many parts of the FxA ecosystem assume that TLS is correctly providing an authenticated connection to our servers. I believe the use of Hawk on these requests was intended to be a nice bit of extra mitigation against accidental TLS misconfiguration. There are some notes on the history here on the fxa server protocol page and also Bug 985766 (in particular Bug 985766 Comment 2).

I do understand that a proof-of-possession protocol can have security benefits besides mitigating against a TLS MitM. For example, if an FxA client were to store its sessionToken in some sort of secure enclave, then I can imagine ways in which Hawk would help guard against a client compromise that lets an attacker modify requests but doesn't give access to the enclave. None of our deployed FxA clients have anything approaching that level of sophistication in their handling of the sessionToken in practice.

Hawk is used here because at the time, it seemed like it might provide some nice extra mitigations against any hypothetical TLS problems. I don't think it has worked out particularly well for that purpose given the extra complexity it brings, but we're also not particularly motivated to remove it.

Freddy, does that help clarify?

(Danny, I'm fairly confident that we're on the same page here, but please jump in if you disagree with any of the above).

How so, if we never check the cryptographic signatures that the user is in possession of the key?

This framing doesn't seem right to me. We do still check the signature - try modifying any of the request method, request path, hawk identifier etc in the intercepted request and it will be rejected. This is specifically about checking the request body against the signature, other parts of the request are still correctly checked.

Therefore, it appears that given the Mozilla services provided the hash - you might still choose to ignore it on the server?
If that is correct, you control the implementation of both client and server, why would you have an implementation where a client sends
the optional hash but the server chooses to not apply payload verification? Seems to be a charade, security smoke and mirrors.

Calling it "security smoke and mirrors" is an uncharitable interpretation, but OK, if you're expecting this system to be using Hawk for all sorts of orthogonal security benefits beyond mitigating network MitM, I can see how you might interpret it this way. By contrast, I don't think I'd put Hawk in here at all if I were designing this thing over again from scratch. YMMV.

It is up to the server if and when it validates the payload for any given request, based solely on its security policy and the nature of the data included

wouldn't you agree that it is inauthentic if the spec algorithm states

I wasn't going to comment further in response to Comment 6, but since I'm here...no, I don't agree, in fact I think the spec is pretty unambiguous that the server can choose to validate the payload hash or not on any request for any reason.

But I'm also far less interested in "do we comply with every aspect of the Hawk spec?" than I am in "what is the concrete risk to Firefox Account users?". So just to reiterate: I think we should add the missing payload validation on the FxA server routes identified in this report, but I don't think its absence poses a significant risk to our users in practice in the meantime.

Reiterating for completeness, lest it appear I only looked at the FxA-related routes...

The provided mitm-hawk-0001 through mitm-hawk-0010 are requests to the Sync storage servers. The sync storage servers deliberately do not validate the request payload and I don't think it's worth us spending any effort to add it, because request payloads in Sync use separate client-side keys for authenticity of the submitted data (in additional to all the TLS-related reasons for FxA above).

To be absolutely clear, I do not, I have never, not even once, suggested Mozilla Services should be doing "payload verification".
This bug, it's evidence, are all about Signatures working when they should fail. Even if payload verification is not used on the server (because i cannot see this i cannot claim one way or the other). period.

Chris, does that accurately represent the mechanics of what you are reporting?

Indeed I agree with every word, however i'll only add that there was never any mention of TLS as a vector for mitm. The payload modification "mitm": true is done in the browser "prior to starting the TLS handshake" specifically. I injected some JavaScript commands (trial and error by hand) in the developer console, a malicious attacker can achieve the same before TLS via many other basic/common XSS/CSRF vectors including; malicious extensions, iframe, postMessage, workers, etc. as well as SSRF or the compromised host vector when a browser is not the client environment.

This sounds broadly right, yes.

As above, broadly, minus the mention of "TLS". I was not introducing TLS into this ticket, I only addressed TLS when it was questioned.

Hawk = nice bit of extra mitigation against accidental TLS misconfiguration

and

Hawk is used here because at the time, it seemed like it might provide some nice extra mitigations against any hypothetical TLS problems.

As mentioned, this bug report is not addressing security concerns in TLS. It is addressing flaws in "Signed Requests". TLS and Signed requests are distinct solutions for distinct security concerns. If Hawk is used it fulfils the security characteristics of "Signed Requests" and any mention of TLS when thinking about or discussing "Signed Requests" is not helpful, they are a separate as bringing up baby formula when discussing the launch of Virgin Galactic and it's implications to tourism. I say this again not to go against Etiquette and Contributor Guidelines but because I am begging the audience here to respect my education about these differences and please consider reading here the IETF draft for Request Signing: https://datatracker.ietf.org/doc/draft-ietf-httpbis-message-signatures/

Given we agree that TLS and Hawk is used here because at the time, it seemed like it might provide some nice extra mitigations against any hypothetical TLS problems.Signed requests are distinct;

the fxa server protocol page and also Bug 985766 (in particular Bug 985766 Comment 2)

These are no longer relevant because we have identified new knowledge and and acknowledged TLS and Signed requests are distinct.

None of our deployed FxA clients have anything approaching that level of sophistication in their handling of the sessionToken in practice.

They do, they use Hawk / Request Signing. The statement sophistication in their handling of the sessionToken in practice must Inherently be true, or you cannot create the MAC on the client side which leverages the secret, and that secret must be stored securely or the entire Hawk / Request Signing falls apart and may as well be removed entirely as it serves no security characteristic purposes if the creation of the MAC on the client side is not secure

This is specifically about checking the request body against the signature, other parts of the request are still correctly checked.

That is slightly true, however inauthentic. It must be acknowledged that the Signature check on the server always has a requirement of including the 'hash' attribute when it exists. Per the API;

Client produces the hash and "signs" https://github.com/mozilla/hawk/blame/main/API.md#L261-L278
Note: hash is in the Signature, Signatures are always always verified on the server - as the base use of Hawk

phase 1: https://github.com/mozilla/hawk/blame/main/API.md#L291-L292
if the server wants to optionally do hash verification first

phase 2, directly following above, https://github.com/mozilla/hawk/blame/main/API.md#L294-L296

If the payload is available at the time of authentication, the server uses the hash value provided by the client to construct
the normalized string and validates the MAC

So in phase 2 it is ok for the server to use the client provided MAC, but if they do not match we early exit

If the MAC is valid, the server calculates the payload hash and compares the value
with the provided payload hash in the header.

If the client provided MAC is VALID we still need to have server-side validation of the MAC "Signature" this is not payload validation, it is still the Signature / MAC

Given that is how Hawk should behave, that is not how we are observing the OAuth interactions.

Calling it "security smoke and mirrors" is an uncharitable interpretation, but OK, if you're expecting this system to be using Hawk

Apologies for being uncharitable, I am frustrated with the confusion with other async activities you are continuing to introduce.
This bug report is not about "users of Hawk" as you are getting confused with, it is about Mozilla Sync, Account, and OAuth having a vulnerability. I can disregard the digression into the uses of libraries. I've asked many times to please not digress about other users of Hawk because that is confusing async activity that has no correlation, only causation, to the Mozilla Sync, Account, and OAuth vulnerabilities. Can we please not confuse open source activities with this Bug report in future exchanges?

these are related;

I think the spec is pretty unambiguous that the server can choose to validate the payload hash or not on any request for any reason

and

But I'm also far less interested in "do we comply with every aspect of the Hawk spec?"

So you acknowledge that Signature (includes hash if provided, always), payload verification is optional before Signatures? And you agree that the basic use of Hawk is Signatures? I am "only" talking about "Signatures" and have tried so many times to avoid any mention of optional "payload verification". the basic issue is in Signature check, without the Signature check there is no Hawk.

I reiterate:

To be absolutely clear, I do not, I have never, not even once, suggested Mozilla Services should be doing "payload verification"
This bug, it's evidence, are all about Signatures working when they should fail. Even if payload verification is not used on the server (because i cannot see this i cannot claim one way or the other). period.

Can we please have a fresh start? Please do not bring TLS into this, and no mention of payload verification. These are not part of my report. I am only reporting a failure in Signature checking.

This is specifically about checking the request body against the signature, other parts of the request are still correctly checked.

That is slightly true, however inauthentic.

I don't understand what you mean by "inauthentic" here. It comes across as though you're accusing me of being deliberately tricky with my language, and between this and the conversation in github it has been an emotionally exhausting exchange to feel like we're not engaging in good faith.

It must be acknowledged that the Signature check on the server always has a requirement
of including the 'hash' attribute when it exists.

Yes, and they do. The signature check always incorporates the hash attribute when it is provided by the client.

I am "only" talking about "Signatures" and have tried so many times to avoid any mention of optional "payload verification".
the basic issue is in Signature check, without the Signature check there is no Hawk.
[...]
To be absolutely clear, I do not, I have never, not even once, suggested Mozilla Services should be doing "payload verification"
This bug, it's evidence, are all about Signatures working when they should fail.
[...]
Please [...] no mention of payload verification.
These are not part of my report. I am only reporting a failure in Signature checking.

I do not understand the distinction you're trying to draw here. The signatures are checked and the provided request traces do not show otherwise. Specifically, the signatures are checked as follows:

  • We take the id parameter from the Authorization header and use it to look up the corresponding secret key.
  • We take the request method and host and path, along with ts and nonce and hash from the Authorization header, and form them into a normalized request string.
  • We calculate a MAC over this normalized request string using the secret key.
  • We check whether the calculated MAC matches the value provided in the Authorization header, and reject the request if it does not.

In the provided request traces, the values involved in this calculation have not been modified from their original values, so the signature is still valid and it still accepted by the server.

From your report, I interpret that you expect a change in the request body to have invalidated the signature, because the hash of the request body does not match the hash used in the signature. But checking whether the provided hash matches the request body is exactly the operation that Hawk refers to as "payload validation", and the server opting not to do payload validation is AFAICT the only reason that your modified requests are succeeding.

phase 1: https://github.com/mozilla/hawk/blame/main/API.md#L291-L292
if the server wants to optionally do hash verification first

I cannot see any interpretation of this paragraph other than "it is up to the server if and when it performs the validation steps in the paragraphs that follow". Indeed, the fourth paragraph is a specific warning to implementers about the security implications if they decide not to do it, a paragraph which would be entirely unnecessary if the server did not have discretion here.

As far as I can tell, your interpretation that the protocol must somehow incorporate checking of the incoming request payload as part of checking the signature is simply not correct, and I don't think there is value in discussing it further.

The request traces submitted in this bug do not represent a failure of signature checking.

As far as I can tell, your interpretation that the protocol must somehow incorporate checking of the incoming request payload as part of checking the signature is simply not correct, and I don't think there is value in discussing it further.

The request traces submitted in this bug do not represent a failure of signature checking.

I concur.

the server should sign the modified JSON it received (unknowingly at this stage) and the resulting signature would not match the mac provided by the client (thus detect the mitm changes).

This is not how hawk works. Yes, the modified JSON body has a different hash from the hash value provided in the hawk header. In hawk checking whether the hash value matches the actual payload hash is not part of validating the mac. It is a separate, optional, operation.

Message received loud and clear.

Hawk provides only client-side assurance of message integrity by default, server assurance of secure MAC Signatures are optional.

This is how Hawk works today, I agree. Clearly I know this because that is the nature of the Bug I submitted.

To be clear you're telling me that because Hawk currently provides no server assurance by default currently that Hawk is working as intended?

So I assume that given Hawk is not supposed to provide server-side, you're stating the root cause of the exploit works as intended and the system works as designed? The exploit is by-design?

If you see no reason to acknowledge the ezploits are a vulnerability I assume I am permitted to publicly disclose my findings? They are as you say, the system work as designed..

You are welcome to post you're findings. If you haven't already I encourage you to read the hawk source code around authenticating a request to understand what is considered a valid signature versus a valid payload: https://github.com/mozilla/hawk/blob/main/lib/server.js#L156-L176

Thank you Danny.


regarding Node.js source

I have read the source code, Ryan and I have had numerous conversations from our reviews already.


To catch Danny up on our learnings

To address the lines you submit for me to review above, you should follow Crypto.calculateMac() (you linked it, you should read what you linked)
https://github.com/mozilla/hawk/blob/main/lib/crypto.js#L44

It leverages generateNormalizedString()

https://github.com/mozilla/hawk/blob/main/lib/crypto.js#L54

and included options.hash !! for the calculateMac ..
https://github.com/mozilla/hawk/blob/main/lib/crypto.js#L71

As you can see, it will of course depend on where options originates to determine if there is any assurance of a security controls provided. So we follow your link of artifacts (which is passed as options to Crypto.calculateMac()), and we see artifacts populated artifacts.hash using attributes.hash.
https://github.com/mozilla/hawk/blob/main/lib/server.js#L113

So where is the origin of attributes? we need the origin to gain insight to the assurance we have..

https://github.com/mozilla/hawk/blob/main/lib/server.js#L102

This shows that we take the client provided values to populate attributes!

I.e. the server-side MAC calculation uses ONLY client provided values to calculate the MAC Signature.

I agree and always have agreed that this is the current state, I consistently agree this is true.

What we disagree on is the assurance of security controls.


Why is this a security concern or a Bug in my mind?

It is the very nature of this in both JavaScript and Python that we gain no assurances on the server of any security controls by default however the Hawk API describes it is a derivitive of IEFT draft-hammer-oauth-v2-mac-token and takes many sections almost verbatum from the draft:
https://datatracker.ietf.org/doc/html/draft-hammer-oauth-v2-mac-token#section-1.1

Hawk follows the section 3.2 also;
https://datatracker.ietf.org/doc/html/draft-hammer-oauth-v2-mac-token#section-3.2

In the IETF draft and the Hawk API document, both state if the hash is present it must be calculated on the server before the MAC because the MAC must include the calculated hash (if it was provided).

This bug report assumed the intent was to perform server-side calculation to gain assurance of the MAC Signature on the server as both the Hawk API and the IETF draft described.

for clarity: Hash represents integrity - if provided the integrity check is part of the MAC.
Both IETF draft https://datatracker.ietf.org/doc/html/draft-hammer-oauth-v2-mac-token#section-4 and Hawk API describe this https://github.com/mozilla/hawk/blame/main/API.md#L277

The exploit examples show the hash was sent, and the exploit exists because the server did not include the provided hash into the MAC calculation. If it had, the alteration would be detected as Ryan has acknowledged.

So we have the Hawk API intent not match the implementation. We also have the source spec (Hawk is almost a verbatim derivative of) not match the implementation. And the system implementation is working as designed, any provided exploits are not valid because the system design allows these functionalities as expected uses of the system.

Therefore this Bug report claims the system is by-design different from the Hawk API intent, and works as intended


I'll summarise what you are saying is true about Hawk, (using your logic not my opinion);

Hawk leverages MAC signatures generated on a client and trusted by a server, the server only uses client provided values to confirm the client values are valid and signed using a shared secret. Hawk provides no server-side assurances that any part of a request has been modified after the MAC was signed by the client. If assurances of Hawk MAC matches a given request is a server requirement, then the server may apply optional payload validation. An undocumented security characteristic of Hawk is: **If the server security posture and system design mandates, Hawk on the server may also circumvent the client integrity assurance mechanism by explicitly ignoring payload hash calculation on the server and inclusion in MAC verification when a client provided the hash and expected payload verification.


My reflection on my own mistaken knowledge

Humbly, It has been my mistake.

We all have always agreed on the documented Hawk use case.

I was unaware that there was an undocumented nuance, and it was my mistake to assume the nuance was a Bug

From a trained security professional who has achieved IRAP compliance, worked with APRA to grant banking licenses on 4 occasions, led new and reoccurring PCI DSS and PCI PA compliance projects, Achieved 10+ ISO certifications, implemented controls to achieve SOC2 Type II, and contributed to OWASP, CSA CCM v4, AWS SDK botocore crypto libraries, and architected 50+ organisations for hardened security posture as a consultant.

It was my misunderstanding given these professional security experiences, that Hawk security characteristics allows a server to also circumvent client expectations.
I made this mistake because it does not adhere to my professional experience, and that is on me.

I could not imagine that the client expectations are not met, so there is also no client-side assurance.. When there is also no server-side verification performed distinct from client provided values (meaning the server has inherent trust in the client) and no assurance of security controls has been gained on the server or the client. This seems to be a solution looking for a problem, because it offers no security characteristics by-default that I can derive from your responses.


Conclusion of this bug report

  • Hawk clients did provide the hash
  • Hawk servers (Sync, Accounts, OAuth endpoints) by-design decide that the hash is not needed to be calculated on the server (as stated in the Hawk API and derivative IETF draft) at all even when clients provide them
  • It is the system design decision to allow payload modification of a Hawk request, there is no convincing you these exploits prove a vulnerability you acknowledge is a threat
  • Hawk will not be patched to provide server-side assurance by-default, this would contradict the 3 design decisions above. Therefore Hawk is by-design a client assurance solution, and optionally may be used for server assurance regardless of the client's indication it should provide message integrity.
  • The system will not change it's design decision given the exploits provided?

There are no objections to the above?

If we agree on these, i'll accept the outcome that you acknowledge these exploits are a valid vulnerability.
And I accept that you will leave the system operating as-is, because it is intentional and expected as part of the system design that these functionalities exist so they will remain unchanged and allow operations.

The exploit examples show the hash was sent, and the exploit exists because the server did not
include the provided hash into the MAC calculation. If it had, the alteration would be detected as Ryan has acknowledged.

I don't think we disagree on the technical details here, but I want to be as clear as possible.

I do not agree with this interpretation of what I said. You seem to be using "provided hash" here to mean "the hash of the request body as received by the server". In my humble opinion, the only interpretation of "provided hash" which is consistent with both the rest of the document and with the code that it documents is "the hash value provided by the client".

The specific thing that the server did not do was it did not check the provided hash against the actual request body, and indeed if it had done this then the alteration would have been detected.

Hawk provides no server-side assurances that any part of a request has been modified after the MAC was signed by the client

Again, I don't think we disagree on the technical details, but I want to be as clear as possible: saying "any part of the request" does not accurately convey what's happening here.

The Hawk MAC validates several important parts of a request including the request method, path, hostname and port.

It also validates an independent client-provided hash of the request body, which the server may choose to then separately validate (or not) against the incoming request body depending on its security requirements.

If the server security posture and system design mandates, Hawk on the server may also circumvent the client integrity
assurance mechanism by explicitly ignoring payload hash calculation on the server and inclusion in MAC verification when
a client provided the hash and expected payload verification.

I think I agree with what you're saying here. Let me see if I can reword it into my own mental model: there is no way for the client to insist that a Hawk server reject a request if the request body has been modified in transit, this is purely at the discretion of the server.

Does that sound right?

There are no objections to the above?
If we agree on these, i'll accept the outcome that you acknowledge these exploits are a valid vulnerability.

As above, I don't think we disagree on any technical details, but I want to be as clear as possible...

I personally do think it's useful to use the words "exploit" and "vulnerability" to describe a known and expected behaviour of the system that doesn't affect its intended security model.

That Hawk makes payload validation optional could certainly be described as a weakness of the protocol, and using it in a system where that weakness matters to the threat model could certainly be described as a vulnerability. Fine, yes, I agree. AFAICT we both also agree that this isn't what happened here, because the threat model for the system is not one where this weakness makes a material difference to security.

This probably sounds like I'm playing some sort of semantic game with words here, and I won't pretend to be an expert in terminology. But the difference between these two things really matters to me on a personal and professional level:

  • "There is an Authentication bypass in the Sync service"
    • I will (and did) drop literally anything else I am doing to investigate the issue immediately.
  • "There is a weakness in the Hawk protocol that makes it unsuitable for some usecases"
    • I understand, but those are not our usecases, and this weakness exists as a deliberate tradeoff to make the protocol a better fit for other usecases.

Whether and how that difference matters to our bug bounty, CVE process, etc etc, I personally don't really know or particularly care. That difference is what matters to me when deciding how to deal with this issue from an engineering perspective.

Just my 2c. Word are hard, they can convey so much difference in meaning to different people.

The system will not change it's design decision given the exploits provided?

I think Danny and I have talked this out as much as we can from an engineering perspective, and indeed we do not intend any changes to the design of the system. We do intend a specific change to the FxA server routes that you identified, to enable payload verification in cases where its omission was accidental. Let's do that in a public bug over in https://github.com/mozilla/fxa.

Chris, thank you for investigating this system and for uncovering the missing payload verification on those two routes.

Freddy, do you have what you need for further handling of this bug by the security team? If you need any more input please needinfo me.

Flags: needinfo?(fbraun)

It seems we are aligned, agree, and have an understanding.

Toneless text aside, this has been enjoyable.

Freddy, it appears this validates a bounty for authentication bypass. Includes IDORs that bypass authentication or authorization for significant actions

Per Ryan;

We do intend a specific change to the FxA server routes that you identified, to enable payload verification in cases where its omission was accidental.

This would confirm the Bounty, I identified the authentication bypass, produced IDORs, you've acknowledged there's a plan to remove the exploitability in the system by;

enable payload verification in cases where its omission was accidental

These are in the OAuth provider (FxA Server), a significant system.

Therefore you've identified the validity of the report, for specific routes where this exploit makes the system vulnerable unintentionally or by mistake.

Thank you for your attention and clarification on this Ryan! I've already prepared analysis of FxA to share elsewhere.

(In reply to Ryan Kelly [:rfkelly] from comment #22)

Freddy, do you have what you need for further handling of this bug by the security team? If you need any more input please needinfo me.

Almost. Ryan, can you please make sure that the issue you are creating over at the public fxa repo will be mentioned in this bug's "See Also" field?

(In reply to Chris from comment #23)

Freddy, it appears this validates a bounty for authentication bypass. Includes IDORs that bypass authentication or authorization for significant actions

I am setting the flag to nominate this for bounty considerations. Naturally, this is at the discretion of the bounty panel, which meets about once a week but a bit less reliably frequently during the summer. Note that we often times wait until a fix is present as it makes it much, much easier to fully judge and understand the severity and implications of a specific report. I realize this will require your patience, but the general intent is to be able to argue in favor or the participant's case, not against. This is in accordance to the various notes you will find in https://www.mozilla.org/en-US/security/bug-bounty/faq/ and might help you get a better feeling of the process.

We do intend a specific change to the FxA server routes that you identified, to enable payload verification in cases where its omission was accidental.

This would confirm the Bounty, I identified the authentication bypass, produced IDORs, you've acknowledged there's a plan to remove the exploitability in the system by;

He didn't really say that and please, let's not jump to conclusions.
I'm OK with arguing on the grounds of technical details, but twisting an open statement into a very specific direction in favor of your case will just make everyone less likely to comment frankly and amicably... Let's not do that.

I've already prepared analysis of FxA to share elsewhere.

As you may or may not have seen on https://www.mozilla.org/en-US/security/bug-bounty/faq/#nondisclosure, we do not require you to keep this confidential. That said, we strongly prefer that security sensitive information is kept private until a fix has shipped to ensure that our users are less likely to be harmed.

Flags: sec-bounty?
Flags: needinfo?(rfkelly)
Flags: needinfo?(fbraun)

By elsewhere I was referring to the original linked bug, there will be no disclosure by me without permission.

Let's not do that.

I may be hastily concluding here, but I regard my text as a question pose to you (i.e. favor when you are ready to grant it) so I am puzzled you missed the questioning nature and saw it as a twisting of words... It was not, i posed it to you to respond.

Almost. Ryan, can you please make sure that the issue you are creating over at the public fxa repo will be mentioned in this bug's "See Also" field?

Danny, could you please take care of filing this as a public bug?

Flags: needinfo?(rfkelly) → needinfo?(dcoates)

Any update from the bounty panel these past 3 weeks?

(In reply to Chris from comment #28)

Any update from the bounty panel these past 3 weeks?

No update. https://github.com/mozilla/fxa/issues/9850 is open and the bounty panel usually waits until the issue is fixed before making hall of fame and bounty decisions. Freddy mentioned this in comment 24 too.

This has been merged for nearly 4 months and I'd appreciate an update please.

Chris: It looks like this slipped through the cracks as it was never properly resolved or even assigned.
Danny, can we consider this closed or is there something else besides the linked pull request?

Assignee: nobody → dcoates
Flags: needinfo?(dcoates)

Yes, this can be closed.

Flags: needinfo?(dcoates)
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Any update on the bounty process?

Flags: needinfo?(fbraun)

I'm expecting this to be picked up by a bounty meeting soon (usually on Mondays). It's just that most regular meetings were skipped during the holiday season.

Flags: needinfo?(fbraun)
Flags: sec-bounty?
Flags: sec-bounty-hof+
Flags: sec-bounty-

This is great news! thank you.

Some suggested CWEs for the CVE details;

  1. CWE-807: Reliance on Untrusted Inputs in a Security Decision
    directly referring to the FxA patches.
  2. CWE-642: External Control of Critical State Data
    references the external element of an HTTP Desync attack to modify the payload undetected prior to the patch, or possibly XSS/CSRF via malicious Ad, js lib, or plugin) as vector/s because the outcomes were related to OAuth and token service that are clearly critical

I suggest CVSS v3.1 Vector AV:A/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:N base score 9.3

Rationale AV:A is the HTTP Desync vector, where as the browser environment based are merely local.
Rationale I:H being the integrity checking for authentication was skipped before the patch, now it exists and completely mitigates the malicious modification of payloads without knowledge to the secret used for signing.
Rationale C:H is the attacker is able to masquerade as the user after compromising OAuth or the token service and retrieve the userdata or perform actions as the user

By all means use of this what you will

Hi, let's get this resolved
Some motivations from my end:

  1. I prefer to just have HOF, even if payout can be passed over and not actually paid out
  2. Any bounty pay outs will be donated 100% anyway
  3. Let's get the CVE reserved, I have no intention to pressure any specific CVSS whatsoever, just trying to help
  4. what can I do to help? Willing to do whatever is necessary
Flags: needinfo?(rose)
Flags: needinfo?(fbraun)
Flags: needinfo?(rose)
Flags: needinfo?(fbraun)

With Danny Coates no longer available, Rose and Frederik ignoring the request, Ryan would you be able to help resolve the outstanding?

Flags: needinfo?(ryan)

I am no longer employed by Mozilla or involved with the project in any significant way, so I don't think I can be of any help here.

Flags: needinfo?(ryan)

So. first of all: Thank you for working with us and providing your insights to help secure our software.

Last year (below comment 34), it's been decided that there should be a Hall of Fame entry. If that has not been created, then please accept our apologies. I will find out what happened here.
At the same time, it has also been decided that this bug is not going to be awarded a bounty. Looks like there is disagreement on the severity of this issue and it has still not been fixed on the hawkauthlib. Given this disagreement, I don't think we're very inclined to issue a CVE (we also don't use CWE or CVSS).

Hello Chris,

This report must have been missed when collecting the list of HoF. How would you like to be included in the HoF?

Thanks,
Frida

Flags: needinfo?(chris)

Yes, thank you Frida. That would be fantastic.

Flags: needinfo?(chris)

would you like us just to use your name? would you like to refer to a link?

Thanks,
Frida

The name as shared her is perfect (Chris Langton) and the link can be https://www.trivialsec.com
Cheers Frida

(In reply to Frederik Braun [:freddy] from comment #39)

Looks like there is disagreement on the severity of this issue and it has still not been fixed on the hawkauthlib. Given this disagreement, I don't think we're very inclined to issue a CVE (we also don't use CWE or CVSS).

We've established that the hawkauthlib is likely abandoned; https://github.com/mozilla-services/hawkauthlib/issues/6#issuecomment-1571446802
That's the reason for "still not been fixed".

An earlier comment in that thread makes it clear that Ryan had little security domain knowledge, he argues for performance only. I found evidence of Mozilla doing the right logic elsewhere, matching the logic in my PR, and other former Mozilla employees agree with the inherent intent of a HMAC-SHA256 supporting my points and not Ryan's. There's also an alternative in Python called mohawk that agree's with my PR and not Ryan's.

While we are in context for the CVE, it is my understandign a CVE can be created regardless of the severity, the CVE will be given a lower or higher severity, the point is there is a vulnerability or an exploit - the severity rating is not a reason to not issue a CVE, right?

All of that argument, interpretation, and opinion put aside.

Is it fair to say that if my PR is merged, bringing hawkauthlib in line with other Go and Rust logic for Hawk, then the CVE for hawkauthlib is acceptable?

Flags: needinfo?(fbraun)

And if I'm to be pedantic again, please keep hawkauthlib discussion with it's own ticket 1719614. Those issues are not resolved (PR not merged).

This ticket 1719964 is for issues (FIXED) in FXA

OK, to keep this focused: This bug will not get a CVE. We do not consider Firefox Accounts some sort of software that we expect people to run on their machines and I don't think we'd be willing to take patches for hawkauthlib. We should mark it as archived and deprecated.

Flags: needinfo?(fbraun)

Understood, I'll stop commenting once the HoF is published - thank you
Any ETA on that?/

Group: cloud-services-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: