Implemented device certificate with Y10K expiry for certificate by providing the not_after field #12795
Conversation
|
|
skhilar
changed the title
|
Hi @skhilar, before we can review this, please be sure to sign the CLA. Thanks! |
hghaf099
added
enhancement
feature-request
secret/pki
and removed
feature-request
labels
|
we also need a changelog for this PR. Please follow this link for instructions https://github.com/hashicorp/vault/blob/main/CHANGELOG.md |
|
@skhilar It looks like the email used to author the commit is different than the one in your profile. You might want to add that email to your GitHub profile (maybe you'll need to resign the CLA in that case?), or update the commit to link to your mail ( |
There was a problem hiding this comment.
Thanks for getting this updated for the CLA, @skhilar.
I talked with @sgmiller about this and I think we'd both prefer if this didn't introduce any new flags, but changed the behavior of the TTL flag. What if we change TTL as follows:
"ttl": {
Type: framework.TypeDurationSecond,
Description: `The lease duration if no specific lease duration is
requested. The lease duration controls the expiration
of certificates issued by this backend. Defaults to
the value of max_ttl. If value exceeds 10,000 years
(3600 * 24 * 365 * 1000 seconds), the certificate's
notAfter field will be set to the IEEE 802.1AR-2018
standard's Y10K value (9999-12-31T23:59:59Z).`,
DisplayAttrs: &framework.DisplayAttributes{
Name: "TTL",
},
},Then in cert_util.go, you can detect a large value and update notAfter appropriately. I think that'd be a cleaner approach, rather than introducing two new flags that are (presently) mostly duplicates.
Thoughts? Are there other values of IEEE standard with different notAfter times that might be useful?
Another alternative we liked was introducing a proper notAfter field as an alternative to the TTL field.
Let us know what you think :)
builtin/logical/pki/path_roles.go
Outdated
| }, | ||
| "ieee_profile": { | ||
| Type: framework.TypeString, | ||
| Description: `Set notAfter field to Y10K to compliant with IEEE 802.1AR-2018 standard for |
There was a problem hiding this comment.
| Description: `Set notAfter field to Y10K to compliant with IEEE 802.1AR-2018 standard for | |
| Description: `Set notAfter field to Y10K to comply with IEEE 802.1AR-2018 standard for |
There was a problem hiding this comment.
(not relevant if review suggestion is taken about TTL).
There was a problem hiding this comment.
Added not_after field which accepts date value in UTC with format YYYY-MM-ddTHH:MM:SSZ. Same value will be set as notAfter field for the certificate as an alternative to ttl.
skhilar
changed the title
|
I have raised PR introducing "not_after" field. @sgmiller and @cipherboy please review and provide your feedback. |
|
@sgmiller and @cipherboy thank you very much for the feedback. I have added a validation for the same. Both "ttl" and "not_after" should not be present in the same request. Please review the code and provide your feedback. |
|
@sgmiller and @cipherboy did you get chance to review the code? I have added the validation for TTL and NotAfter. Please review the code and provide your comments. |
|
@sgmiller this looks good to me -- what about you? |
| } | ||
| if resp == nil { | ||
| t.Fatal("expected ca info") | ||
| } |
There was a problem hiding this comment.
In these tests, can we retrieve the resulting cert from the CA endpoint and validate the correct NotAfter value?
There was a problem hiding this comment.
@sgmiller I have addressed the review comments. Please have a look.
| @@ -0,0 +1,3 @@ | |||
| ```release-note:feature | |||
There was a problem hiding this comment.
We use a particular changelog format for new features. I think this is more of an improvement, so I moved it to that section. @cipherboy I'm going to tag you on an enterprise PR if you don't mind giving it a look.
fixes #12157
As per the IEE 802.1AR-2018 standard, the notAfter field of the IDevID certificate should use the Generalized Time value 99991231235959Z. However, in the PKI secrets engine currently, the max ttl value that can be specified is ~290 years.
Support an additional device_flag in certificate roles, which, when set ignores the ttl field and sets the notAfter value to Y10K.