bitnami-labs / sealed-secrets

A Kubernetes controller and tool for one-way encrypted Secrets
Apache License 2.0
7.52k stars 675 forks source link

This is great until the private key is compromised #123

Closed arielb135 closed 5 years ago

arielb135 commented 5 years ago

Unlike regular TLS which builds the symmetric key using a random generated information (which is exchanged at the session negotiation), then discarded afterward (and replaced every 5 minutes) - in here we basically have a static public/private keypair that if one will be compromised - we will be able to decrypt all history of secrets (instead of only 1 specific payload).

Also - if the private key is compromised - how do i replace the key-pair in a simple way? How are we sure that attacker cannot decrypt all of our past secrets?

If an attacker manages to get the private key (or brute force it - which of course is terribly hard), meanwhile - he collects those secrets from github and builds a database with all of the encrypted secrets, the following will happen, as the secret is composed of: 2 byte encrypted session key length || encrypted session key || encrypted Secret.

  1. Attacker managed to get the private key
  2. Attacker now reads the encrypted session key.
  3. Decrypts this key using the compromised private key
  4. uses the symmetric key to now decrypt the encrypted secret

The issue is that he can do it now with all organization's secrets, not only one specific secret for example.

If this solution relies anyway on a data that is saved inside the cluster (the private key) - then why not adding a salt for each secret (that is also saved inside the cluster)

To address this, one enhancement that can be used is to build the key with a random session data that is not exposed in the secret, but is saved inside the cluster (and thus, the attacker will need also an information from within the cluster to build the symmetric decryption key (per secret). so the logic is:

  1. Take random data, create a symmetric key using this data
  2. Save this data inside the cluster (etcd, etc..) - this is also a database that needs to be protected
  3. Encrypt the secret using the symmetric key
  4. Encrypt the symmetric key using the public key
  5. save the payload in a regular secret

This is important to protect the payload - yes, the attacker has the private key, but he won't be able to build the symmetric key as he now needs to guess the random payload (and even if he did - he won't be able to reach other secrets)

anguslees commented 5 years ago

Thanks for such a detailed suggestion. Yes, you're correct - the current sealed-secrets approach relies on the attacker not being able to read the private key from the cluster, and assumes that brute-forcing a 4k RSA key is infeasible (or whatever size/algorithm is chosen).

If I understand your suggestion correctly, I think it:-

  1. Requires access to the cluster at encryption time (to indirectly access the short-lived symmetric key)
  2. The short-lived keys need to be recorded, so compromising the cluster still gives access to all the encryption key(s) and hence ability to decrypt all sealed secrets.

I think (2) is similar to the current approach - a cluster-level compromise exposes the private key material and allows full decryption in both approaches. (Also if you have full access to etcd then you can presumably just read out the decrypted Secrets anyway.) Some variation of this is unfortunately unavoidable: Fundamentally we're trying to design something that has an encrypted blob with an externally managed (and probably long-lived) lifecycle and that can be decrypted by an automated controller in the cluster - so all the required credentials to decrypt every currently-alive SealedSecret has to be available in the cluster somewhere. Ideally the master private key would be stored in hardware that made it impossible to exfiltrate (note an extra symmetric layer is unlikely to have similar protection), but even then the key material still has to be available in some form to every node where the controller might run.

Re (1): The current public key approach was chosen specifically to avoid requiring access to the cluster as in (1). I agree that if we allow ourselves to talk to the cluster at encryption time, then many other approaches become possible - such as using a unique symmetric key for each Secret, and skipping the asymmetric step altogether. The downside of course is that this also exposes the apiserver to more attackers - there would be robot accounts that only have apiserver access in order to encrypt secrets, whereas in the current implementation they don't need apiserver access at all.


Your fundamental point is a good one however: compromise of the key leads to a compromise of all previous SealedSecrets, even those that are not currently online (and thus also available as plaintext Secrets anyway) - and more practically speaking, it is difficult to rotate the master key once a compromise is suspected.

I think all the solutions here come down to rotating the keys in some way - either rotating a separate symmetric key as in your proposal, or just rotating the primary asymmetric key. I think we need to build support for rotating the primary private key anyway, so I think it makes sense to make that smooth and automatic.

Following that line of thought, I would like to raise this counter-proposal:

The "low watermark" allows you to rotate (re-encrypt) your SealedSecrets on a faster schedule and the private key will be "forgotten" earlier than the hard expiry.

I think this makes it only possible to use a compromised private key to decrypt the "window" of SealedSecrets that were encrypted with the matching public key. It's then up to the local user(s) to manage updating their copy of the public key and re-encrypting SealedSecrets "often enough" to keep this window small. It also allows manually-triggered rotation/deletion of encryption keys in the event of a suspected compromise.

