You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ChenSammi (via GitHub)" <gi...@apache.org> on 2023/05/30 03:05:07 UTC

[GitHub] [ozone] ChenSammi opened a new pull request, #4792: HDDS-8694. Add SCM HA awared Root CA certificate monitor task

ChenSammi opened a new pull request, #4792:
URL: https://github.com/apache/ozone/pull/4792

   https://issues.apache.org/jira/browse/HDDS-8694


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1213871529


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(

Review Comment:
   Good point.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1213871436


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL,
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT);
+    checkInterval = Duration.parse(durationString);
+
+    String timeOfDayString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY,
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT);
+    Matcher matcher = timePattern.matcher(timeOfDayString);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_TIME_OF_DAY +
+          " should follow the hh:mm:ss format.");
+    }
+    timeOfDayString = "1970-01-01T" + timeOfDayString;
+    timeOfDay = Date.from(LocalDateTime.parse(timeOfDayString)
+        .atZone(ZoneId.systemDefault()).toInstant());
+
+    String renewalGraceDurationString = ozoneConf.get(
+        HDDS_X509_RENEW_GRACE_DURATION,
+        HDDS_X509_RENEW_GRACE_DURATION_DEFAULT);
+    renewalGracePeriod = Duration.parse(renewalGraceDurationString);
+
+    if (checkInterval.compareTo(renewalGracePeriod) >= 0) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+    }
+
+    executorService = Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat(threadName)
+            .setDaemon(true).build());
+
+    scmCertClient = scm.getScmCertificateClient();
+    scm.getSCMServiceManager().register(this);
+  }
+
+  /**
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops monitor task if it's running and the current SCM becomes a
+   * follower or enter safe mode. Starts monitor if the current SCM becomes
+   * leader and not in safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    // stop the monitor task
+    if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+      if (isRunning.compareAndSet(true, false)) {
+        LOG.info("notifyStatusChanged: disable monitor task.");
+      }
+      return;
+    }
+
+    if (isRunning.compareAndSet(false, true)) {
+      LOG.info("notifyStatusChanged: enable monitor task");
+    }
+    return;
+  }
+
+  /**
+   * Checks if monitor task should start (after a leader change, restart
+   * etc.) by reading persisted state.
+   * @return true if the persisted state is true, otherwise false
+   */
+  @Override
+  public boolean shouldRun() {
+    return true;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return RootCARotationManager.class.getSimpleName();
+  }
+
+  /**
+   * Schedule monitor task.
+   */
+  @Override
+  public void start() throws SCMServiceException {
+    executorService.scheduleAtFixedRate(
+        new MonitorTask(scmCertClient), 0, checkInterval.toMillis(),
+        TimeUnit.MILLISECONDS);
+    LOG.info("Monitor task for root certificate {} is started with " +
+        "interval {}.", scmCertClient.getCACertificate().getSerialNumber(),
+        checkInterval);
+  }
+
+  public boolean isRunning() {
+    return isRunning.get();
+  }
+
+  /**
+   *  Task to monitor certificate lifetime and start rotation if needed.
+   */
+  public class MonitorTask implements Runnable {
+    private CertificateClient certClient;
+
+    public MonitorTask(CertificateClient client) {
+      this.certClient = client;
+    }
+
+    @Override
+    public void run() {
+      Thread.currentThread().setName(threadName +
+          (isRunning() ? "-Active" : "-Inactive"));
+      if (!isRunning.get() || isScheduled.get()) {
+        return;
+      }
+      // Lock to protect the root CA certificate rotation process,
+      // to make sure there is only one task is ongoing at one time.
+      synchronized (RootCARotationManager.class) {
+        X509Certificate rootCACert = certClient.getCACertificate();

Review Comment:
   For ScmCertificateClient, getCACertificate returns root CA cert, getRootCACertificate returns null. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi merged pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi merged PR #4792:
URL: https://github.com/apache/ozone/pull/4792


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] Galsza commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1216717113


##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -2234,6 +2234,24 @@
       they can be configured separately.
     </description>
   </property>
+  <property>
+    <name>hdds.x509.ca.rotation.check.interval</name>
+    <value>P1D</value>
+    <tag>OZONE, HDDS, SECURITY</tag>
+    <description>Check interval of whether internal root certificate is going
+      to expire and needs to start rotation or not. Default is 1 day. The property
+      value should be less than the value of property hdds.x509.renew.grace.duration.
+    </description>
+  </property>
+  <property>
+    <name>hdds.x509.ca.rotation.time-of-day</name>
+    <value>02:00:00</value>
+    <tag>OZONE, HDDS, SECURITY</tag>
+    <description>Time of day to start the rotation. Default 02:00 AM to avoid impact

Review Comment:
   ```suggestion
       <description>Time of day to start the rotation. Default 02:00 AM to avoid impacting
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] Galsza commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "Galsza (via GitHub)" <gi...@apache.org>.
Galsza commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1216746662


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL,
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT);
+    checkInterval = Duration.parse(durationString);
+
+    String timeOfDayString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY,
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT);
+    Matcher matcher = timePattern.matcher(timeOfDayString);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_TIME_OF_DAY +
+          " should follow the hh:mm:ss format.");
+    }
+    timeOfDayString = "1970-01-01T" + timeOfDayString;
+    timeOfDay = Date.from(LocalDateTime.parse(timeOfDayString)
+        .atZone(ZoneId.systemDefault()).toInstant());
+
+    String renewalGraceDurationString = ozoneConf.get(
+        HDDS_X509_RENEW_GRACE_DURATION,
+        HDDS_X509_RENEW_GRACE_DURATION_DEFAULT);
+    renewalGracePeriod = Duration.parse(renewalGraceDurationString);
+
+    if (checkInterval.compareTo(renewalGracePeriod) >= 0) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+    }
+
+    executorService = Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat(threadName)
+            .setDaemon(true).build());
+
+    scmCertClient = scm.getScmCertificateClient();
+    scm.getSCMServiceManager().register(this);
+  }
+
+  /**
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops monitor task if it's running and the current SCM becomes a
+   * follower or enter safe mode. Starts monitor if the current SCM becomes
+   * leader and not in safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    // stop the monitor task
+    if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+      if (isRunning.compareAndSet(true, false)) {
+        LOG.info("notifyStatusChanged: disable monitor task.");
+      }
+      return;
+    }
+
+    if (isRunning.compareAndSet(false, true)) {
+      LOG.info("notifyStatusChanged: enable monitor task");
+    }
+    return;
+  }
+
+  /**
+   * Checks if monitor task should start (after a leader change, restart
+   * etc.) by reading persisted state.
+   * @return true if the persisted state is true, otherwise false
+   */
+  @Override
+  public boolean shouldRun() {
+    return true;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return RootCARotationManager.class.getSimpleName();
+  }
+
+  /**
+   * Schedule monitor task.
+   */
+  @Override
+  public void start() throws SCMServiceException {
+    executorService.scheduleAtFixedRate(
+        new MonitorTask(scmCertClient), 0, checkInterval.toMillis(),
+        TimeUnit.MILLISECONDS);
+    LOG.info("Monitor task for root certificate {} is started with " +
+        "interval {}.", scmCertClient.getCACertificate().getSerialNumber(),
+        checkInterval);
+  }
+
+  public boolean isRunning() {
+    return isRunning.get();
+  }
+
+  /**
+   *  Task to monitor certificate lifetime and start rotation if needed.
+   */
+  public class MonitorTask implements Runnable {
+    private CertificateClient certClient;
+
+    public MonitorTask(CertificateClient client) {
+      this.certClient = client;
+    }
+
+    @Override
+    public void run() {
+      Thread.currentThread().setName(threadName +
+          (isRunning() ? "-Active" : "-Inactive"));
+      if (!isRunning.get() || isScheduled.get()) {
+        return;
+      }
+      // Lock to protect the root CA certificate rotation process,
+      // to make sure there is only one task is ongoing at one time.
+      synchronized (RootCARotationManager.class) {
+        X509Certificate rootCACert = certClient.getCACertificate();

Review Comment:
   I believe we should change this functionality some time. Let's discuss it a bit during our sync meeting. For now it's okay here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #4792:
URL: https://github.com/apache/ozone/pull/4792#issuecomment-1579903628

   Thanks @fapifta and @Galsza for the code review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ChenSammi commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1213872253


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL,
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT);
+    checkInterval = Duration.parse(durationString);
+
+    String timeOfDayString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY,
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT);
+    Matcher matcher = timePattern.matcher(timeOfDayString);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_TIME_OF_DAY +
+          " should follow the hh:mm:ss format.");
+    }
+    timeOfDayString = "1970-01-01T" + timeOfDayString;
+    timeOfDay = Date.from(LocalDateTime.parse(timeOfDayString)
+        .atZone(ZoneId.systemDefault()).toInstant());
+
+    String renewalGraceDurationString = ozoneConf.get(
+        HDDS_X509_RENEW_GRACE_DURATION,
+        HDDS_X509_RENEW_GRACE_DURATION_DEFAULT);
+    renewalGracePeriod = Duration.parse(renewalGraceDurationString);
+
+    if (checkInterval.compareTo(renewalGracePeriod) >= 0) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+    }
+
+    executorService = Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat(threadName)
+            .setDaemon(true).build());
+
+    scmCertClient = scm.getScmCertificateClient();
+    scm.getSCMServiceManager().register(this);
+  }
+
+  /**
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops monitor task if it's running and the current SCM becomes a
+   * follower or enter safe mode. Starts monitor if the current SCM becomes
+   * leader and not in safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    // stop the monitor task
+    if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+      if (isRunning.compareAndSet(true, false)) {
+        LOG.info("notifyStatusChanged: disable monitor task.");
+      }
+      return;
+    }
+
+    if (isRunning.compareAndSet(false, true)) {
+      LOG.info("notifyStatusChanged: enable monitor task");
+    }
+    return;
+  }
+
+  /**
+   * Checks if monitor task should start (after a leader change, restart
+   * etc.) by reading persisted state.
+   * @return true if the persisted state is true, otherwise false
+   */
+  @Override
+  public boolean shouldRun() {
+    return true;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return RootCARotationManager.class.getSimpleName();
+  }
+
+  /**
+   * Schedule monitor task.
+   */
+  @Override
+  public void start() throws SCMServiceException {
+    executorService.scheduleAtFixedRate(
+        new MonitorTask(scmCertClient), 0, checkInterval.toMillis(),
+        TimeUnit.MILLISECONDS);
+    LOG.info("Monitor task for root certificate {} is started with " +
+        "interval {}.", scmCertClient.getCACertificate().getSerialNumber(),
+        checkInterval);
+  }
+
+  public boolean isRunning() {
+    return isRunning.get();
+  }
+
+  /**
+   *  Task to monitor certificate lifetime and start rotation if needed.
+   */
+  public class MonitorTask implements Runnable {
+    private CertificateClient certClient;
+
+    public MonitorTask(CertificateClient client) {
+      this.certClient = client;
+    }
+
+    @Override
+    public void run() {
+      Thread.currentThread().setName(threadName +
+          (isRunning() ? "-Active" : "-Inactive"));
+      if (!isRunning.get() || isScheduled.get()) {
+        return;
+      }
+      // Lock to protect the root CA certificate rotation process,
+      // to make sure there is only one task is ongoing at one time.
+      synchronized (RootCARotationManager.class) {
+        X509Certificate rootCACert = certClient.getCACertificate();
+        Duration timeLeft = timeBefore2ExpiryGracePeriod(rootCACert);
+        if (timeLeft.isZero()) {
+          LOG.info("Root certificate {} has entered the 2 * expiry" +
+                  " grace period({}).", rootCACert.getSerialNumber().toString(),
+              renewalGracePeriod);
+          // schedule root CA rotation task
+          LocalDateTime now = LocalDateTime.now();
+          LocalDateTime timeToSchedule = LocalDateTime.of(
+              now.getYear(), now.getMonthValue(), now.getDayOfMonth(),
+              timeOfDay.getHours(), timeOfDay.getMinutes(),
+              timeOfDay.getSeconds());
+          if (timeToSchedule.isBefore(now)) {
+            timeToSchedule.plusDays(1);
+          }
+          long delay = Duration.between(now, timeToSchedule).toMillis();
+          if (timeToSchedule.isAfter(rootCACert.getNotAfter().toInstant()
+              .atZone(ZoneId.systemDefault()).toLocalDateTime())) {
+            LOG.warn("Configured rotation time {} is after root" +

Review Comment:
   This is to indicate that rotation process cannot be started on the scheduled timestamp, in case of a short rootCA lifetime. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] fapifta commented on a diff in pull request #4792: HDDS-8694. Add SCM HA aware Root CA certificate monitor task

Posted by "fapifta (via GitHub)" <gi...@apache.org>.
fapifta commented on code in PR #4792:
URL: https://github.com/apache/ozone/pull/4792#discussion_r1213025520


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(

Review Comment:
   Instead of relying on OzoneConfiguration, wouldn't it be better to add these config processing to the SecurityConfig object, and get the values from there with the properly typed values?
   I think these we might need in other services as well in order to determine the time when other services should start to expect a new rootCA certificate, so that seems to be a natural common place for getting these values.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL,
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT);
+    checkInterval = Duration.parse(durationString);
+
+    String timeOfDayString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY,
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT);
+    Matcher matcher = timePattern.matcher(timeOfDayString);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_TIME_OF_DAY +
+          " should follow the hh:mm:ss format.");
+    }
+    timeOfDayString = "1970-01-01T" + timeOfDayString;
+    timeOfDay = Date.from(LocalDateTime.parse(timeOfDayString)
+        .atZone(ZoneId.systemDefault()).toInstant());
+
+    String renewalGraceDurationString = ozoneConf.get(
+        HDDS_X509_RENEW_GRACE_DURATION,
+        HDDS_X509_RENEW_GRACE_DURATION_DEFAULT);
+    renewalGracePeriod = Duration.parse(renewalGraceDurationString);
+
+    if (checkInterval.compareTo(renewalGracePeriod) >= 0) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+    }
+
+    executorService = Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat(threadName)
+            .setDaemon(true).build());
+
+    scmCertClient = scm.getScmCertificateClient();
+    scm.getSCMServiceManager().register(this);
+  }
+
+  /**
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops monitor task if it's running and the current SCM becomes a
+   * follower or enter safe mode. Starts monitor if the current SCM becomes
+   * leader and not in safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    // stop the monitor task
+    if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+      if (isRunning.compareAndSet(true, false)) {
+        LOG.info("notifyStatusChanged: disable monitor task.");
+      }
+      return;
+    }
+
+    if (isRunning.compareAndSet(false, true)) {
+      LOG.info("notifyStatusChanged: enable monitor task");
+    }
+    return;
+  }
+
+  /**
+   * Checks if monitor task should start (after a leader change, restart
+   * etc.) by reading persisted state.
+   * @return true if the persisted state is true, otherwise false
+   */
+  @Override
+  public boolean shouldRun() {
+    return true;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return RootCARotationManager.class.getSimpleName();
+  }
+
+  /**
+   * Schedule monitor task.
+   */
+  @Override
+  public void start() throws SCMServiceException {
+    executorService.scheduleAtFixedRate(
+        new MonitorTask(scmCertClient), 0, checkInterval.toMillis(),
+        TimeUnit.MILLISECONDS);
+    LOG.info("Monitor task for root certificate {} is started with " +
+        "interval {}.", scmCertClient.getCACertificate().getSerialNumber(),
+        checkInterval);
+  }
+
+  public boolean isRunning() {
+    return isRunning.get();
+  }
+
+  /**
+   *  Task to monitor certificate lifetime and start rotation if needed.
+   */
+  public class MonitorTask implements Runnable {
+    private CertificateClient certClient;
+
+    public MonitorTask(CertificateClient client) {
+      this.certClient = client;
+    }
+
+    @Override
+    public void run() {
+      Thread.currentThread().setName(threadName +
+          (isRunning() ? "-Active" : "-Inactive"));
+      if (!isRunning.get() || isScheduled.get()) {
+        return;
+      }
+      // Lock to protect the root CA certificate rotation process,
+      // to make sure there is only one task is ongoing at one time.
+      synchronized (RootCARotationManager.class) {
+        X509Certificate rootCACert = certClient.getCACertificate();

Review Comment:
   On the certClient we have a getRootCACertificate method, I believe we should use that one here.



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/RootCARotationManager.java:
##########
@@ -0,0 +1,303 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.security;
+
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ha.SCMContext;
+import org.apache.hadoop.hdds.scm.ha.SCMService;
+import org.apache.hadoop.hdds.scm.ha.SCMServiceException;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
+import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION_DEFAULT;
+
+/**
+ * Root CA Rotation Manager is a service in SCM to control the CA rotation.
+ */
+public class RootCARotationManager implements SCMService {
+
+  public static final Logger LOG =
+      LoggerFactory.getLogger(RootCARotationManager.class);
+
+  private StorageContainerManager scm;
+  private final SCMContext scmContext;
+  private OzoneConfiguration ozoneConf;
+  private ScheduledExecutorService executorService;
+  private Duration checkInterval;
+  private Duration renewalGracePeriod;
+  private Pattern timePattern = Pattern.compile("\\d{2}:\\d{2}:\\d{2}");
+  private Date timeOfDay;
+  private CertificateClient scmCertClient;
+  private AtomicBoolean isRunning = new AtomicBoolean(false);
+  private AtomicBoolean isScheduled = new AtomicBoolean(false);
+  private String threadName = this.getClass().getSimpleName() + "-MonitorTask";
+
+  /**
+   * Constructs RootCARotationManager with the specified arguments.
+   *
+   * @param scm the storage container manager
+   */
+  public RootCARotationManager(StorageContainerManager scm) {
+    this.scm = scm;
+    this.ozoneConf = scm.getConfiguration();
+
+    this.scmContext = scm.getScmContext();
+    String durationString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL,
+        HDDS_X509_CA_ROTATION_CHECK_INTERNAL_DEFAULT);
+    checkInterval = Duration.parse(durationString);
+
+    String timeOfDayString = ozoneConf.get(
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY,
+        HDDS_X509_CA_ROTATION_TIME_OF_DAY_DEFAULT);
+    Matcher matcher = timePattern.matcher(timeOfDayString);
+    if (!matcher.matches()) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_TIME_OF_DAY +
+          " should follow the hh:mm:ss format.");
+    }
+    timeOfDayString = "1970-01-01T" + timeOfDayString;
+    timeOfDay = Date.from(LocalDateTime.parse(timeOfDayString)
+        .atZone(ZoneId.systemDefault()).toInstant());
+
+    String renewalGraceDurationString = ozoneConf.get(
+        HDDS_X509_RENEW_GRACE_DURATION,
+        HDDS_X509_RENEW_GRACE_DURATION_DEFAULT);
+    renewalGracePeriod = Duration.parse(renewalGraceDurationString);
+
+    if (checkInterval.compareTo(renewalGracePeriod) >= 0) {
+      throw new IllegalArgumentException("Property value of " +
+          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+    }
+
+    executorService = Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat(threadName)
+            .setDaemon(true).build());
+
+    scmCertClient = scm.getScmCertificateClient();
+    scm.getSCMServiceManager().register(this);
+  }
+
+  /**
+   * Receives a notification for raft or safe mode related status changes.
+   * Stops monitor task if it's running and the current SCM becomes a
+   * follower or enter safe mode. Starts monitor if the current SCM becomes
+   * leader and not in safe mode.
+   */
+  @Override
+  public void notifyStatusChanged() {
+    // stop the monitor task
+    if (!scmContext.isLeader() || scmContext.isInSafeMode()) {
+      if (isRunning.compareAndSet(true, false)) {
+        LOG.info("notifyStatusChanged: disable monitor task.");
+      }
+      return;
+    }
+
+    if (isRunning.compareAndSet(false, true)) {
+      LOG.info("notifyStatusChanged: enable monitor task");
+    }
+    return;
+  }
+
+  /**
+   * Checks if monitor task should start (after a leader change, restart
+   * etc.) by reading persisted state.
+   * @return true if the persisted state is true, otherwise false
+   */
+  @Override
+  public boolean shouldRun() {
+    return true;
+  }
+
+  /**
+   * @return Name of this service.
+   */
+  @Override
+  public String getServiceName() {
+    return RootCARotationManager.class.getSimpleName();
+  }
+
+  /**
+   * Schedule monitor task.
+   */
+  @Override
+  public void start() throws SCMServiceException {
+    executorService.scheduleAtFixedRate(
+        new MonitorTask(scmCertClient), 0, checkInterval.toMillis(),
+        TimeUnit.MILLISECONDS);
+    LOG.info("Monitor task for root certificate {} is started with " +
+        "interval {}.", scmCertClient.getCACertificate().getSerialNumber(),
+        checkInterval);
+  }
+
+  public boolean isRunning() {
+    return isRunning.get();
+  }
+
+  /**
+   *  Task to monitor certificate lifetime and start rotation if needed.
+   */
+  public class MonitorTask implements Runnable {
+    private CertificateClient certClient;
+
+    public MonitorTask(CertificateClient client) {
+      this.certClient = client;
+    }
+
+    @Override
+    public void run() {
+      Thread.currentThread().setName(threadName +
+          (isRunning() ? "-Active" : "-Inactive"));
+      if (!isRunning.get() || isScheduled.get()) {
+        return;
+      }
+      // Lock to protect the root CA certificate rotation process,
+      // to make sure there is only one task is ongoing at one time.
+      synchronized (RootCARotationManager.class) {
+        X509Certificate rootCACert = certClient.getCACertificate();
+        Duration timeLeft = timeBefore2ExpiryGracePeriod(rootCACert);
+        if (timeLeft.isZero()) {
+          LOG.info("Root certificate {} has entered the 2 * expiry" +
+                  " grace period({}).", rootCACert.getSerialNumber().toString(),
+              renewalGracePeriod);
+          // schedule root CA rotation task
+          LocalDateTime now = LocalDateTime.now();
+          LocalDateTime timeToSchedule = LocalDateTime.of(
+              now.getYear(), now.getMonthValue(), now.getDayOfMonth(),
+              timeOfDay.getHours(), timeOfDay.getMinutes(),
+              timeOfDay.getSeconds());
+          if (timeToSchedule.isBefore(now)) {
+            timeToSchedule.plusDays(1);
+          }
+          long delay = Duration.between(now, timeToSchedule).toMillis();
+          if (timeToSchedule.isAfter(rootCACert.getNotAfter().toInstant()
+              .atZone(ZoneId.systemDefault()).toLocalDateTime())) {
+            LOG.warn("Configured rotation time {} is after root" +

Review Comment:
   I am not sure about the warn level, is there a case where getting into this state indicates a real problem in the system other than short rootCA lifetime?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org