Allow specifying a threshold to prevent soon-expiring CAs from being published

[dervoeti]

Needed for #625

I propose adding a new optional field caExpiryThreshold to the TrustStore CRD resource.

apiVersion: secrets.stackable.tech/v1alpha1
kind: TrustStore
metadata:
  name: truststore-cas
spec:
  secretClassName: tls-ca
  format: tls-pem
  caExpiryThreshold: 1d

CAs that are expired or will expire in the next 24 hours would not be available in the ConfigMap.

Currently, even expires CAs are persent in the ConfigMap. That behavior would not change if caExpiryThreshold is not defined, so the change is backwards compatible.

Implementation: #633

[NickLarsenNZ]

Do we think there will be additional CA related configs? If so, maybe it can be like:

apiVersion: secrets.stackable.tech/v1alpha1
kind: TrustStore
metadata:
  name: truststore-cas
spec:
  secretClassName: tls-ca
  format: tls-pem
  ca:
    expiryThreshold: 1d

Not saying it should... but we should think about what could also be configurable in that context

[sbernauer]

I think the property could be a bit more descriptive, that this is about filtering what CAs are included.
I think something like this makes this a bit more clear:

spec.includeCaExpirationThreshold
spec.includeCaRemainingLifetimeThreshold
spec.caExpirationThresholdFilter

For all variants we can also put them below spec.includeCAs of spec.ca.include (e.g. spec.includeCAs.remainingLifetimeThreshold), this would give some place for other CA filtering.

Regarding expiry vs expiration: https://www.reddit.com/r/MandelaEffect/comments/sbk3uv/expiration_or_expiry_date/ or "Google KI"

"Expiry" and "expiration" both mean the end or termination of something, but "expiration" is preferred in American English and "expiry" is preferred in British English.

I personally prefer American English and therefore expiration, but that's up for discussion ;)

[Techassi]

I don't like the name caExpiryThreshold. It doesn't convey that CAs which are considered expired according to the threshold are not published. Alternative CRD designs might be:

• publishThreshold (or caPublishThreshold, depending on the nesting): With this option, expired CAs are automatically not published anymore.
• Split it into two settings (Nesting is recommended):
  • expiryThreshold: Set the expire threshold
  • puplishExpired: Set if expired CAs should be published (true/false)

[siegfriedweber]

The meaning of the TrustStore is to make certificates validable. If a pod sends a valid certificate signed by a non-expired CA, then the TrustStore must contain this CA. It is not the task of the TrustStore to invalidate valid certificates by excluding issuer certificates by a threshold.

The SecretClass must define how long a CA is used and all issuer certificates must be exposed as long as valid certificates exist.

[dervoeti]

Hmmm. I see your point.

I initially wanted to add it to the SecretClass, but decided against it, because:
The SecretClass defines the source of the certificates, but each workload that uses the SecretClass in one way or another could decide on its own whether it wants to tolerate expired CAs and if not, what the threshold should be.
Different workloads can have different requirements and startup times, while using the same SecretClass.
Of course, only CA certificates matching that condition can sign certificates. That means by default, only valid CAs can sign new certificates.
And if a workload specifies a threshold, only CAs that won't expire within that threshold can sign new certs.

That probably works great for each workload independently. But I see that this can lead to two different workloads not communicating with each other even though they use the same SecretClass, because one defined a threshold and the other one didn't. Which is a problem. You could say that once the workload has started, the "old" CAs should have expired anyway and thus no certificates signed by this CA should be used. But you can't rely on it because the threshold won't exactly match the startup time.

If we define it in the SecretClass, then the workload with the highest requirements will impose these conditions on all other workloads that use the same SecretClass. But I think that's needed if workloads using the same SecretClass want to reliably communicate with each other.

@sbernauer I'd like to have at least your opinion on this as well, I think that you were also in favor of the annotation/TrustStore solution but I don't know if you had other arguments than me or agree with the above reasoning.

[nightkr]

It has to go on the SecretClass since it also affects certificate issuance (you want to use the oldest CA that all workloads will trust for the lifetime of the certificate). It's also confusing if this only affects explicitly requested CA bundles (via TrustStore) and not the implicit bundles injected by secret volumes.

[nightkr]

It might make more sense to conceptualize it as a safety margin between its stated expiry time, and when we take it out of service. (Beyond which time there should no longer exist any valid certificates issued by it.)

[sbernauer]

Thanks all (especially @nightkr!) for their comments!

I though we raised this decision after noticing that opensearch crashes in case there is an expired CA in it's truststore.

So first try is easy: Slap an attribute "I only want CAs that are valid as long as my cert is" on the volume and only use that for opensearch.

That introduces the (IMHO unlikely) case that a third party is signed by a CA that is valid on opensearch startup, but shorter than the opensearch certificate. So the CA is not added and opensearch does not tust the third-party cert.

To fix this we could need a threshold to the SecretClass (can't be anywhere else) which tells secret-op to pick a CA for all certificate signings that lives as long as the to be signed cert + threshold.
But do we really need this threshold attribute? Can't we just go with spec.backend.autols.maxCertificateLifetime?
As we know a cert can have a maximum valid lifetime of t, we know t <= maxCertificateLifetime. Not having a threshold makes things IMHO less complex and reduces potential errors (in the form of a Pod requesting a cert lifetime that is greater than t).

Summary of my idea

1. Don't change the SecretClass.
2. CA picking now only picks CA with a lifetime of cert lifetime + spec.backend.autols.maxCertificateLifetime
3. opensearch sets a new volume attribute: "I only want CAs that are valid as long as my cert is": "true". Attribute name TBD
4. OPTONAL: Add a "I only want non-expired CAs" to the TrustStore CRD. But don't offer a threshold here (at least for now)

What do you think of this? :)

[siegfriedweber]

opensearch sets a new volume attribute: "I only want CAs that are valid as long as my cert is": "true".

A pod does not verify its own certificate. It uses the CA to verify the certificates of other pods, i.e. of other OpenSearch nodes. Therefore, the correct volume attribute would be "I only want CAs that are valid as long as other certs are". And that can be achieved by adding the threshold to the SecretClass.

Not having a threshold makes things IMHO less complex

Your suggestions are more complex than just adding the threshold to the SecretClass. There are already a bunch of hard-coded constants (https://github.com/stackabletech/secret-operator/blob/release-25.7/rust/operator-binary/src/backend/tls/mod.rs#L43-L66). If the implementation should be as little complex as possible, then just add another constant.

[dervoeti]

If the implementation should be as little complex as possible, then just add another constant.

I think I wouldn't add a constant but instead a new optional field to the SecretClass, to be backwards compatible with the current behavior when upgrading SDP. So filtering CAs would be opt-in, like this:

kind: SecretClass
spec:
  backend:
    autoTls:
      ca:
        autoGenerate: true
        caCertificateLifetime: 30d
        publishThreshold: 10m # New CRD attribute, optional

[sbernauer]

opensearch sets a new volume attribute: "I only want CAs that are valid as long as my cert is": "true".

A pod does not verify its own certificate

Correct, let me rephrase:

opensearch sets a new volume attribute: "I only want CAs that are valid as long as my Pod is going to live. My Pod is at a maximum living as long as my certificate is valid": "true".

Maybe I'm under a false assumption, so a relevant question: What happens when opensearch starts up and a CA expires while opensearch is running? I assumed it would crash, but might be wrong here. Maybe this is the mismatch of our thinking.

[siegfriedweber]

What happens when opensearch starts up and a CA expires while opensearch is running?

I tested that case and it continued to run. The check happens apparently only on startup.

[siegfriedweber]

decided in #625 (comment)
implemented in #650