HDDS-13781. Certificate expiry date should consider time zone daylight saving impact

Author: ChenSammi

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java

@@ -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;
@@ -206,7 +208,7 @@ public Future<CertPath> requestCertificate(
       CertificateApprover.ApprovalType approverType, NodeType role,
       String certSerialId) {
     LocalDateTime beginDate = LocalDateTime.now();
-    LocalDateTime endDate = expiryFor(beginDate, role);
+    Duration certDuration = getDuration(beginDate, role);
 
     CompletableFuture<Void> csrInspection = approver.inspectCSR(csr);
     CompletableFuture<CertPath> certPathPromise = new CompletableFuture<>();
@@ -224,7 +226,7 @@ public Future<CertPath> requestCertificate(
         break;
       case KERBEROS_TRUSTED:
       case TESTING_AUTOMATIC:
-        X509Certificate signedCertificate = signAndStoreCertificate(beginDate, endDate, csr, role, certSerialId);
+        X509Certificate signedCertificate = signAndStoreCertificate(beginDate, certDuration, csr, role, certSerialId);
         CertificateCodec codec = new CertificateCodec(config, componentName);
         CertPath certPath = codec.getCertPath();
         CertPath updatedCertPath = codec.prependCertToCertPath(signedCertificate, certPath);
@@ -240,18 +242,21 @@ public Future<CertPath> requestCertificate(
     return certPathPromise;
   }
 
-  private LocalDateTime expiryFor(LocalDateTime beginDate, NodeType role) {
+  private Duration getDuration(LocalDateTime beginDate, 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
+      LocalDateTime beginDate, Duration duration, PKCS10CertificationRequest csr, NodeType role, String certSerialId
   ) throws IOException, OperatorCreationException, CertificateException {
 
+    ZoneId zoneId = ZoneId.systemDefault();
+    ZonedDateTime endDate = beginDate.atZone(zoneId).plus(duration);
+
     lock.lock();
     X509Certificate xcert;
     try {
@@ -260,7 +265,7 @@ private X509Certificate signAndStoreCertificate(
           getPrivateKey(),
           getCACertificate(),
           Date.from(beginDate.atZone(ZoneId.systemDefault()).toInstant()),
-          Date.from(endDate.atZone(ZoneId.systemDefault()).toInstant()),
+          Date.from(endDate.toInstant()),
           csr, scmID, clusterID, certSerialId);
       if (store != null) {
         store.checkValidCertID(xcert.getSerialNumber());
@@ -487,14 +492,14 @@ private void generateRootCertificate(
       throws IOException, SCMSecurityException {
     Preconditions.checkNotNull(this.config);
     LocalDateTime beginDate = LocalDateTime.now();
-    LocalDateTime endDate =
-        beginDate.plus(securityConfig.getMaxCertificateDuration());
+    ZoneId zoneId = ZoneId.systemDefault();
+    ZonedDateTime endDate = beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration());
     SelfSignedCertificate.Builder builder = SelfSignedCertificate.newBuilder()
         .setSubject(this.subject)
         .setScmID(this.scmID)
         .setClusterID(this.clusterID)
         .setBeginDate(beginDate)
-        .setEndDate(endDate)
+        .setEndDate(endDate.toLocalDateTime())
         .makeCA(rootCertificateId)
         .setConfiguration(securityConfig)
         .setKey(key);

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/TestDefaultCAServer.java

@@ -46,7 +46,9 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZoneId;
+import java.util.Date;
 import java.util.List;
+import java.util.TimeZone;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Future;
@@ -450,6 +452,61 @@ clusterId, scmId, caStore, new DefaultProfile(),
     }
   }
 
+  @Test
+  public void testDaylightSavingZone() throws Exception {
+    TimeZone defaultTimeZone = TimeZone.getDefault();
+    TimeZone.setDefault(TimeZone.getTimeZone("America/New_York"));
+
+    String scmId = RandomStringUtils.secure().nextAlphabetic(4);
+    String clusterId = RandomStringUtils.secure().nextAlphabetic(4);
+    KeyPair keyPair =
+        new HDDSKeyGenerator(securityConfig).generateKey();
+    //TODO: generateCSR!
+    PKCS10CertificationRequest csr = new CertificateSignRequest.Builder()
+        .addDnsName("hadoop.apache.org")
+        .addIpAddress("8.8.8.8")
+        .addServiceName("OzoneMarketingCluster002")
+        .setCA(false)
+        .setClusterID(clusterId)
+        .setScmID(scmId)
+        .setSubject("Ozone Cluster")
+        .setConfiguration(securityConfig)
+        .setKey(keyPair)
+        .build()
+        .generateCSR();
+
+    CertificateServer testCA = new DefaultCAServer("testCA",
+        clusterId, scmId, caStore,
+        new DefaultProfile(),
+        Paths.get(SCM_CA_CERT_STORAGE_DIR, SCM_CA_PATH).toString());
+    testCA.init(securityConfig, CAType.ROOT);
+
+    Future<CertPath> holder = testCA.requestCertificate(
+        csr, CertificateApprover.ApprovalType.TESTING_AUTOMATIC, SCM,
+        String.valueOf(System.nanoTime()));
+    // Right now our calls are synchronous. Eventually this will have to wait.
+    assertTrue(holder.isDone());
+    //Test that the cert path returned contains the CA certificate in proper
+    // place
+    List<? extends Certificate> certBundle = holder.get().getCertificates();
+
+    // verify new created SCM certificate
+    X509Certificate certificate = (X509Certificate) certBundle.get(0);
+    Date startDate = certificate.getNotBefore();
+    Date endDate = certificate.getNotAfter();
+    assertEquals(securityConfig.getMaxCertificateDuration().toMillis(),
+        endDate.toInstant().toEpochMilli() - startDate.toInstant().toEpochMilli());
+
+    // verify root CA
+    List<? extends Certificate> certificateList = testCA.getCaCertPath().getCertificates();
+    certificate = (X509Certificate) certificateList.get(0);
+    startDate = certificate.getNotBefore();
+    endDate = certificate.getNotAfter();
+    assertEquals(securityConfig.getMaxCertificateDuration().toMillis(),
+        endDate.toInstant().toEpochMilli() - startDate.toInstant().toEpochMilli());
+    TimeZone.setDefault(defaultTimeZone);
+  }
+
   private X509Certificate generateExternalCert(KeyPair keyPair) throws Exception {
     LocalDateTime notBefore = LocalDateTime.now();
     LocalDateTime notAfter = notBefore.plusYears(1);