hasura / graphql-engine

Blazing fast, instant realtime GraphQL APIs on your DB with fine grained access control, also trigger webhooks on database events.
https://hasura.io
Apache License 2.0
31.06k stars 2.76k forks source link

docs: document security for action handlers #4645

Closed danielkcz closed 1 year ago

danielkcz commented 4 years ago

Current behavior

By default, the action handlers have a single security layer - by obscurity. Unless action handlers URLs are predictable or stored somewhere (migration files in VCS).

The second "easy" protection layer is setting up roles for the backend while using the admin secret. That certainly helps prevent unwanted access to data but it still doesn't prevent anyone from calling that handler directly. Without proper checks in those handlers, it could be breaking the app flow unexpectedly.

Enabling "Forward client headers to webhook" for action handlers means we get the Authorization header, but without knowing the signature key we cannot easily validate it. Nor I don't feel we should.

Proposed solution

There was some discussion at Discord today and some said they have their own solution for rotating keys that are sent to headers of action handler.

While that's nice, I wonder why everyone has to develop such a solution on their own. I think Hasura should have additional security layer out of the box.

I have the following idea that's definitely rough around the edges as I am not a security expert. I haven't used migrations in practice yet so I am not sure how it would work there.

Auto-generated action secret

Hasura can auto-generate a secret key (different from admin secret) which is then passed to all action handlers in headers/body. This secret would be manually obtainable from the console and could be stored in action handlers env. Alternatively, the backend could run an admin-only graphql query to obtain the current (and previous) secret (in serverless) or even a subscription (with a full server).

The main point this is solving is to avoid manually updating each action one-by-one. The generated secret would be automatically used for all the actions or opt-out completely.

Additionally, the rotation of such a key would be useful in case of an accidental leak. It's probably enough to do it manually.


In other words ... https://github.com/hasura/graphql-engine/issues/4645#issuecomment-643344612 and https://github.com/hasura/graphql-engine/issues/4645#issuecomment-644491468. No need to read rest which mainly focuses on making a more secure variant with HMAC which isn't part of this issue.

aroraenterprise commented 4 years ago

Hey, I think you missed an important action feature.

You can set custom headers in actions in Hasura, which means that you can set a special token that only Hasura knows. I use an environment variable such as ACTION_SECRET in my heroku secrets. This action secret is only known by HASURA and is passed to my endpoint at every call. this is the only way I know for sure that my action was called by my HASURA. So there is absolutely another layer of security already in the box. This is why I recommended writing a script that would rotate this environment variable in the Heroku Config for both my express app (action endpoint) and my hasura.

And for this to work your backend does not need to know about authorization secret or anything.

danielkcz commented 4 years ago

You are right, I haven't realized that exists. I mean I saw that option there to use env, but I wasn't sure about how it can be useful for anything 🙈

I guess it should be at least documented in a better way. Maybe some blog article about securing actions could come out of this :)

Thanks for pointing this out.

haf commented 4 years ago

If you're e.g. on a LAN and get DNS poisoned, the attacker might learn the secret, if you keep sending it in plaintext every request; one single request is enough to impersonate until key rotation. Consider an HMAC scheme alike that of Twitter, and for increased security, consider adding support to Hasura for opting into challenge-response to authenticate the receiver of the request (your code).

So; level 1:

your code HMACs incoming requests, compares to known shared key HMAC digest (like Twitter, Twilio, Stripe, etc)

level 2:

sending code validates receiver

danielkcz commented 4 years ago

@haf There is always a security risk and it's probably more relevant depending on how "lucrative" is your software for possible attackers. They can always find a way if it's worth their time. That's not the point of this issue. For advanced security, people can choose whatever they want. Here I want a simple solution out of the box while there is none currently.

@aroraenterprise Anyway, new day, new thoughts. I still think there is a value in Hasura providing a better built-in solution. It's nice you can have env value in the action, but it's a manual task to do it in every action. You can always make a mistake there. Unless there is some automated way I am not aware yet.

