Pin notAfter dates for certificates to max out at signing CA notAfter date #378
Conversation
date This prevents one from issuing a certificate that appears to be valid, but in reality typically won't validate as being so. Partially fixes cloudfoundry#377
|
We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story. The labels on this github issue will be updated when the story is started. |
|
|
Accepting the CLA from our env is challenging (we've clicked through CLAs before for I think other parts of Cloudfoundry, but that particular one seems to be more complex for us to navigate for some reason). It's a trivial PR, feel free to pick up the few lines added into a new one if necessary. Else, and if you'd like this change, let me know and I'll try harder to get the CLA organised. |
| @@ -93,11 +93,24 @@ private X509Certificate getSignedByIssuer( | |||
|
|
|||
| final BigInteger certificateSerialNumber = serialNumberGenerator.generate(); | |||
|
|
|||
| Date nvb = Date.from(now); | |||
| Data nva = Date.from(now.plus(Duration.ofDays(params.getDuration()))); | |||
There was a problem hiding this comment.
This doesn't compile - should be "Date". Please run "gradlew test" after fixing.
| @@ -93,11 +93,24 @@ private X509Certificate getSignedByIssuer( | |||
|
|
|||
| final BigInteger certificateSerialNumber = serialNumberGenerator.generate(); | |||
|
|
|||
| Date nvb = Date.from(now); | |||
There was a problem hiding this comment.
I suggest more descriptive names like notBefore and notAfter.
| @@ -93,11 +93,24 @@ private X509Certificate getSignedByIssuer( | |||
|
|
|||
| final BigInteger certificateSerialNumber = serialNumberGenerator.generate(); | |||
|
|
|||
| Date nvb = Date.from(now); | |||
| Data nva = Date.from(now.plus(Duration.ofDays(params.getDuration()))); | |||
| if (issuerCertificate != null) { | |||
There was a problem hiding this comment.
Could you add unit test coverage for this new logic? I'm assuming we didn't already have good assertions for it, even though the tests hit this code.
| Data nva = Date.from(now.plus(Duration.ofDays(params.getDuration()))); | ||
| if (issuerCertificate != null) { | ||
| // don't allow a child certificate to be valid before the signing certificate | ||
| if (nvb.isBefore(issuerCertificate.getNotBefore())) { |
There was a problem hiding this comment.
There isn't an isBefore method on Date, but you can use before.
| nvb = issuerCertificate.getNotBefore(); | ||
| } | ||
| // don't allow a child certificate to be valid after the signing certificate | ||
| if (nva.isAfter(issuerCertificate.getNotAfter())) { |
There was a problem hiding this comment.
isAfter should be `after'.
|
@ae-govau Do you plan to complete the CLA in the near future? |
This prevents one from issuing a certificate that appears to be valid, but in reality typically won't validate as being so.
Partially fixes #377