Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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;
Expand Down Expand Up @@ -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<>();
Expand All @@ -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);
Expand All @@ -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());
Expand Down Expand Up @@ -487,14 +491,14 @@ private void generateRootCertificate(
throws IOException, SCMSecurityException {
Preconditions.checkNotNull(this.config);
LocalDateTime beginDate = LocalDateTime.now();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LocalDateTime beginDate = LocalDateTime.now();
ZonedDateTime beginDate = ZonedDateTime.now();

LocalDateTime endDate =
beginDate.plus(securityConfig.getMaxCertificateDuration());
ZoneId zoneId = ZoneId.systemDefault();
ZonedDateTime endDate = beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration());
Copy link
Contributor

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());

Copy link
Contributor

@adoroszlai adoroszlai Nov 10, 2025

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.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ZoneId zoneId = ZoneId.systemDefault();
ZonedDateTime endDate = beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration());
ZonedDateTime endDate = beginDate.plus(securityConfig.getMaxCertificateDuration());

SelfSignedCertificate.Builder builder = SelfSignedCertificate.newBuilder()
.setSubject(this.subject)
.setScmID(this.scmID)
.setClusterID(this.clusterID)
.setBeginDate(beginDate)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.setBeginDate(beginDate)
.setBeginDate(beginDate.toLocalDateTime())

.setEndDate(endDate)
.setEndDate(endDate.toLocalDateTime())
.makeCA(rootCertificateId)
.setConfiguration(securityConfig)
.setKey(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down