With my proposal, there would be a generated secret by Hasura which already lifts the burden from the developer about picking secure enough key. Then with a simple flick of a checkbox, this secret would be sent along with all actions. No manual work when you add a new action is necessary anymore. And if you want something more secure, you can always turn it off.

Unrelated note, thinking about secure enough keys, I would be nice if the admin secret could be generated by Hasura as well. More like a suggestion, in this case, not automatically applied since it has to be in env.

danielkcz commented 4 years ago

@marionschleifer Based on my last comment, I am convinced this is more than a docs update. I am proposing here a secure solution out of the box for the initial phases of the developed project. With the option to turn it off if more strict security is required.

haf commented 4 years ago

There is always a security risk and it's probably more relevant depending on how "lucrative" is your software for possible attackers. They can always find a way if it's worth their time.

No, there are things that are proven correct, axiomatically, and then it's not a risk for all properties proven correct. You can e.g. have a look at Project Everest coming from miTLS, or formal methods for security analysis. If you can map the proofs 1-1 with the code, you can literally prove that the code doesn't contain vulnerabilities. But I digress...

That's not the point of this issue. For advanced security, people can choose whatever they want. Here I want a simple solution out of the box while there is none currently.

Me too. You should not send a secret, use HMAC, it's as easy and provides better security. Or in other words, there is no trade-off here; HMAC is just plainly a better solution than sending a secret with each request.

danielkcz commented 4 years ago

@haf As much as HMAC sounds simple to you, it's not for me and I would dare to say for many others. I agree with you it would be nice to have a built-in more secure way, but it should start at "level 0" where it just sends the secret that can be just compared to env value. That's ideal solution for early development phases.

haf commented 4 years ago

I can explain it; it's not hard, it's really level 0! 👍 :) You have some shared secret, like you suggest to start with, and:

Now you check whether;

HMAC(shared secret, incoming message) = incoming message hash and if that returns "true", you accept the message, otherwise you don't.

In Haskell;

{-# LANGUAGE OverloadedStrings #-}
import Data.Digest.Pure.SHA

main = do
  let a = hmacSha256 "shared secret" "incoming message"
  mapM_ print [showDigest a]

from https://stackoverflow.com/questions/24840383/generating-hmacsha256-hexdigest-in-haskell

or in NodeJS, server side:

require("crypto").createHmac("sha256", "shared secret")
  .update("incoming message")
  .digest("hex");

TypeScript, express flavour:

import crypto from 'crypto'
function acceptHook(req: Request, res: Response) {
  const hmac = crypto.createHMac("sha256", process.env.SHARED_SECRET || 'deadbeef')
  const allow = hmac.update(req.body.incomingMessage).digest('hex') === req.headers.messageDigest
  if (allow) {
    res.status(201).json({ error: false, message: "Ok, I will restart!" })
  } else {
    res.status(401).json({ error: true, message: "Operation denied" })
  }
}
lionelB commented 4 years ago

Is there something like described here https://developer.github.com/webhooks/securing/?# for securing webhooks ?

haf commented 4 years ago

Yes, that's the algo described above

post '/payload' do
  request.body.rewind
  payload_body = request.body.read
  verify_signature(payload_body)
  push = JSON.parse(params[:payload])
  "I got some JSON: #{push.inspect}"
end

def verify_signature(payload_body)
  signature = 'sha1=' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), ENV['SECRET_TOKEN'], payload_body)
  return halt 500, "Signatures didn't match!" unless Rack::Utils.secure_compare(signature, request.env['HTTP_X_HUB_SIGNATURE'])
end
webdeb commented 4 years ago

I like the HMAC approach, since it does not leak out any security information, but only signs the actual data. I would like, if that approach could also be applied to event-handlers the same way.

Event-Handlers are like actions, just a remote system which needs to know who is calling it.

webdeb commented 4 years ago

@haf As much as HMAC sounds simple to you, it's not for me and I would dare to say for many others.

