-
Notifications
You must be signed in to change notification settings - Fork 580
HDDS-13781. Certificate expiry date should consider time zone daylight saving impact #9209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,8 +34,10 @@ | |||||||
| import java.security.cert.CertPath; | ||||||||
| import java.security.cert.CertificateException; | ||||||||
| import java.security.cert.X509Certificate; | ||||||||
| import java.time.Duration; | ||||||||
| import java.time.LocalDateTime; | ||||||||
| import java.time.ZoneId; | ||||||||
| import java.time.ZonedDateTime; | ||||||||
| import java.util.Date; | ||||||||
| import java.util.List; | ||||||||
| import java.util.concurrent.CompletableFuture; | ||||||||
|
|
@@ -205,8 +207,7 @@ public Future<CertPath> requestCertificate( | |||||||
| PKCS10CertificationRequest csr, | ||||||||
| CertificateApprover.ApprovalType approverType, NodeType role, | ||||||||
| String certSerialId) { | ||||||||
| LocalDateTime beginDate = LocalDateTime.now(); | ||||||||
| LocalDateTime endDate = expiryFor(beginDate, role); | ||||||||
| Duration certDuration = getDuration(role); | ||||||||
|
|
||||||||
| CompletableFuture<Void> csrInspection = approver.inspectCSR(csr); | ||||||||
| CompletableFuture<CertPath> certPathPromise = new CompletableFuture<>(); | ||||||||
|
|
@@ -224,7 +225,7 @@ public Future<CertPath> requestCertificate( | |||||||
| break; | ||||||||
| case KERBEROS_TRUSTED: | ||||||||
| case TESTING_AUTOMATIC: | ||||||||
| X509Certificate signedCertificate = signAndStoreCertificate(beginDate, endDate, csr, role, certSerialId); | ||||||||
| X509Certificate signedCertificate = signAndStoreCertificate(certDuration, csr, role, certSerialId); | ||||||||
| CertificateCodec codec = new CertificateCodec(config, componentName); | ||||||||
| CertPath certPath = codec.getCertPath(); | ||||||||
| CertPath updatedCertPath = codec.prependCertToCertPath(signedCertificate, certPath); | ||||||||
|
|
@@ -240,27 +241,30 @@ public Future<CertPath> requestCertificate( | |||||||
| return certPathPromise; | ||||||||
| } | ||||||||
|
|
||||||||
| private LocalDateTime expiryFor(LocalDateTime beginDate, NodeType role) { | ||||||||
| private Duration getDuration(NodeType role) { | ||||||||
| // When issuing certificates for sub-ca use the max certificate duration similar to self-signed root certificate. | ||||||||
| if (role == NodeType.SCM) { | ||||||||
| return beginDate.plus(config.getMaxCertificateDuration()); | ||||||||
| return config.getMaxCertificateDuration(); | ||||||||
| } | ||||||||
| return beginDate.plus(config.getDefaultCertDuration()); | ||||||||
| return config.getDefaultCertDuration(); | ||||||||
| } | ||||||||
|
|
||||||||
| private X509Certificate signAndStoreCertificate( | ||||||||
| LocalDateTime beginDate, LocalDateTime endDate, PKCS10CertificationRequest csr, NodeType role, String certSerialId | ||||||||
| Duration duration, PKCS10CertificationRequest csr, NodeType role, String certSerialId | ||||||||
| ) throws IOException, OperatorCreationException, CertificateException { | ||||||||
|
|
||||||||
| ZonedDateTime beginDate = ZonedDateTime.now(); | ||||||||
| ZonedDateTime endDate = beginDate.plus(duration); | ||||||||
|
|
||||||||
| lock.lock(); | ||||||||
| X509Certificate xcert; | ||||||||
| try { | ||||||||
| Preconditions.checkState(!Strings.isNullOrEmpty(certSerialId)); | ||||||||
| xcert = approver.sign(config, | ||||||||
| getPrivateKey(), | ||||||||
| getCACertificate(), | ||||||||
| Date.from(beginDate.atZone(ZoneId.systemDefault()).toInstant()), | ||||||||
| Date.from(endDate.atZone(ZoneId.systemDefault()).toInstant()), | ||||||||
| Date.from(beginDate.toInstant()), | ||||||||
| Date.from(endDate.toInstant()), | ||||||||
| csr, scmID, clusterID, certSerialId); | ||||||||
| if (store != null) { | ||||||||
| store.checkValidCertID(xcert.getSerialNumber()); | ||||||||
|
|
@@ -487,14 +491,14 @@ private void generateRootCertificate( | |||||||
| throws IOException, SCMSecurityException { | ||||||||
| Preconditions.checkNotNull(this.config); | ||||||||
| LocalDateTime beginDate = LocalDateTime.now(); | ||||||||
|
||||||||
| LocalDateTime beginDate = LocalDateTime.now(); | |
| ZonedDateTime beginDate = ZonedDateTime.now(); |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, here:
ZonedDateTime beginDate = ZonedDateTime.now();
ZonedDateTime endDate = beginDate.plus(securityConfig.getMaxCertificateDuration());There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ChenSammi for updating the patch. It seems this comment was missed.
(Note: LocalDateTime and ZoneId imports will be unused after this change.)
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ZoneId zoneId = ZoneId.systemDefault(); | |
| ZonedDateTime endDate = beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration()); | |
| ZonedDateTime endDate = beginDate.plus(securityConfig.getMaxCertificateDuration()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .setBeginDate(beginDate) | |
| .setBeginDate(beginDate.toLocalDateTime()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.