The incoming hash of the payload is being trusted and not verified

[chrisdlangton]

This line of code

hawkauthlib/hawkauthlib/utils.py

Line 154 in 706d498

bits.append(params.get("hash", ""))

simply takes the hash provided by the client when calculating the server-side verification.

what should happen?

The server should read the raw payload + content-type and calculate it's own hash to compare with the client provided value.

Why is this a problem?

A malicious actor in the middle can alter the payload and the server side will not identify the modification occurred because it simply uses the client provided value instead of verify the hash provided against the modified payload.

[chrisdlangton]

There doesn't appear to be any tests for generating or verifying that the optional payload validation i.e. specified via Authorization: Hawk hash="sha256( payload+content-type )" which may have led to this issue not being detected earlier.

Many implementations may not use the optional feature and if they do many would not have audited this library to ensure the optional feature works as intended i.e. implements the security characteristics defined in the spec.

[rfk]

Thanks for digging into the details here, and for the linked PR which I will take a look at shortly.

The readme does mention that "Calculating or verifying payload hashes" is not supported, and the linked spec does say "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".

However, I do see a reasonable argument that users of the library could expect it to "fail closed" and reject any request with a payload hash, rather than "failing open" and just ignoring the provided hash.

[chrisdlangton]

is not supported

This is commentary only, in fact the signature is being calculated i.e. the signature is supported and includes the hash contrary to the quoted commentary. The obvious thing here is that the commentary is written by someone who ignores that the hash is included in the signature if provided therefore stating the hash is not supported is untruthful/unauthentic

And worse, it implies the signature calculation is not following the specification - effectively when the signature is calculated using client provided values the entire purpose of Hawk Authentication is fundamentally broken, effectively not using Hawk at all and using it like this is identical from a security characteristics perspective. Taking the client provided values and trusting them provides zero assurances and is not following the Hawk Authentication specification at all.

[rfk]

Just to be clear up-front: absolutely this library should let consumers perform proper payload validation, this is a missing feature and is a potential security footgun for consumers if they assume that payload validation is being performed automatically.

it implies the signature calculation is not following the specification -
effectively when the signature is calculated using client provided values the entire purpose of Hawk Authentication is
fundamentally broken

The canonical nodejs implementation checks the signature using the client-provided hash value and then optionally independently checks whether the provided hash matched the actual payload, either by using the payload provided by the caller or by letting the caller check the hash themselves. AFAICT there is no requirement to calculate the signature using a hash of the received payload, and notably, it requires the caller to opt in to the payload checking either way.

effectively not using Hawk at all and using it like this is identical from a security characteristics perspective

Please stop with this hyperbole; being able to replay a captured request with different payload is very different from having no authentication at all.

[chrisdlangton]

Saying hyperbole it not constructive, it is a plain attempt to discredit the reporter and frankly a rude and unethical way to avoid the truth.

I read this quote of yours 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..

Your quote is taken out of context.. Your own quote continues in context to clarify why the hash is calculated first with the client provided values;
"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 your quote was 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 then 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, specific to payload verification (if the optional hash is present) the server may chose to do payload verification - this is the intent of the quote you provided when you actually read the context.

Simply using client provided values, phase 1, and ignoring the default phase 2 and 3 seems to be a security charade and this is not actually performing the basic expectation of Hawk spec at all in it's current form. Not hyperbole, just fact.

[chrisdlangton]

Look at mohawk who follow the spec properly

[rfk]

It's clear that we disagree on the severity of this issue, so I wanted to make another attempt at explaining where I'm coming from here without attempting to language-lawyer anything from the spec. After all, per the spec you linked, the code is the specification.

The Hawk.server.authenticate function in the reference implementation behaves as follows:

• Parses the MAC artifacts from the Authorization header.
• Looks up the credentials based on the client-provided id.
• Calculates the expected signature using the client-provided artifacts, including the client-provided hash value.
• If and only if the caller provided the options.payload value, checks whether the client-provided hash value matches the received payload.

The default behaviour of the reference implementation is to accept the client-provided hash value, unless the caller specifically requests that it be checked against the actual request payload. The docs for this function mention that you can validate it separately if you don't want to pass it in, but they don't insist that you do so.

The usage example from the spec document calls Hawk.server.authenticate without providing a request payload, and without any indication that it needs to be checked out of band.

All of this leads me to believe that it is intentionally left up to the server whether to validate the provided hash parameter, if any.