@FredyC Thats a good point. But it should't be an excuse to not trying to secure a system. Its somewhat okeyish to pass ENV variables around in a closed network. However, it's not good to encourage people to do it by default, because it is potentially risky, like storing your JWTs in localStorage like many are doing it.

HMAC is just a way to keep secrets secret.

So the idea here is not to let the people implement their HMAC logic itself, but provide a simple way (with packages, examples etc) to use these techniques in a common way and keep their services secure.

danielkcz commented 4 years ago

@webdeb I am merely building on top of the premise that currently there is no security. Automatically passing some secret key is at least something and without some basic effort from a hacker, it will prevent from easy security breach just by knowing action handler url.

So the idea here is not to let the people implement their HMAC logic itself, but provide a simple way (with packages, examples etc) to use these techniques in a common way and keep their services secure.

Something like that, yea. Now realizing that it's effectively opt-in like that because it's up to you to verify that HMAC in your code, but that's fine. It won't slow down the initial development phases.

hugbubby commented 4 years ago

One in-house solution we have for now: Our backend action handlers automatically register their actions with Hasura using the API every time they start up, so that we don't have to configure hasura when we move the handler domain or IP. During this process, it attaches an autogenerated header to the action that includes a random key, which the action handler checks the existence of before accepting the or rejecting the request.

@haf Message signatures and encryption within the header or of the request itself are security theater, because Hasura should already be connecting to the action handler using TLS, and TLS's purpose is to ensure that the content of the action hasn't been seen or modified. If you are only using HMAC to protect your action requests and otherwise send them over bog-standard HTTP, then you're only protecting the integrity of those requests, not their confidentiality. The point of the key is to authenticate the hasura instance to the action handler, and that's what this scheme does.

haf commented 4 years ago

@hugbubby I may not fully understand what you intend to say. I'm not encrypting any headers. I'm adding a signature to them, which validates that the sender has the identity of an authorised sender. So HMAC protects both the integrity and gives you sender authentication and that is different from transport encryption (TLS). Perhaps I'm misunderstanding you, could you please clarify?

perrosnk commented 4 years ago

I don't fully understand either. Could someone provide an example with some steps and some code on a node/express handler?

danielkcz commented 4 years ago

@perrosnk You don't understand what exactly? Check the linked PR for basic security docs that is possible right now. This issue is about implementing a more robust solution.

hugbubby commented 4 years ago

@haf I'm saying TLS already provides integrity verification. There is no need to use HMAC or a similar algorithm a second time to check that the contents of the message have not been modified, because TLS handles this for you. You've overcomplicated the problem and just need to send the key without using HMAC, unobfuscated.

danielkcz commented 4 years ago

@hugbubby Huh? Even if your action handler is running with TLS, it doesn't give you any validation about the origin of the call. It's pretty much public resources anyone can execute (with a token). And as it was already said, TLS won't stop the provider to log the request including headers. Unless of course, it's your own machine with your own SSL certificate, but that's not that common anymore imo.

haf commented 4 years ago

@hugbubby What TLS gives you is authenticated encryption (e.g. with the modern GCM cipher) not authenticated caller.

You can of TLS as something that wraps a TCP socket which streams data on Layer 4, but the app needing to authenticate the sender in OSI Model Layer 7

So while HMAC also gives you integrity checking, it also gives sender authentication. You're stating that it's better to separate authentication and integrity checking, by checking the key alone; but what you're failing to take into consideration is network attacks, primarily MiTM.

Example scenarios:

All of the above are protected from by using HMAC. It's a bit like the End to End principle. Or one could say that you've undercomplicated the problem ;)

Corollary; why would all these big services, including Twitter and Github, use HMAC if it's wrong?

haf commented 4 years ago

@perrosnk https://github.com/hasura/graphql-engine/issues/4645#issuecomment-623163124 has sample code for a node/express handler.

hugbubby commented 4 years ago

@FredyC @haf I realize that TLS does not authenticate the client. What I am suggesting, in place of (if I understand how you accomplished this) forking hasura's code and introducing HMAC signatures in the header of your already MAC'd HTTPS request, is to simply attach the pre-shared key itself to the "added headers" field and check its value at the action handler.

