Skip to content

Commit

Permalink
fix: allow absent agreement keys on disk and in state (#15340) (#15358)
Browse files Browse the repository at this point in the history 
Signed-off-by: Edward Wertz <[email protected]>
  • Loading branch information
@edward-swirldslabs
edward-swirldslabs authored Sep 5, 2024
1 parent 20a3e24 commit 124eaf4
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 32 deletions.
11 changes: 2 additions & 9 deletions ...orm-sdk/swirlds-platform-core/src/main/java/com/swirlds/platform/crypto/CryptoStatic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,15 +455,8 @@ static void copyPublicKeys(final PublicStores publicStores, final AddressBook ad
final Address add = addressBook.getAddress(nodeId);
final String name = nameToAlias(add.getSelfName());
final X509Certificate sigCert = publicStores.getCertificate(SIGNING, name);
try {
final X509Certificate agrCert = publicStores.getCertificate(KeyCertPurpose.AGREEMENT, name);
addressBook.add(
addressBook.getAddress(nodeId).copySetSigCert(sigCert).copySetAgreeCert(agrCert));
} catch (final KeyLoadingException e) {
// the agreement key is allowed to be absent and is never used from the address book anymore.
addressBook.add(
addressBook.getAddress(nodeId).copySetSigCert(sigCert).copySetAgreeCert(null));
}
// the agreement key is never used from the address book anymore and is left null.
addressBook.add(addressBook.getAddress(nodeId).copySetSigCert(sigCert));
}
}

Expand Down
51 changes: 33 additions & 18 deletions ...irlds-platform-core/src/main/java/com/swirlds/platform/crypto/EnhancedKeyStoreLoader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,12 @@ public EnhancedKeyStoreLoader generateIfNecessary()