Should this library provide the ability to validate the hash? Absolutely yes, thank you for contributing it! Should validating the hash be opt-out rather than opt-in? Probably yes, that seems like a reasonable thing to avoid footguns, let's make it happen. Would Hawk be better if it mandated checking the hash rather than leaving it up to the server's discretion? Maybe, I don't really have an opinion on that.

But I do not accept that this missing feature is "identical from a security characteristics perspective to not using Hawk at all" given that we behave identically to the reference implementation in its default configuration.

Next, specific to payload verification (if the optional hash is present) the server may chose to do payload verification

• this is the intent of the quote you provided when you actually read the context.

I don't understand what you mean here - what exactly is the server choosing to do or not do under your interpretation?

[chrisdlangton]

I may have to step back from this security fix PR as it is clear that Hawk's security characteristic is and will always be defined and controlled by the client rather that give assurances to the server. If the server only trusts client values and only validates MAC Signature when a client directs it too, then Hawk is a solution to provide the Client assurances not the server.

these led me to the above conclusion;

the code is the specification.

in the reference implementation behaves as follows

(conveniently ignoring If the MAC is valid

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

i.e. If the client provided MAC is VALID it still needs to have server-side validation of the MAC "Signature" which is Signature not "payload validation", it is still the MAC.

so you promote client-side assurance step, but stop. You skip "If the MAC is valid" and anything after this for unknown reasons... But I have to accept that you believe this and i'll close the PR that implements ONLY what should happen AFTER "If the MAC is valid"..

The default behaviour of the reference implementation is to accept the client-provided hash value

client-side assurance

unless the caller specifically requests that it be checked against the actual request payload.

client-side assurance

---

I'm guessing you meant "client" not server when you wrote All of this leads me to believe that it is intentionally left up to the server ?? if it was truly left up to the server all previous statements about the client no longer make sense as they are a contradiction. if you truly mean "server" then how can you clarify the balance that? you must be thinking differently to the user journey above? how does that client/server conflict fit into or effect these journeys?

---

Assuming this PR stays

Should this library provide the ability to validate the hash? Absolutely yes, thank you for contributing it!

no problem, it's been fun - contrary to "emotionless" text might seem..

Should validating the hash be opt-out rather than opt-in? Probably yes, that seems like a reasonable thing to avoid footguns, let's make it happen.

Actually, i've never suggested payload validating is opt-out. I believe it should be opt-in or user journey 2 & 3 are not possible...

Would Hawk be better if it mandated checking the hash rather than leaving it up to the server's discretion? Maybe, I don't really have an opinion on that.

The API (linked above) already mandated that the hash is part of the Signature if it is provided (distinct from payload verification). I have no opinions that differ from that in the API document.

---

What are the next steps?

I'm left wondering;

1. Is hawk really only client-side assurance by default? and server-side assurance only when payload verification is explicit on the server? If so, then Hawk fundamentally is not the implementation of Signed Requests I wish to use or contribute to becasue to me that serves a niche that gives me no security characteristics i can use.
2. Will we merge parts of this? I am happy to make a new PR with only the optional payload verification.
3. Assuming 2, maybe i'll wait and come back to try and address the security characteristics needed for a server-side assurance, maybe..

[rfk]

Chris, assuming you do step away from this PR, are you happy for me to merge and build upon your work-in-progress? As stated, I do believe it's a valuable improvement. (But I wouldn't merge if if you weren't happy for us to do so).

i.e. If the client provided MAC is VALID it still needs to have server-side validation of the MAC "Signature"
which is Signature not "payload validation", it is still the MAC.

Honestly, I am simply not able to parse what you mean here. Authentication both here and in the JS reference implementation checks the client-provided mac value against the expected signature calculated from the secret key and the normalized request string, and they both calculate the normalized request string using the value of hash provided by the client. The signature is still checked and will be rejected if invalid.

[chrisdlangton]

best to clarify this first

Honestly, I am simply not able to parse what you mean here. Authentication both here and in the JS reference implementation checks the client-provided mac value against the expected signature calculated from the secret key and the normalized request string, and they both calculate the normalized request string using the value of hash provided by the client. The signature is still checked and will be rejected if invalid.

Stage 1 optional payload validation

As described you may have optional payload validation if the server wants to do so. This is well known and agreed upon.

Assuming we do or do not perform the optional payload validation, stages 2 and 3 are distinct from payload validation.

Stage 2, Check the Signature using client provided values

(directly following above)

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

It is OK for the server to use the client provided MAC, if they do not match we early exit.

We are agree so far, this is what you have argued and I always agree, in fact it is exactly how hawkauthlib works today.. BUT please continue reading the spec, hawkauthlib has not done the next stage to complete a valid Signature Check

Stage 3 complete a valid Signature Check

quickly, Signature is hawk.1.header
Payload validation is hawk.1.payload

(Included in the above from stage 2)

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

We reach your question here, i wrote, and you questioned the meaning. So i'll break it down;

If the MAC is valid

that is stage 2 result

the server calculates

In stage 2 it used client provided value, for quick result / early exit

the server calculates the payload hash and compares the value with the provided payload hash in the header.

Specifically the header. It is referring to the MAC "Signature" not payload validation.

If the client did not provide a hash, stage 2 and stage 3 are identical and stage 3 is not needed. If the client did provide the hash, we must include the hash the client provided in stage 2 but in stage 3 we must not inherently trust the client value.

---

Stage 3 is about server-side assurance. If the client provided the hash, and the stage 2 validation result was VALID this is not giving assurance.

Stage 3 is where this PR adds to Hawk the missing completion to it's Signature check, the part it never had but should have had for server-side assurance.

---

Happy to push on with the PR if we are aiming for server-side assurance of Signature checking

---

If Hawk is to remain client assurance and optional server assurance, please use my PR contribution as you wish 👍 it's been fun

[chrisdlangton]

I am keen to resolve the dispute so the new version can be released, while not actually patching the default behaviour it is at least offering the new secure functionality as a secure-if-configured securely option that implementations out there could leverage going forward.

I wondered if I might be wrong, so I wen looking for Hawk implementations to confirm the view one way or the other, preferably a Mozilla example to reference.

My hypothesis is the NodeS code that everything stemmed from was just poor coding, or poor interpretations, and the secure characteristics described by the spec (and an expectation of signed requests using a HMAC) was always the intention and goal.

Either way it would provide closure 2 years on.

Stefan Arentz @st3fan a former Mobile Browser Engineer at Mozilla - Interpreted the Hawk spec in the exact way I did for his Go implementation.

Specifically the signature generation and then comparison calculates the hash from the payload.

Nowhere in the implementation is the client provided hash simply taken and used in place of the signature generation prior to a proper validation.

Stefan never takes the client signature for any signature assembly shortcuts, the signature is always assembling the payload if it is present, and will generate it's own hash for payload validation.

An assembled signature that uses a client provided payload and client provided hash intended to validate the payload integrity, then not assemble the hash signature itself to do payload integrity checking (this repo)

You have a situation where the client provided values are compared to client provided values - of course they match! What is the point of that? The logic is trash.

Kumar McMillan @kumar303 author of mohawk (another python interpretation like this repo) also agrees with Stefan and my own view in his code.

The only places I see the errors are the original nodejs that now exists only in the FXA fork, this repo, and mozilla/hawk (originally hapijs/hawk) .

If you disagree with Kumar, Stefan's, and my own views, we may be wrong, please educate us.
Please reason rationally with the interpretation you hold and be specific about the security characteristics that demand a HMAC-SHA256 for it's implementation detail that are self evident and able to be assumed inherently knowing the properties of HMAC-SHA256 (like Kumar, Stefan, and I leverage in our domain knowledge of this)

I am sure we can be all open minded, but noone has done more than explain how it work, we are still awaiting to learn why it works that way, the benefit related to the purpose of Authentication. It's accepted your opinions of performance make sense, but this library represents Authentication, not performance guarantees. So please expalin why it operates this way in context to the goals.

I hope a review of the work by Stefan, Kumar, and my own PR will be enough input to reach a rational conclusion here.

[chrisdlangton]

Given @rfk messaged me to let me know he no longer works for Mozilla, I'm wondering what the maintenance status is for this library?

ping @return42

[return42]

I'm wondering what the maintenance status is for this library?

Can't say much about this since I'm not from mozilla .. I think this lib is dead.

---

I just did some boilerplate code / in context of the Mozilla Services to make the requirements of the service py3 ready .. but that was a waste of time, because mozilla decided to let the whole project die ... of course without telling the contributors, that's mozilla style .. so don't expect to much feedback ..