Let me explain why each of your example scenarios is a silly reason to attach a second HMAC signature within the plaintext HTTP request:

You move to host at api.example.com = 4.3.2.1, and someone else takes over 1.2.3.4, but Hasura keeps sending your credentials to 1.2.3.4: now someone else can impersonate Hasura and your security model is broken

This is expressly the kind of thing TLS and HTTPS protect against. You're confused about what TLS is supposed to offer. When your browser connects to https://github.com, it verifies that github.com is github.com through public key encryption and CA chains. If someone tries to connect to an old IP address, their browser will throw up a warning page asking for confirmation, or refuse to connect, because the people at 1.2.3.4 don't have Github's private keys and can't present a valid certificate.

You're internal in an enterprise, there's no TLS, someone sniffs the network...

Then the solution is to get TLS. What you're doing is just poor in-house TLS, that comes without any confidentiality guarantees, certificate management, or key distribution mechanisms in place. Your solution for "layered" HMAC authentication doesn't even have the property of public key cryptosystems where you can just have one identifying public and private key per host, so on top of being less secure, now you have to manually manage a new pre-shared-key for each pair of hosts that want to talk to each other. I have no idea why someone would want to subject themselves to this.

You're internal in an enterprise, there is TLS, but no Network Access Protection; someone comes in, gets a TLS because every client gets a cert, they DNS poison your service, and you're owned.

A TLS cert is not a general badge on your server that says "TLS-Certified", it authenticates their identity to incoming clients. The fact that they can decrypt incoming traffic encrypted with their public key proves they are the possessor of their unique private key. If you are giving everyone's private key to everyone who comes onto the network, then you have completely destroyed the point of using TLS in the first place.

...or they take over the IP via an ARP attack, you're owned again

Again, no, because without the private key from the server you ARP spoofed, they will connect to your host and you won't be able to decrypt traffic they send you, or you'll send them a bad certificate and they won't connect.

...or the sender has disabled CA validation (because you're using internal CAs), so the attacker can use a self-signed cert, and you're owned

...or the admin has disabled CA validation for the same reason

Then don't disable CA validation. If you're using internal CA's, you should be enabling CA validation so that the sender can use your internal CA's. If you're not using internal CA's because they're "too hard", then why engage in this arduous process of manually distributing pre-shared-keys? Why not just distribute public keys instead and fail if the sender doesn't receive the right one? You have to deal with manual key management, but you did anyways under this system of in-house HMAC?

Corollary; why would all these big services, including Twitter and Github, use HMAC if it's wrong?

They use HMAC. I am suggesting you use HMAC. I am suggesting you use TLS, which includes message authentication. If someone at these big companies is using HMAC like you're suggesting without any additional context, they're wrong.

Edit: If they get access to your DNS name you're toast in most cases, because what the cert authenticates and what the client checks is ownership of the "github.com" domain, not the organization. There was a clause here that said otherwise.

danielkcz commented 4 years ago

I think we got fairly off track here.

@hugbubby I am still not seeing how can TLS validate that you are coming from Hasura and not from the outside. Unless Hasura is going to act like CA and issue certificates for our action handlers :) Assuming you somehow get access to a token to impersonate the user you still shouldn't be able to call that action handler outside of the Hasura. And that's why I started this issue in the first place.

I now understand that with simple static secret sent in headers it does not impose any protection. If someone is able to obtain a token from the request, getting the "secret" is the same. Hence the HMAC that uniquely identifies the request gives much better results. Sure, it's still possible to "repeat" the same request, but that's such an edge case that I doubt would be problematic.

hugbubby commented 4 years ago

I am still not seeing how can TLS validate that you are coming from Hasura and not from the outside.