for (final NodeId node : localNodes) {
if (!agrPrivateKeys.containsKey(node)) {
final String alias = nameToAlias(addressBook.getAddress(node).getSelfName());
logger.info(
STARTUP.getMarker(),
"Generating agreement key pair for local node {} [ alias = {} ]",
node,
alias);
// Generate a new agreement key since it does not exist
final KeyPair agrKeyPair = KeysAndCerts.generateAgreementKeyPair();
agrPrivateKeys.put(node, agrKeyPair.getPrivate());
Expand All @@ -330,8 +336,7 @@ public EnhancedKeyStoreLoader generateIfNecessary()
final KeyPair signingKeyPair = new KeyPair(publicSigningKey, privateSigningKey);

// generate the agreement certificate
final String dnA = CryptoStatic.distinguishedName(KeyCertPurpose.AGREEMENT.storeName(
nameToAlias(addressBook.getAddress(node).getSelfName())));
final String dnA = CryptoStatic.distinguishedName(KeyCertPurpose.AGREEMENT.storeName(alias));
final X509Certificate agrCert = CryptoStatic.generateCertificate(
dnA,
agrKeyPair,
Expand Down Expand Up @@ -391,17 +396,18 @@ public EnhancedKeyStoreLoader verify(@NonNull final AddressBook validatingBook)
throw new KeyLoadingException("No private key found for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.AGREEMENT));
}

// the agreement certificate must be present for local nodes
if (!agrCertificates.containsKey(nodeId)) {
throw new KeyLoadingException("No certificate found for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.AGREEMENT));
}
}

if (!sigCertificates.containsKey(nodeId)) {
throw new KeyLoadingException("No certificate found for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.SIGNING));
}

if (!agrCertificates.containsKey(nodeId)) {
throw new KeyLoadingException("No certificate found for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.AGREEMENT));
}
});

return this;
Expand Down Expand Up @@ -441,19 +447,14 @@ public Map<NodeId, KeysAndCerts> keysAndCerts(@NonNull final AddressBook validat

iterateAddressBook(validatingBook, (i, nodeId, address, nodeAlias) -> {
final X509Certificate sigCert = publicStores.getCertificate(KeyCertPurpose.SIGNING, nodeAlias);
final X509Certificate agrCert = publicStores.getCertificate(KeyCertPurpose.AGREEMENT, nodeAlias);

if (sigCert == null) {
throw new KeyLoadingException(
"No signing certificate found for node %s [ alias = %s ]".formatted(nodeId, nodeAlias));
}

if (agrCert == null) {
throw new KeyLoadingException(
"No agreement certificate found for node %s [ alias = %s ]".formatted(nodeId, nodeAlias));
}

if (localNodes.contains(nodeId)) {
final X509Certificate agrCert = publicStores.getCertificate(KeyCertPurpose.AGREEMENT, nodeAlias);
final PrivateKey sigPrivateKey = sigPrivateKeys.get(nodeId);
final PrivateKey agrPrivateKey = agrPrivateKeys.get(nodeId);

Expand All @@ -467,6 +468,12 @@ public Map<NodeId, KeysAndCerts> keysAndCerts(@NonNull final AddressBook validat
"No agreement private key found for node %s [ alias = %s ]".formatted(nodeId, nodeAlias));
}

// the agreement certificate must be present for local nodes
if (agrCert == null) {
throw new KeyLoadingException(
"No agreement certificate found for node %s [ alias = %s ]".formatted(nodeId, nodeAlias));
}

final KeyPair sigKeyPair = new KeyPair(sigCert.getPublicKey(), sigPrivateKey);
final KeyPair agrKeyPair = new KeyPair(agrCert.getPublicKey(), agrPrivateKey);

Expand Down Expand Up @@ -550,14 +557,22 @@ public PublicStores publicStores(@NonNull final AddressBook validatingBook)
.formatted(nodeId, nodeAlias, KeyCertPurpose.SIGNING));
}

if (!(agrCert instanceof X509Certificate)) {
throw new KeyLoadingException(
"Illegal agreement certificate type for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.AGREEMENT));
if (isLocal(address)) {
// The agreement certificate is loaded by the local nodes and provided to peers through mTLS handshaking
logger.trace(
STARTUP.getMarker(),
"Injecting agreement certificate for local node {} [ alias = {} ] into public stores",
nodeId,
nodeAlias);
if (!(agrCert instanceof X509Certificate)) {
throw new KeyLoadingException(
"Illegal agreement certificate type for node %s [ alias = %s, purpose = %s ]"
.formatted(nodeId, nodeAlias, KeyCertPurpose.AGREEMENT));
}
publicStores.setCertificate(KeyCertPurpose.AGREEMENT, (X509Certificate) agrCert, nodeAlias);
}

publicStores.setCertificate(KeyCertPurpose.SIGNING, (X509Certificate) sigCert, nodeAlias);
publicStores.setCertificate(KeyCertPurpose.AGREEMENT, (X509Certificate) agrCert, nodeAlias);
});

return publicStores;
Expand Down
9 changes: 6 additions & 3 deletions .../swirlds-platform-core/src/main/java/com/swirlds/platform/state/service/PbjConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,12 @@ private static com.hedera.hapi.platform.state.MinimumJudgeInfo toPbjMinimumJudge
return new com.hedera.hapi.platform.state.MinimumJudgeInfo(v.round(), v.minimumJudgeAncientThreshold());
}

@NonNull
private static SerializableX509Certificate fromPbjX509Certificate(@NonNull final Bytes bytes) {
requireNonNull(bytes);
@Nullable
private static SerializableX509Certificate fromPbjX509Certificate(@Nullable final Bytes bytes) {
if (bytes == null || bytes.length() == 0) {
// as of release 0.55.0, future address books in state will not have the agreement key serialized.
return null;
}
final byte[] encoded = bytes.toByteArray();
try {
return new SerializableX509Certificate((X509Certificate)
Expand Down
2 changes: 1 addition & 1 deletion ...s-platform-core/src/test/java/com/swirlds/platform/crypto/EnhancedKeyStoreLoaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ void keyStoreLoaderPositiveTest(final String directoryName)
assertThat(keysAndCerts.sigKeyPair()).isNotNull();
}

assertThat(addr.getAgreePublicKey()).isNotNull();
assertThat(addr.getSigPublicKey()).isNotNull();
assertThat(addr.getAgreePublicKey()).isNull();
}
}

Expand Down
2 changes: 1 addition & 1 deletion ...orm-core/src/test/resources/com/swirlds/platform/crypto/EnhancedKeyStoreLoader/config.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ swirld, 123

address, 0, A, Alice, 1, 127.0.0.1, 15301, 127.0.0.1, 15301
address, 1, B, Bob, 1, 127.0.0.1, 15302, 127.0.0.1, 15302
address, 2, C, Carol, 1, 127.0.0.1, 15303, 127.0.0.1, 15303
address, 2, C, Carol, 1, 8.8.8.8, 15303, 127.0.0.1, 15303

nextNodeId, 3

0 comments on commit 124eaf4

Please sign in to comment.