How does that sound? Is that sufficient to address the private-key-compromise scenario? Instead of a bunch of symmetric keys with a private key to protect, we now have a bunch of private keys to protect. Instead of online single-use symmetric keys, we now have an offline symmetric key (that may be used more than once, but should be much less than "one key for all"). Importantly with both proposals, old encrypted material will no longer be decryptable with new keys (and vice versa).

Of course none of this matters if the user never rotates the actual credentials stored inside the SealedSecrets/Secrets, but that's someone else's problem...

stephenmoloney commented 5 years ago

The idea of key rotation sounds appealing to me and ties in with this issue. I would like to suggest though that if key rotation is added that the documentation be written well so as to avoid confusion about how it works and what steps users need to take.

Delete keys that have passed a "high watermark" expiry date (year-ish timescale?)

:+1: (with documentation as a warning to users)

Opportunistically garbage collect keys that are not used by any existing SealedSecret, and have passed a "low watermark" expiry date (maybe "grace period" is a better name).

:+1: (with documentation as a warning to users)

Optionally(?) we could re-encrypt the SealedSecrets on the server side with more recent keys, but I fear having the SealedSecret contents change magically could greatly upset declarative (gitops) pipelines.

Optional external integrations with gitops platforms like weaveworks/flux would provide an alternative solution to the issue mentioned above. Some sort of integration with weaveworks would be a really nice feature in harmony with bitnami/sealed-secrets whereby the keys re-encrypted on the server side could then be commited by the flux-operator back into the git repository. Effectively, keeping the gitops approach intact.

It's then up to the local user(s) to manage updating their copy of the public key and re-encrypting SealedSecrets "often enough" to keep this window small

Like mentioned about external integrations with gitops platforms, I think this can be automated with some thought/external integrations/collaborations. The concern I have if it is not automated is that often enough will not be so often for some users and will probably be not be automatic either.

On a separate note, while the original post outlines that the weak point is getting access to the private key, is getting access to the kubernetes cluster not equivalent in terms of being compromised - whether keys have been rotated or not because if one can get access to the kubernetes cluster (by getting the kubernetes cluster credentials), one has access to all sealed secrets and the latest public and private key ?

arielb135 commented 5 years ago

My point actually did not focus rotating the keys (where it has been offered in another post i think), which is of course important - and also did not answer the case where the cluster is compromised, but i do refer to the case where all of organization secrets are encrypted with the same key - you have 1 point of failure that collapses the entire solution.

So we have 2 vectors of attacks:

  1. Compromising the cluster
  2. Guessing / discovering the private key

When talking about security, we need to make the attacker's life hard as possible - either by protecting the cluster (applying all security measures), and also make it hard to discover potential secrets.

If the user has a farm of supercomputers (government lets say?) and it does manage eventually to guess the private key - he will now be able to decrypt everything, so why not adding another layer?

Even if you'll do a private/public key rotation - the attacker can store let's say 10 secrets, and once he gets the private key he will be able to decrypt all of them, instead of now trying to guess each one's symmetric key.

The perfect solution in my end can guide to the following:

  1. each secret will be encrypted using a generated symmetric key
  2. the key will be encrypted using an asymmetric key
  3. a mechanism that periodically re-encrypts the secrets and generates new key pair 3.1. (Plugin?) push the new secrets into github, or wherever the company saves their YAML files. --- Note - if we do save the symmetric keys, the asymmetric encryption might not be needed.

i think it's too much to create multiple asymmetric keys, it's unnecessary. a built-in key rotation is the key here, and will cause the solution to be much secured. but also, avoid giving the attacker an opening to collect all company's secrets (make him work hard for each secret, not only to guess the private key, but for each secret - try to generate the symmetric key)

As also mentioned before, the area that decrypts the info is the weak point - we have to protect it at all cause - for example https://github.com/freach/kubernetes-security-best-practice

but just don't let an attacker to sit offline on your data and eventually find the bingo.

stephenmoloney commented 5 years ago

a built-in key rotation is the key here, and will cause the solution to be much secured. but also, avoid giving the attacker an opening to collect all company's secrets (make him work hard for each secret, not only to guess the private key, but for each secret - try to generate the symmetric key)

Is there a way to achieve this ? for each secret - try to generate the symmetric key

In theory, if this adds another layer of protection and can be achieved in a way that doesn't create other vulnerabilities, then it should be a priority I think.


Re (1): The current public key approach was chosen specifically to avoid requiring access to the cluster as in (1). I agree that if we allow ourselves to talk to the cluster at encryption time, then many other approaches become possible - such as using a unique symmetric key for each Secret, and skipping the asymmetric step altogether.

So if I understand correctly, doing the encryption with access to the cluster allow the possibility of using a unique symmetric key for each sealed-secret.