@FredyC TLS doesn't validate that particular thing. The header entry validate's the client (Hasura's) identity to the action handler. TLS validates, to both parties, that whoever is connecting to them did not have their connections muddied with or seen, and to the client (Hasura) that they are connecting to the action handler and not some other MITM person. This is why TLS obviates (or rather includes) the security benefits of HMAC, but not the header entry.

If someone is able to obtain a token from the request, getting the "secret" is the same.

Which is why you use TLS, and thus prevent this token capture from happening between the endpoints. No need to worry about replay attacks from attackers MITMing. TLS covers these edge cases.

danielkcz commented 4 years ago

I guess that makes sense. Not that I would believe that TLS is the only level of security you need. There are surely other means of gaining access besides MITM, but I am no security expert.

I don't care that much ultimately. My initial concern came from pure laziness where I want Hasura to take care of including a secret for me to all actions by default. If that's going to be HMAC or something else ... 🤷‍♂️

haf commented 4 years ago

What I am suggesting, in place of (if I understand how you accomplished this) forking hasura's code and introducing HMAC signatures in the header of your already MAC'd HTTPS request, is to simply attach the pre-shared key itself to the "added headers" field and check its value at the action handler.

I'm suggesting Hasura includes HMAC in its source.

They use HMAC. I am suggesting you use HMAC. I am suggesting you use TLS, which includes message authentication.

So we're in agreement there.

The point is this; under MiTM attacks, it's safer to use HMAC than to send the secret.

webdeb commented 4 years ago

@haf just curios, how should hasura sign the whole request with HMAC, I mean just for strings like base64 encoded jwt payload it's not a problem, or signed urls. But we would need to sign headers and the payload then add the signature somewhere into the headers also. I guess that the only way of doing it right, would be to encode everything as a stringified json, input & headers

Btw. Right now, I am also just using the ACTION_KEY approach, which just works but I am convinced, that it's not very good to pass the KEY around, the same goes for "backend-only" operations, where you have to include the "ADMIN_SECRET" into the request, which actually makes it very easy to take over your whole system if things go wrong: https://hasura.io/docs/1.0/graphql/manual/auth/authorization/permission-rules.html#backend-only-permissions

haf commented 4 years ago

@webdeb I think it would be enough to calculate hmac(key, request body). You'd run into problems with middleware (proxies and such) adding headers, otherwise. You can go pretty far with request validation, and I'm not suggesting we go much further than doing it for the body.

webdeb commented 4 years ago

Hmm, well, that could be indeed enough. Then it would be possible to go also the other way with the same approach "x-hasura-signature" to authenticate the sender as a trusted-backend for backend-only operations

webdeb commented 4 years ago

@FredyC Since this topic goes beyond action handlers, I would like to suggest to change the title to something like "Built-in security for services & backends". As it also touches event-handlers and backends

hugbubby commented 4 years ago

@FredyC

Not that I would believe that TLS is the only level of security you need. There are surely other means of gaining access besides MITM, but I am no security expert.

There are other ways to break this scheme without breaking TLS, but they require you to compromise the endpoints, in which case there's no reason you wouldn't also have the pre-shared-key used to forge the X-Hasura-Action-Secret header. On the other hand, if you've broken TLS, an additional HMAC signature on top of the existing one doesn't help.

@haf

So we're in agreement there.

I can't understand how you'd get through my comment and end up believing this. You keep suggesting the Hasura devs to include HMAC signatures of the payload inside an already existing TLS connection, and I don't know how else to explain why this is redundant.

The point is this; under MiTM attacks, it's safer to use HMAC than to send the secret.

Subject to strong diminishing returns

haf commented 4 years ago

@hugbubby You're conflating concerns: TLS is a transport protocol and deals with authentication of message bits of the cipher stream, but HMAC gives you authentication the requestor. Yes, a pre-shared key also gives you authN of the requestor, BUT IN REAL LIFE there are attacks that happen, and not every OS and client have perfectly configured CA chains, or trust all ~250 CAs in browsers or even have the manpower to set up internal CAs in enterprises. It's not about "breaking TLS", it's about the soft stuff in between.

danielkcz commented 4 years ago

@hugbubby

You keep suggesting the Hasura devs to include HMAC signatures of the payload inside an already existing TLS connection, and I don't know how else to explain why this is redundant.

I wonder why do you keep fighting against HMAC so much? I am sure that adding config boolean to disable it is fairly trivial so if anyone believes that TLS is enough, they can opt-out. And besides, even if you receive HMAC in the header, you don't need to do anything with it. So ultimately we talking about the tiny calculation. Is it really worth arguing that much? :)

@webdeb

Since this topic goes beyond action handlers, I would like to suggest to change the title to something like "Built-in security for services & backends".

Sorry, not my problem that others keep hijacking it here. I try to focus on the topic at hand here. Feel free to open new issue if you want to go deeper.

danielkcz commented 4 years ago

I think it would be enough to calculate hmac(key, request body). You'd run into problems with middleware (proxies and such) adding headers, otherwise. You can go pretty far with request validation, and I'm not suggesting we go much further than doing it for the body.

I think it might be useful to also add a timestamp header along with the signature that would part of the calculation. Basically to ensure that every request has a different signature even if the payload is the same. Relying on the user to include a timestamp in the body doesn't sound like a good approach and it's actually annoying given a need to declare schema for each action.

Toub commented 4 years ago

@haf just curios, how should hasura sign the whole request with HMAC

Right. What about signing only:

So irrelevant headers and ones added by proxies will be ignored.

Additionnaly:

danielkcz commented 4 years ago
  • this HMAC should be optional so avoid wasting resources if not used (configuration by action/event trigger)

I don't think it's necessary for every action. Either you want enhanced security or not. Let's keep it as simple as graphql engine option.

  • a "/hmac_verify" hasura end-point could be added so the web-hook service can verify the request without implementing HMAC check

Frankly, that sounds like more work than implementing it or grabbing from some shared library. Grabbing relevant headers/body and resending somewhere else is not exactly one-liner :) Also, it could be a target for DDOS and attempting to force brute the key without some rate-limiting.

