autoTls: bound certificate lifetime by the CA that will actually sign it, not the configured caCertificateLifetime
Nessuno ha ancora preso questa issue.
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 48/100
- Tipo di issue
- Bug
- Chiarezza
- Abbastanza chiara
- Stato di attività
- Attiva
- Stack tecnologico
- kubernetes, rust
- Ambito
- infrastructure, security
Direzione di ricerca
Inizia da TlsGenerate::get_or_create_k8s_certificate, chiamata mentre NodePublishVolume costruisce il backend, e esamina ca::Manager::load_or_create per il caricamento dei certificati CA del Secret. Implementa il limite di durata utilizzando la scadenza più lontana della CA, mantenendo il fallimento rigido esistente quando nessuna CA copre l’istante attuale più il buffer di riavvio. Esamina il comportamento esistente degli avvisi o delle condizioni e convalida la durata del certificato e i casi limite di errore con i test pertinenti.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
get_or_create_k8s_certificate clamps maxCertificateLifetime to (caCertificateLifetime - caCertificateRetirementDuration) / 4.
That is good in general, bad when someone brings their own CA as secret operator never looks at the actual CA that is provided. It just looks at what is in the CR:
ca:
secret:
name: secret-provisioner-tls-ca
namespace: default
autoGenerate: false
caCertificateLifetime: 700d
caCertificateRetirementDuration: 1d
maxCertificateLifetime = (700 - 1) / 4 = 174 (or 175, not sure, doesn't matter here)
That clamps to 174d . But we have no idea what's really in the CA.
If it has 200 days left:
- above 174 days remaining: works
- 174 down to 139: flip-flops, because jitter puts each cert somewhere in 139-174d and only some of them still fit
- below 139: every mount fails
That is not great.
It only happens when autoGenerate is false, but still....
Scope
- Calculate the clamp by looking at the CA in the Secret that expires last
- Keep it a hard failure where no CA covers
now + restartBuffer - Warning Event and/or condition on the Pod when the cert had to be shortened because of the CA's remaining lifetime?
- Open question....do we do this elsewhere? Easy to do? I think it'd be good.
[!NOTE]
This next bit is complicated and I'm not 100% sure it is all correct. I think it makes sense but I have a nagging feeling that something is still off. It is 100% written by hand but I'm not sure if the new steps 3 & 4 are correct to be honest. So many edge cases.
When a pod mounts a TLS volume, NodePublishVolume builds the backend for that SecretClass.
For autoTls that is TlsGenerate::get_or_create_k8s_certificate, which does, in this order:
- safe_max_cert_lifetime = (caCertificateLifetime - caCertificateRetirementDuration) / 4, from the CR fields
- max_cert_lifetime = min(maxCertificateLifetime, safe_max_cert_lifetime). This is the clamp.
- ca::Manager::load_or_create, which reads the CA certificates from the Secret
We need to load the CAs (step 3) before computing the clamp (step 2), and additionally bound it (the max cert lifetime) by the remaining lifetime (not_after - now, minus caCertificateRetirementDuration) of the CA that expires last. This way we never create certificates that can outlive the last expiring CA.
- ca::Manager::load_or_create, reads the CA certificates from the Secret
- safe_max_cert_lifetime = (caCertificateLifetime - caCertificateRetirementDuration) / 4, from the CR, unchanged
- ca_bound = not_after(CA that expires last) - now - caCertificateRetirementDuration, from the loaded certificates. This is new.
- max_cert_lifetime = min(maxCertificateLifetime, safe_max_cert_lifetime, ca_bound)
[!NOTE]
Do we even still needsafe_max_cert_lifetime? I believe it makes sure that all Pods can still talk to each other even if some already have a new CA and others don't but 🤯
Open questions
- Check the existing warnings whether they are correctly calculated and/or whether we want something new
- Event or Pod condition or both? An Event per mount is noisy on a large StatefulSet, otoh we might want noisy in this case?
- I believe it'd be easy to fold #94 into this. If you agree and it adds less than a day you're welcome to do so. Otherwise skip.
- I understand that this is underspecified but at least a log line and maybe another condition woudl be good here?
Out of scope
- Validating the SecretClass on admission
Expected effort
1-2 days. If it takes longer, stop and flag it.
- Lingua principale
- Rust
- Stelle
- 13
- Fork
- 8
- Merge medio
- 1g 8h
- PR unite (30g)
- 10
Guida per i contributori
Nessuna guida per i contributori indicizzata per questo repository
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di stackabletech/secret-operator
-
type/bug
Difficoltà 4/5 3-5 giorni Idoneità per principianti 45/100
stackabletech/secret-operator#754 ·
-
Difficoltà 3/5 1-2 giorni Idoneità per principianti 72/100
stackabletech/secret-operator#720 · 1 commento ·
-
customer-request type/bug
Difficoltà 3/5 1-2 giorni Idoneità per principianti 38/100
stackabletech/secret-operator#666 · 2 commenti ·
-
customer-request type/feature-improvement
stackabletech/secret-operator#630 · 7 commenti · 1 assegnatario ·
-
stackabletech/secret-operator#620 · 2 commenti · 1 assegnatario ·
Tutte le issue di stackabletech/secret-operator
Issue simili
-
risk:low runtime status:in-progress type:test
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 92/100
zeroclaw-labs/zeroclaw#11023 ·
-
good first issue refactor
Difficoltà 2/5 1-3 ore Idoneità per principianti 72/100
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 86/100
kwakseongjae/auto-hwp#319 ·
-
area:cli bug filter-quality good first issue priority:medium
Difficoltà 2/5 1-3 ore Idoneità per principianti 84/100
-
Difficoltà 1/5 Meno di un'ora Idoneità per principianti 72/100
bevyengine/bevy#25861 ·