The downside of course is that this also exposes the apiserver to more attackers - there would be robot accounts that only have apiserver access in order to encrypt secrets, whereas in the current implementation they don't need apiserver access at all

Can you elaborate more on this vulnerability @anguslees. Can this point be explored a bit more as this seems to be the core issue.

anguslees commented 5 years ago

[sorry, this is long - but I wanted to do justice to the discussion]

but just don't let an attacker to sit offline on your data and eventually find the bingo.

I'm explicitly not concerned about this attack vector, given a sufficiently large key size (sealed-secrets currently uses 4k RSA keys). There are numerous other widespread assymetric systems (like PGP/gnupg, SSL/TLS) that also rely on publishing the public key openly, involve significantly more valuable targets, and yet attacks that involve brute forcing the private key remain infeasible even to governments.

I feel like some of @arielb135's concern is that someone might "guess" the private key, if the public key is publicly available. So I want to explicitly address that concern: Sealed-secrets deliberately uses a very straightforward application of the standard golang RSA library function, so this line of thought basically comes down to: 1) the RSA algorithm conceptually is vulnerable (industry-wide issue) a) the key size is insufficient for the value/longevity of intended use cases (this is easy to increase if you wish. NIST suggests 2k is sufficient until 2030, and 4k is already extremely conservative aiui) 2) the golang implementation is vulnerable (language-community-wide issue) 3) the sealed-secrets code uses the RSA library incorrectly (please review! in particular: complicating the encryption path further will make this worse)

If we're happy with the above, then we're happy that brute forcing the private key is infeasible, even if we publish the public key and a bunch of cyphertexts for a decade or so (given 4k key size). This is emotionally counter-intuitive and important.


So afaics the remaining concern (which is a good one!) is "leaking" the private key through some other means (cluster compromise, ex-employee, etc) - and we would like to minimise the damage from this scenario.

These attacks involve an attacker that has complete (admin) access to the current cluster. In particular, they can read all the current (decrypted) Secrets, and if we store multiple keys then they can (presumably) read all those keys with only slightly more difficulty than reading a single master key.

Something I've been trying to do with sealed-secrets is to avoid promising false security, and afaics breaking the master key up into multiple master keys (symmetric or assymetric) that are all stored together in the same server is false security.

So: if an attacker can read the master secret(s), then they can decrypt all the currently "valid" SealedSecrets. There's no way we can design around this: fundamentally the ability to decrypt a SealedSecret has to persist for as long as we want that particular SealedSecret to work. Given the declarative/gitops use case, we want this to be long-ish (weeks to year; definitely not hours). So we have to accept that an attacker that pwns the cluster can also decrypt all the SealedSecrets that were "valid" at that time.

This is my primary concern with the original proposal in this issue (as written) - we've added complexity (in both implementation and explanation), but a cluster-level compromise still exposes all "valid" SealedSecrets just by vacuuming out all the symmetric keys too.


Once we separate out these aspects, the bit that I like from the original proposal is that SealedSecrets don't remain "valid" forever. New keys are generated and can eventually be removed, and admins can use this removal to recover from a suspected cluster compromise, etc. Rotating the primary key is clearly desirable and important and we need to have the ability to do this for many reasons. My understanding is that rotating the master key also addresses the remaining concern behind this issue - and that the other proposed alternatives all come down to being equivalent to rotating the master key.

(Obviously the local admin also needs to rotate all the secrets within any compromised SealedSecrets too, but that is not within our technical scope)

stephenmoloney commented 5 years ago

@anguslees Your previous post makes sense.

So this would bring us back to your original proposal:

Following that line of thought, I would like to raise this counter-proposal:

Keep a list of private keys, publish the most recent public key Generate a new private key (and public key) periodically automatically, on a monthly/weekly timescale. We could also re-generate based on the number of newly created SealedSecrets, although this exposes a DoS by authenticated users (trigger the creation of many keys). We could also do this per-namespace or similar. Store a (plaintext/unprotected) "key id" in the encrypted SealedSecret, and use that to cheaply find the key for decryption. Delete keys that have passed a "high watermark" expiry date (year-ish timescale?) Opportunistically garbage collect keys that are not used by any existing SealedSecret, and have passed a "low watermark" expiry date (maybe "grace period" is a better name). Optionally(?) we could re-encrypt the SealedSecrets on the server side with more recent keys, but I fear having the SealedSecret contents change magically could greatly upset declarative (gitops) pipelines.

The one thing I mentioned before is that I think a gitops approach to pushing the rotated secrets back into the source git repo as an optional feature is a worthy addition to that list.

anguslees commented 5 years ago

Closing in favour of #120 - please comment/reopen if you think there are additional remaining issues arising from the above discussion.

(and thanks again to everyone above for such thoughtful, in-depth discussion)