Comments (15)
What is assured by the MAC is that the artifacts
have not been tampered with not that the actual payload hash was used to generate the MAC. No assurance that a hash
provided in the header matches the actual hash of the payload is assumed. The MAC assures that the artifacts
are genuine and nothing more.
This shows that we take the client provided values to populate
attributes
This is by design. If the MAC is valid and the artifacts
contains a hash
we know that the hash
hasn't been tampered with and can be trusted to optionally validate the integrity of the payload.
It would be rather impractical to require the entire payload to be transferred and hashed before the MAC could be validated. This is why the hash
value exists in the hawk header, so the MAC can be validated quickly and independently from the payload.
All scenarios are covered:
- If the
hash
in the hawk header was modified in flight the MAC validation will fail - If the
hash
and the payload are modified in flight the MAC validation will fail - If only the payload was modified in flight the MAC validation will pass, but the payload validation will fail
- If the
hash
doesn't match the payload hash for any other reason payload validation will fail
from hawk.
* Client expectation is by providing the hash, the server will perform payload validation
This is not implied. The documentation clearly states:
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.
* Client recognises it's up to the server to perform payload validation or not, so a client includes the hash in the signature because MAC validation is not optional
This is a bad assumption. Nowhere is it implied that the payload itself is protected by the mac, only the artifacts
which may include the hash
value.
* The server uses only _client provided values_ (blind trust) to perform Signature calculation, nothing is validated on the server
This is false. The server holds a secret for the given id
that it uses to validate the mac. This is sufficient to determine if the artifacts
are genuine. An attacker would need to obtain this secret in order to tamper with the artifacts without detection.
Hawk servers implementating this library gain no server assurance against modified request body when a hash of the body is signed and meant to be validated
If hawk is insecure in the way you describe it should be easy to prove. If you can show that a request can be tampered with and pass both mac and payload validation on the server then I will agree that it is insecure and we will work to fix it.
So why then is there a hash in the MAC at all?
It can be considered an optimization to improve mac verification performance. If the request passes mac validation the hash
is known to be genuine and can be used to verify the payload. Without a hash
in the hawk header, to achieve payload validation the entire payload would need to be hashed before the mac could be validated. Integrating the payload hash directly into the mac would implicitly make payload validation required.
To be definitive, what you've described so far is neither a bug nor a vulnerability when mac and payload validation are used as intended. I'm not interested in changing the hawk protocol or implementation merely to meet your proposed additional assurances because the existing protocol is sufficiently secure.
from hawk.
the current code doesn't have the functionality 'to test'
Are you saying that the vulnerability is not solely in the hawk library, and rather in how the library is used in an application? It sounds like you are emphasizing the point that an application that omits payload validation is vulnerable and going further to say that it is a vulnerability that hawk can be used in that way.
from hawk.
Everything you've written is true, according to the current state.
Does that make it secure?
To answer that, take a step back and think about what is secure, to be secure requires assurance, assurance provided by whom, to whom.
You wrote
If only the payload was modified in flight the MAC validation will pass
Which is factual, but incomplete. To be a useful statement it needs to be complete. This is the same statement with the key part you missed, to indicate the security characteristics of it
If only the payload was modified
in flightafter signed by the client the MAC validation which included a hash attribute will pass on the server
That's the factual truth currently,. What security characteristic applies when:
- Client signed with a payload hash
- Client expectation is by providing the hash, the server will perform payload validation
- Client recognises it's up to the server to perform payload validation or not, so a client includes the hash in the signature because MAC validation is not optional
- An insecurely configured server skips payload validation, that should occur first. That is its right
- The server uses only client provided values (blind trust) to perform Signature calculation, nothing is validated on the server
Tell the reader what is untruthful about the full description i provide. Then tell them where they gain confidence in the security of Hawk in it's current default usage.
The issue is nothing is actually validated on the server, it is merely assembled using client values and signed. That is not validation, and Hawk servers implementating this library gain no server assurance against modified request body when a hash of the body is signed and meant to be validated, and client's equally get no assurance by providing a hash or signing the hash in the MAC.
So why then is there a hash in the MAC at all? Why bother, do or do not, the security is the same.
Perhaps it was intended to be secure, but code currently has a flaw and it's insecure until fixed
from hawk.
That can all be summarised as distinction without a difference.
Concisely, the distinction is 1. MAC Signature with hash without payload validation, and 2. Signatature with hash with payload validation. I.e. both have the hash but only 1 validates it is the distinction.
The 'without a difference' in distinction without a difference is both have the hash, and the security characteristics of the hash doesn't change. The hash representation of the payload only maintained it's secure characteristic if it is validated...
So all your word gymnastics only further prove to readers the point that currently the security characteristics of the hash in the MAC Signature are ignored by default. Thank you.
The MAC Signature maintenains the same security control with or without a payload hash. This is what is the security flaw that is easily exploited.
from hawk.
This is clearly described by CWE-642: External Control of Critical State Data achieved simply by the modification of payloads (in JavaScript via malicious plugin, unconfined Ad, or CSRF/XSS), or using HTTP Desync. Because the payload modifications are only detected by the server if the HMAC verification occurs on the server, and all modifications were done without knowledge of the secret needed to generate the HMAC because the original was signed by the real client who has the key the server decides to use the trusted client provided HMAC completely violating the core concept of Authentication entirely by relying on trusted client signatures.
We have undeniable proof now from the example of CWE-807: Reliance on Untrusted Inputs in a Security Decision that has been patched in FxA by doing the validation, ergo the authentication mechanism, on the server side. i.e. the entire point of HawkAuth was to have this built in, not optional. I digress.
The bounty has been tagged as a hall of fame bug now, so before the CVE is issued and a write up is published, you may consider rethinking your view on his issue and adapt the PR I raised to fix the flaw, or your own variation of that PR.
from hawk.
I adapted the PR for you.
Mozilla abandoned the Python repo, but this repo has 1.1 million NPM downloads a week.
I suggest this get patched, an Advisory added, and CVE requested.
If the the risk team see this, please find an active maintainer ASAP
from hawk.
@chrisdlangton We are re-investigating this issue to provide an updated response. I have added your test to the current hawk repo (i.e. one without your patch) and it passes. Are you able to provide a similar test that fails against the original hawk repo and illustrates there is a scenario where tampered data passes both MAC and payload validation?
tom@t moz/hawk (main *) » npm test 127 ↵
> [email protected] test
> lab -a @hapi/code -t 100 -L
..................................................
..................................................
..................................................
..........................
176 tests complete
Test duration: 63 ms
Assertions count: 241 (verbosity: 1.37)
The following leaks were detected:AggregateError, atob, btoa, AbortController, AbortSignal, EventTarget, Event, MessageChannel, MessagePort, MessageEvent, performance
Coverage: 100.00%
Lint: No issues
tom@t moz/hawk (main *) » subl test/index.js 1
tom@t moz/hawk (main *) » npm test
> [email protected] test
> lab -a @hapi/code -t 100 -L
..................................................
..................................................
..................................................
...........................
177 tests complete
Test duration: 63 ms
Assertions count: 246 (verbosity: 1.39)
The following leaks were detected:AggregateError, atob, btoa, AbortController, AbortSignal, EventTarget, Event, MessageChannel, MessagePort, MessageEvent, performance
Coverage: 100.00%
Lint: No issues
from hawk.
The code being introduced into the PR is because, as you've discovered, the current code doesn't have the functionality 'to test'.
The vulnerability being shown in a unit test case would be trying to prove a negative, which I shouldn't need to point out proving a negative is never convincing as proving nonexistence is an oxymoron.
But if you have an endpoint you could use the PoC in my gist to check the issue as an end-to-end test case, rather than until testing for code that isn't there...
from hawk.
You can see the 3 scenarios in the Gist PoC now
from hawk.
Hey @tomrittervg what is the status, or plan here? I see a few options:
- Patch (PR) merged, NPM will get the patched version soon, including a GHSA for Dependabot + CVE issued by GitHub (recommended, expected, current best practice).
- NPM will not get the patched version but the PR will be merged in this repo + CVE issued by Mozilla, no GHSA (unexpected, old best practice because Dependabot will not alert users).
- NPM will not get the patched version but the PR will be merged in this repo + CVE issued by Mozilla, with GHSA (Not a bad compromise).
- Patch will be abandoned and repo archived (following Hawk in other programming languages, i.e. python) - not recommended
Or (4) status quo; it will this remain semi-abandoned, maintenance-mode only - but not really doing any maintenance in reality.
The Bugzilla ticket stated Hawk will be archived - and Python has already done that.
Or was that ticket statement not referring to Hawk repositories being archived, and rather some other meaning of archive? It might have been specific about what it means by archive so I assume it was referring to the GitHub meaning, is that right?
from hawk.
@lotas will this repo be archived? If not, what was the meaning of the archived comment? Or has the archived decision changed now?
Will there be a GHSA added, and linked to the NPM package, and version that patched the issue? If not, why would you choose to not alert the 1.1 million weekly dependants to not get notified by NPM audit?
Will the GHSA also request a CVE from GitHub so there is a Dependabot alert? If not, why would you choose to not alert the unknown (too numerous number of pages of GitHub dependant projects) millions of repos not be notified by Dependabot?
from hawk.
Requesting an update please.
I realise 6 weeks is nothing compared to nearly 3 years late on advisory - but I stress that an advisory late is better than no advisory at all
I highly recommend the GHSA be created ASAP, you may use this as a guide (I am not asking you to copy it, it's just FYI to help speed this up for you), then I'll delete mine once yours is published.
from hawk.
Hi @chrisdlangton,
thank you once again for submitting #290 patch
@lotas will this repo be archived? If not, what was the meaning of the archived comment? Or has the archived decision changed now?
I don’t think this repo will be archived, as hawk is being used by Taskcluster still (Firefox CI)
Will there be a GHSA added, and linked to the NPM package, and version that patched the issue? If not, why would you choose to not alert the 1.1 million weekly dependants to not get notified by NPM audit?
After speaking with security team internally we came to a conclusion that HAWK API documentation made it clear that payload hash validation is optional and up to developer to implement. This way, users were already notified.
Further investigation of dependent packages at https://www.npmjs.com/browse/depended/hawk shows that most of the packages are either outdated, unmaintained or use HAWK for client-side only.
Looking into github’s dependency graph https://github.com/mozilla/hawk/network/dependents?package_id=UGFja2FnZS01MDY4NTM5OTU%3D it make it look as there are million of packages dependent on this, but deeper investigation shows that “hawk” library is being referenced as dependency of a dependency (of a dependency):
- For example deperecated
request
module was referencinghawk
- some repositories don’t even contain
hawk
in package.json/lock files, does github keeps dependency tree up to date? - older versions of
jest
were usingrequest
which referencedhawk
To our awareness, at the time of changing ownership of hawk library, there were other known interested parties who were using hawk on the server side besides Mozilla.
Will the GHSA also request a CVE from GitHub so there is a Dependabot alert? If not, why would you choose to not alert the unknown (too numerous number of pages of GitHub dependant projects) millions of repos not be notified by Dependabot?
For the same reasons as above
from hawk.
Same reason as above
Saying 1.1 million NPM downloads is the same as the GitHub dependency graph
It is like saying water is harmless to humans, and acid must be the same.
Or we see no one buys cars from dealership's in Australia anymore so we'll stop shipping to dealership's worldwide where we have no data, just because we know it's the same everywhere
No
Just no
Try harder maybe?
from hawk.
Related Issues (20)
- Action required: Greenkeeper could not be activated 🚨 HOT 1
- Update deps HOT 1
- Rust Implementation HOT 3
- No Changelog HOT 4
- Only node 12
- Drop hapi 17
- Add TypeScript typings HOT 1
- Import changes made at hapi HOT 5
- Update mozilla/hawk to Mozilla repo standards HOT 1
- Decide future of hapi hawk plugin HOT 2
- Set up CI HOT 4
- Support webpack'ing hawk HOT 1
- Support use of this library in frontend code HOT 9
- Travis CI free usage ends Dec 3; mozilla repos should switch to other CI platforms HOT 1
- Using hawk in Angular app for absence.io
- Examples in API.md are broken
- Porting test framework HOT 2
- types.js:8 Uncaught ReferenceError: Buffer is not defined HOT 1
- calculateMac() should not be deprecated and split into two different versions HOT 8
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from hawk.