You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by bh...@apache.org on 2021/03/19 10:35:13 UTC
[ozone] branch HDDS-2823 updated: HDDS-4998. [SCM HA Security] Make
storeValidCertificate method idempotent (#2063)
This is an automated email from the ASF dual-hosted git repository.
bharat pushed a commit to branch HDDS-2823
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/HDDS-2823 by this push:
new e2997ef HDDS-4998. [SCM HA Security] Make storeValidCertificate method idempotent (#2063)
e2997ef is described below
commit e2997efc85171f8bb93739816a45eaaea799cfb7
Author: Bharat Viswanadham <bh...@apache.org>
AuthorDate: Fri Mar 19 16:04:50 2021 +0530
HDDS-4998. [SCM HA Security] Make storeValidCertificate method idempotent (#2063)
---
.../certificate/authority/CertificateStore.java | 8 ++++++++
.../certificate/authority/DefaultApprover.java | 11 +++++++++--
.../certificate/authority/DefaultCAServer.java | 23 ++++++++++++++++------
.../x509/certificate/authority/MockCAStore.java | 5 +++++
.../hadoop/hdds/scm/server/SCMCertStore.java | 15 ++++++++------
5 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/CertificateStore.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/CertificateStore.java
index b1de2c6..0a669b9 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/CertificateStore.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/CertificateStore.java
@@ -55,6 +55,14 @@ public interface CertificateStore {
X509Certificate certificate, NodeType role) throws IOException;
/**
+ * Check certificate serialID exists or not. If exists throws an exception.
+ * @param serialID
+ * @throws IOException
+ */
+ void checkValidCertID(BigInteger serialID) throws IOException;
+
+
+ /**
* Adds the certificates to be revoked to a new CRL and moves all the
* certificates in a transactional manner from valid certificate to
* revoked certificate state. Returns an empty {@code Optional} instance if
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultApprover.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultApprover.java
index 7ea28b6..39f610c 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultApprover.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultApprover.java
@@ -89,7 +89,8 @@ public class DefaultApprover extends BaseApprover {
Date validTill,
PKCS10CertificationRequest certificationRequest,
String scmId,
- String clusterId) throws IOException, OperatorCreationException {
+ String clusterId) throws IOException,
+ OperatorCreationException {
AlgorithmIdentifier sigAlgId = new
DefaultSignatureAlgorithmIdentifierFinder().find(
@@ -135,7 +136,7 @@ public class DefaultApprover extends BaseApprover {
new X509v3CertificateBuilder(
caCertificate.getSubject(),
// Serial is not sequential but it is monotonically increasing.
- BigInteger.valueOf(Time.monotonicNowNanos()),
+ BigInteger.valueOf(generateSerialId()),
validFrom,
validTill,
x500Name, keyInfo);
@@ -155,6 +156,12 @@ public class DefaultApprover extends BaseApprover {
}
+ public long generateSerialId() {
+ // TODO: to make generation of serialId distributed.
+ // This issue will be fixed in HDDS-4999.
+ return Time.monotonicNowNanos();
+ }
+
@Override
public CompletableFuture<X509CertificateHolder> inspectCSR(String csr)
throws IOException {
diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
index 2cd8993..dc70e4f 100644
--- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
+++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java
@@ -60,6 +60,8 @@ import java.util.List;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.Future;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer;
import static org.apache.hadoop.hdds.security.x509.certificates.utils.CertificateSignRequest.getCertificationRequest;
@@ -129,6 +131,7 @@ public class DefaultCAServer implements CertificateServer {
private CertificateApprover approver;
private CRLApprover crlApprover;
private CertificateStore store;
+ private Lock lock;
/**
* Create an Instance of DefaultCAServer.
@@ -143,6 +146,7 @@ public class DefaultCAServer implements CertificateServer {
this.clusterID = clusterID;
this.scmID = scmID;
this.store = certificateStore;
+ lock = new ReentrantLock();
}
@Override
@@ -265,12 +269,19 @@ public class DefaultCAServer implements CertificateServer {
LocalDate endDate, PKCS10CertificationRequest csr, NodeType role)
throws IOException,
OperatorCreationException, CertificateException {
- X509CertificateHolder xcert = approver.sign(config,
- getCAKeys().getPrivate(),
- getCACertificate(), java.sql.Date.valueOf(beginDate),
- java.sql.Date.valueOf(endDate), csr, scmID, clusterID);
- store.storeValidCertificate(xcert.getSerialNumber(),
- CertificateCodec.getX509Certificate(xcert), role);
+ lock.lock();
+ X509CertificateHolder xcert;
+ try {
+ xcert = approver.sign(config,
+ getCAKeys().getPrivate(),
+ getCACertificate(), java.sql.Date.valueOf(beginDate),
+ java.sql.Date.valueOf(endDate), csr, scmID, clusterID);
+ store.checkValidCertID(xcert.getSerialNumber());
+ store.storeValidCertificate(xcert.getSerialNumber(),
+ CertificateCodec.getX509Certificate(xcert), role);
+ } finally {
+ lock.unlock();
+ }
return xcert;
}
diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/MockCAStore.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/MockCAStore.java
index 366b285..c60c975 100644
--- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/MockCAStore.java
+++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/security/x509/certificate/authority/MockCAStore.java
@@ -43,6 +43,11 @@ public class MockCAStore implements CertificateStore {
}
@Override
+ public void checkValidCertID(BigInteger serialID) throws IOException {
+
+ }
+
+ @Override
public Optional<Long> revokeCertificates(
List<BigInteger> serialIDs,
X509CertificateHolder caCertificateHolder,
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java
index afbfc07..ee4bc20 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMCertStore.java
@@ -80,7 +80,6 @@ public final class SCMCertStore implements CertificateStore {
lock.lock();
try {
// This makes sure that no certificate IDs are reusable.
- checkValidCertID(serialID);
if (role == SCM) {
// If the role is SCM, store certificate in scm cert table
// and valid cert table. This is to help to return scm certs during
@@ -106,7 +105,6 @@ public final class SCMCertStore implements CertificateStore {
X509Certificate certificate) throws IOException {
lock.lock();
try {
- checkValidCertID(serialID);
BatchOperation batchOperation =
scmMetadataStore.getBatchHandler().initBatchOperation();
scmMetadataStore.getValidSCMCertsTable().putWithBatch(batchOperation,
@@ -119,10 +117,15 @@ public final class SCMCertStore implements CertificateStore {
}
}
- private void checkValidCertID(BigInteger serialID) throws IOException {
- if ((getCertificateByID(serialID, VALID_CERTS) != null) ||
- (getCertificateByID(serialID, CertType.REVOKED_CERTS) != null)) {
- throw new SCMSecurityException("Conflicting certificate ID");
+ public void checkValidCertID(BigInteger serialID) throws IOException {
+ lock.lock();
+ try {
+ if ((getCertificateByID(serialID, VALID_CERTS) != null) ||
+ (getCertificateByID(serialID, CertType.REVOKED_CERTS) != null)) {
+ throw new SCMSecurityException("Conflicting certificate ID" + serialID);
+ }
+ } finally {
+ lock.unlock();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org