Otherwise, I agree with subjects of the signature.

haf commented 4 years ago

hmac(key, (headers - signature header) | body)

In my linked external sites above, I would only find them signing the body. That doesn't mean we couldn't sign more than the body, but let's think it through. What you gain:

What you lose:

Alternative:

If I wanted to ensure non-repudiability of "Hasura sent that UserId = 123", I would probably not solve it this way; instead I would have Hasura forward the JWT that triggered the schedule of the action, so I could use JWKS to verify its signature/HMAC.

Thus; I think there's more to lose than to gain from having all Hasura headers as input to the hmac function.

this HMAC should be optional so avoid wasting resources if not used (configuration by action/event trigger)

Doing a sha256 hash and iterating over an in-memory byte array, that is the body, is unlikely to cause performance problems, especially since it's such a common operation that it's available in hardware. I would argue it's better to let security be opt-out, since defaults matter for uptake. Worst case is, receiver ignores the header. I believe the header is present in all linked examples above, too.

a "/hmac_verify" hasura end-point could be added

I would advice against it, since now the receiver also has to have trusted configuration about where to send the request, i.e. it can't just trust the "who sent the request" proxy values (otherwise the signature is moot). It's better to have a 15 line snippet for every language in the docs; I can help write them :)

[timestamp] Basically to ensure that every request has a different signature even if the payload is the same.

This would only really help to avoid replay attacks, and only if you also provide a header for the original timestamp. Having a different signature on each request is probably not what you want.

I could imagine it being useful if you wanted linearalisability. Then, having a signature that is unique to the logical request lets receiving services save that signature as a way to ensure idempotency/just-once-processing.

What you gain:

What you lose:

If you want to be able to process/schedule an identical body multiple times, and you want idempotence in the receiving service, you could add x-request-id as a header as well as having a signature. If Hasura sees the request timing out, it would send a new, identical request with the same x-request-id.

hugbubby commented 4 years ago

@webdeb I think it would be enough to calculate hmac(key, request body). You'd run into problems with middleware (proxies and such) adding headers, otherwise. You can go pretty far with request validation, and I'm not suggesting we go much further than doing it for the body.

