Add support for global secret changing

[ianseyer]

Currently, the global secret value used for encrypting/decrypting strings is unable to change. See goharbor/harbor-helm#325 (comment)

The value is specified here (when using helm): https://github.com/goharbor/harbor-helm/blob/main/values.yaml#L360

and used by: https://github.com/goharbor/harbor/blob/main/src/lib/encrypt/encrypt.go

More technically, it is able to change, which results in Harbor being unable to decrypt already-encrypted values. This puts harbor into an inoperable state. Additionally, once changed, even reverting the secret does not make Harbor healthy again (I imagine because the new value has been used to encrypt new strings? Unsure.)

This means that a simple value change being applied will break Harbor, seemingly forever.

I see a three-pronged solution to this problem:

1. Documentation
   Provide documentation that clearly indicates that this value must not change. Additionally, documentation must be provided that guides users on how to repair Harbor if the value does change.

2. Logging & Locking to allow reversion
   Harbor should, if the global secret were to change, preempt issues by refusing to perform encryption/decryption and immediately log a FATAL error linking users to the documentation from # 1.
   This could potentially be accomplished by, on boot, immediately attempting to decrypt a secret. If that fails, abort boot.

3. Supporting changing
   Harbor should provide tooling to support changing this secret. Plenty of companies (ours included) have policies around rotations and the like. Additionally, it would be trivial for an engineer to mistakenly change the value.

I imagine this would work as follows, though I am not sure:

• use the current global-secret to decrypt all secrets
• encrypt all secrets using the new global-secret
• discard the old global-secret

I have flagged this as [Urgent] primarily because it there is no documentation around how to revert this change; I view an "undo" guide to be the MVP for a solution here. We recently had to nuke our staging postgres environment because we changed this secret value. If this has happened in production, we would likely look for alternatives to Harbor.

[ianseyer]

Additional followup question: is this encryption/decryption even necesary? If traffic & is encrypted in transit and at rest, what is the purpose of this security layer?

I am asking to probe wether or not this security measure exists in the wrong layer of the application stack. Can it be disabled entirely?

The current security model does not function: if this secret is ever compromised, there is no way to change the value.

[reasonerjt]

@ianseyer
I recall it was added in a very early security review to make sure when DB is compromised, the sensitive data like password for external registry (replication target) will not be leaked.

But I agree this is a gap that the secret can't be rotated. Do you think it would be helpful if we introduce an API to allow Harbor's system admin to update the secret? When it's called all the persisted data will be updated and re-encrypted with the new secret.

[ianseyer]

I would argue that this is security being implemented at the wrong layer - I don't think Harbor should be responsible for encrypting db contents. I would rather this feature be removed. Modern databases are encrypted at rest and require TLS.

If that is (however unfortunately) not feasible without a new major release (e.g. a feature flag?), then an API endpoint to update the secret would be nice, with documentation describing how to do that for both helm and non-helm deployments.

Additionally, I think a supplemental document should be written for all current releases outlining how to recover if this secret is ever changed. As mentioned, this was responsible for wiping out one of our environments.

[github-actions]

This issue is being marked stale due to a period of inactivity. If this issue is still relevant, please comment or remove the stale label. Otherwise, this issue will close in 30 days.