Support for generating Delta CRLs #16773
Conversation
cipherboy
force-pushed
the
cipherboy-identify-issuer-on-revocation
branch
2 times, most recently
from
c99f0c2
to
93da855
Compare
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
2 times, most recently
from
032235f
to
76bad87
Compare
cipherboy
changed the base branch from
cipherboy-identify-issuer-on-revocation
to
main
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
2 times, most recently
from
05f2f2f
to
b814748
Compare
| } | ||
|
|
||
| // Last is setup during newCRLBuilder(...), so we don't need to deal with | ||
| // a zero condition. |
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
4 times, most recently
from
48b668d
to
ceef225
Compare
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
from
0301ba7
to
a91cb51
Compare
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
from
a91cb51
to
442cc4c
Compare
| cfg, err := cb.getConfigWithUpdate(sc) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if !cfg.EnableDelta { | ||
| // We explicitly do not update the last check time here, as we | ||
| // want to persist the last rebuild window if it hasn't been set. | ||
| return nil | ||
| } | ||
|
|
||
| deltaRebuildDuration, err := time.ParseDuration(cfg.DeltaRebuildInterval) | ||
| if err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Could this happen prior to us grabbing the _builder.Lock()?
There was a problem hiding this comment.
Do we want to fix the config instead? Or do you want to delay the lock until after this conditional, so if fetching the config in the first place fails, we bail?
That said, we shouldn't bail here as we verify this condition during setting of the config :P
There was a problem hiding this comment.
Either option is acceptable to me, if we are fixing then the lock prior makes sense. If we are just going to bail might as well do it prior to grabbing the lock.
There was a problem hiding this comment.
Should we wait to grab the lock until we commit to doing the actual rebuild then...? Hmmm
| if _, err := time.ParseDuration(deltaRebuildInterval); err != nil { | ||
| return logical.ErrorResponse(fmt.Sprintf("given delta_rebuild_interval could not be decoded: %s", err)), nil | ||
| } | ||
| config.DeltaRebuildInterval = deltaRebuildInterval |
There was a problem hiding this comment.
Validate that we haven't been passed in a value less than periodFunc interval?
There was a problem hiding this comment.
Sadly we don't have a response right now for this handler, so we can't add warnings... Do we want to return the set structure instead?
We (abuse) this code path in the test cases right now, like we do with AutoRebuild grace period, maybe we should later refactor the config to return the set config and add the warnings?
While switching to periodic rebuilds of CRLs alleviates the constant rebuild pressure on Vault during times of high revocation, the CRL proper becomes stale. One response to this is to switch to OCSP, but not every system has support for this. Additionally, OCSP usually requires connectivity and isn't used to augment a pre-distributed CRL (and is instead used independently). By generating delta CRLs containing only new revocations, an existing CRL can be supplemented with newer revocations without requiring Vault to rebuild all complete CRLs. Admins can periodically fetch the delta CRL and add it to the existing CRL and applications should be able to support using serials from both. Because delta CRLs are emptied when the next complete CRL is rebuilt, it is important that applications fetch the delta CRL and correlate it to their complete CRL; if their complete CRL is older than the delta CRL's extension number, applications MUST fetch the newer complete CRL to ensure they have a correct combination. This modifies the revocation process and adds several new configuration options, controlling whether Delta CRLs are enabled and when we'll rebuild it. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
Thanks Steve! Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
from
442cc4c
to
fb8dd84
Compare
|
@stevendpclark I've updated the PR so it only invokes the periodic func on active nodes. |
We need to ensure we read the updated config (in case of OCSP request handling on standby nodes), but otherwise want to avoid CRL/DeltaCRL re-building. Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
cipherboy
force-pushed
the
cipherboy-delta-crls
branch
from
10702f6
to
ea54cb0
Compare
While switching to periodic rebuilds of CRLs alleviates the constant
rebuild pressure on Vault during times of high revocation, the CRL
proper becomes stale. One response to this is to switch to OCSP, but not
every system has support for this. Additionally, OCSP usually requires
connectivity and isn't used to augment a pre-distributed CRL (and is
instead used independently).
By generating delta CRLs containing only new revocations, an existing
CRL can be supplemented with newer revocations without requiring Vault
to rebuild all complete CRLs. Admins can periodically fetch the delta
CRL and add it to the existing CRL and applications should be able to
support using serials from both.
Because delta CRLs are emptied when the next complete CRL is rebuilt, it
is important that applications fetch the delta CRL and correlate it to
their complete CRL; if their complete CRL is older than the delta CRL's
extension number, applications MUST fetch the newer complete CRL to
ensure they have a correct combination.
This modifies the revocation process and adds several new configuration
options, controlling whether Delta CRLs are enabled and when we'll
rebuild it.
Add-- waiting on this until asked fordelta_crl_pathto AIA URLs (rebase chain after Add per-issuer AIA URI information to PKI secrets engine #16563 merges) --- requires new X509 extension.