Dear God.

njaremko commented 4 years ago

I'd like to thank @hugbubby for taking the time and trying to explain why adding HMAC is a bad idea here.

The correct way to secure a Hasura action endpoint is to define a shared secret on both ends of the connection. This can be done by:

  1. Pre-configuring Hasura with an ACTION_SECRET env var
  2. Having your service hit the Hasura admin API and configure a randomly generated action secret on startup.

You'd then be sending that in a X-Hasura-Action-Secret header, where you'll check for its presence in your action handler.

With that established, all you need to do is make sure wherever you're hosting Hasura and your action handler provides TLS (which Heroku, AWS Lambda, and pretty much everyone else will handle automatically).

With TLS, you get:

Attempting to roll your own additional security on top of this with an additional HMAC is likely "security theater", as @hugbubby pointed out. The only situation where TLS will fail you is when an attacker can compromise your system and insert their own CA to allow interception of TLS traffic...which isn't really a valid threat model between two backend servers that you own.

It was mentioned earlier that other companies do HMAC on some stuff (most notably companies like Stripe and Github on their webhooks). They do this because they allow non-TLS endpoints (which are a bad idea, that is supported for legacy reasons).

Aside: The confidence with which the people in this issue have been repeatedly telling @hugbubby that they're wrong, when they themselves seem to know very little about cryptography or the TLS spec is astounding.

danielkcz commented 4 years ago

@njaremko That's all nice and possibly truthful, but I keep reminding that the original reason for this issue was "Built-in security for action handlers". It doesn't matter if it will be HMAC or shared secret, but to have it there by default and users would only consume it when they want.

If you want to keep discussing how to make actions super secure, please open a new issue for that.

njaremko commented 4 years ago

@FredyC I'm not discussing making actions "super secure", I'm saying that all the talk of adding HMAC (which has been the vast majority of this issue's discussion), is misplaced.

On the topic of adding a default, I think the furthest that Hasura should go on this matter is specifying that if you provide an environment variable to Hasura, lets say HASURA_ACTION_SECRET, then it will automatically include that in the session_variables object as x-hasura-action-secret for all actions by default.

Doing things like randomly generating keys and maintaining them are outside of Hasura's purview.

haf commented 4 years ago

@njaremko

The entirety of the message being sent is HMAC-ed, you can be sure no one messed with it.

Not the message, the transported contents. https://en.wikipedia.org/wiki/Authenticated_encryption This makes a difference. (I've worked with both methods)

Confidence that both parties in the communication are who they claim to be.

Only if you have mutual TLS authentication, which is very rare in practise. https://en.wikipedia.org/wiki/Mutual_authentication (I've set this up)

Normally deployments of TLS only have server authentication via public CA:s as trust roots.

If you use a service mesh like Istio, Citaldel/SPIFFE gives you mTLS (same with Linkerd); but now it becomes a question of PKI key management instead. https://istio.io/latest/docs/concepts/security/#mutual-tls-authentication (I've set up this in a large enterprise, given lectures about it)

If you have a normal deployment, you're implicitly trusting the public CA:s and every intermediary they've ever signed. https://security.stackexchange.com/a/158998

Confidentiality between both parties.

Not if you're in an enterprise that intercepts TLS traffic at the network boundary and re-encrypts it with its own certificate. Now you have no way of securely validating your incoming message in your app. (I've been in this position)

It was mentioned earlier that other companies do HMAC on some stuff (most notably companies like Stripe and Github on their webhooks). They do this because they allow non-TLS endpoints (which are a bad idea, that is supported for legacy reasons).

This is false, at least for Github https://developer.github.com/v3/#schema

$ curl -v -i http://api.github.com/hasura/graphql-engine/releases
*   Trying 140.82.118.5...
* TCP_NODELAY set
* Connected to api.github.com (140.82.118.5) port 80 (#0)
> GET /hasura/graphql-engine/releases HTTP/1.1
> Host: api.github.com
> User-Agent: curl/7.64.1
> Accept: */*
>
< HTTP/1.1 301 Moved Permanently
HTTP/1.1 301 Moved Permanently
< Content-length: 0
Content-length: 0
< Location: https://api.github.com/hasura/graphql-engine/releases
Location: https://api.github.com/hasura/graphql-engine/releases

<
* Connection #0 to host api.github.com left intact
* Closing connection 0

Aside: The confidence with which the people in this issue have been repeatedly telling @hugbubby that they're wrong, when they themselves seem to know very little about cryptography or the TLS spec is astounding.

Right back at you 👍

danielkcz commented 4 years ago

@haf It's not funny anymore. Please, if you have a need to explain your great knowledge to others, do it in your own issue that's about picking "the most secure solution". Thank you!

haf commented 4 years ago

@FredyC I agree, it's not about the most secure solution, it's about choosing a good default that is easy to work with and implement. If people like @njaremko directly attack me personally, I feel the need to defend myself.

danielkcz commented 4 years ago

Please, there is a nifty little option "Reference in a new issue" hidden under those 3 dots and you can take your discussion to a different thread with all details. Let's act like civilized people instead of bickering over something repeatedly.

haf commented 4 years ago

@FredyC If you read your thread topic https://github.com/hasura/graphql-engine/issues/4645#issue-610955597 again you'll find this is on topic. @njaremko is partially correct when he says:

On the topic of adding a default, I think the furthest that Hasura should go on this matter is specifying that if you provide an environment variable to Hasura, lets say HASURA_ACTION_SECRET, [and let clients validate incoming message signatures based on this] (square brackets added by me)

Rotation, like I wrote above is easiest done by allowing two concurrent keys in the clients, and verifying the signature against each in turn, to see if at least one matches. It should, again like @njaremko wrote, be outside the purview of Hasura.

But it should be done with HMAC for the above mentioned reasons.

Let's act like civilized people instead of bickering over something repeatedly.

What do you feel I have said that is uncivilized?

njaremko commented 4 years ago

@haf I did not intend to personally attack you when I wrote my above comment.

As @FredyC said, let's keep discussion here on topic. HMAC would be a new authentication scheme for Hasura, so isn't really within the scope of the "built-in" security that this issue is focusing on.

@haf if you want to create another issue specifically focusing on HMAC-based authentication for hasura, I'll discuss with you there.

haf commented 4 years ago

@njaremko Really?

"when they themselves seem to know very little about cryptography or the TLS spec is astounding."

As for the topic then, there already IS security in the action handlers; https://hasura.io/docs/1.0/graphql/manual/actions/action-handlers.html#securing-your-action-handler so then this whole suggestion/thread should be rejected with "already exists", and as for key rotation for this existing security, it is easier/better to do in the clients, so that bit of the topic should be rejected. In my opinion.

njaremko commented 4 years ago

@haf You weren't the only person discussing HMAC above, and I still disagree with much of what you've said above.

To add security to an action handler presently, you must manually configure an environment variable and manually add it to be included in all of your action requests. I've interpreted this issue to focus on the automatic inclusion of an action secret if defined in a hasura environment variable, as I've outlined above.

haf commented 4 years ago

I've interpreted this issue to focus on the automatic inclusion of an action secret if defined in a hasura environment variable, as I've outlined above.

Ok, fair enough, so a solution to that could be, again using a simple primitive on the server, hmac(algo: sha256, key: admin secret as a byte array, message: action name as a utf8-encoded string as a byte array); that way, you can easily secure your actions out of the box, if you know your admin key, and it would be a ~ three-liner to include a CLI command to deduce the action handler key. And key rotation could be by explicitly defining the action key env var; or even better, an "action variable", so you can support multiple action consumers without doing key rotation for all of them.

$ HASURA_ADMIN_KEY='beef' hasura action key-for "https://example.com/v1/purchase-foo-bars"
43312aced847bc29bc4504f4660f04ba10fae8753a0a8c9f6ceac18299e845da