From 81b7849a8308b3a1da1ac5f247eb295c12ac070b Mon Sep 17 00:00:00 2001 From: Michael Tinker Date: Tue, 19 Nov 2024 15:33:21 -0600 Subject: [PATCH] chore: Do stricter validation of X.509 gossip cert in DAB transactions (#16666) Signed-off-by: Michael Tinker --- .../impl/validators/AddressBookValidator.java | 25 ++++++--- .../impl/test/CertificatePemTest.java | 4 +- .../validators/AddressBookValidatorTest.java | 55 +++++++++++++++++++ .../addressbook/AddressBookHelper.java | 53 ++++++++++++------ 4 files changed, 108 insertions(+), 29 deletions(-) create mode 100644 hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidatorTest.java diff --git a/hedera-node/hedera-addressbook-service-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidator.java b/hedera-node/hedera-addressbook-service-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidator.java index a342485c8c02..284911e4a4ff 100644 --- a/hedera-node/hedera-addressbook-service-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidator.java +++ b/hedera-node/hedera-addressbook-service-impl/src/main/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidator.java @@ -30,6 +30,8 @@ import static com.hedera.hapi.node.base.ResponseCodeEnum.IP_FQDN_CANNOT_BE_SET_FOR_SAME_ENDPOINT; import static com.hedera.hapi.node.base.ResponseCodeEnum.KEY_REQUIRED; import static com.hedera.hapi.node.base.ResponseCodeEnum.SERVICE_ENDPOINTS_EXCEEDED_LIMIT; +import static com.hedera.node.app.service.addressbook.AddressBookHelper.readCertificatePemFile; +import static com.hedera.node.app.service.addressbook.AddressBookHelper.writeCertificatePemFile; import static com.hedera.node.app.spi.key.KeyUtils.isEmpty; import static com.hedera.node.app.spi.key.KeyUtils.isValid; import static com.hedera.node.app.spi.validation.Validations.validateAccountID; @@ -47,10 +49,8 @@ import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; -import java.security.cert.CertificateException; -import java.security.cert.CertificateFactory; -import java.security.cert.X509Certificate; import java.util.List; import javax.inject.Inject; import javax.inject.Singleton; @@ -175,15 +175,22 @@ private void validateEndpoint(@NonNull final ServiceEndpoint endpoint, @NonNull } /** - * Validate the Bytes is a real X509Certificate bytes. - * @param certBytes the Bytes to validate + * Validates the given bytes encode an X509 certificate can be serialized and deserialized from + * PEM format to recover a usable certificate. + * @param x509CertBytes the bytes to validate * @throws PreCheckException if the certificate is invalid */ - public static void validateX509Certificate(@NonNull Bytes certBytes) throws PreCheckException { + public static void validateX509Certificate(@NonNull final Bytes x509CertBytes) throws PreCheckException { try { - final var cert = (X509Certificate) CertificateFactory.getInstance("X.509") - .generateCertificate(new ByteArrayInputStream(certBytes.toByteArray())); - } catch (final CertificateException e) { + // Serialize the given bytes to a PEM file just as we would on a PREPARE_UPGRADE + final var baos = new ByteArrayOutputStream(); + writeCertificatePemFile(x509CertBytes.toByteArray(), baos); + // Deserialize an X509 certificate from the resulting PEM file + final var bais = new ByteArrayInputStream(baos.toByteArray()); + final var cert = readCertificatePemFile(bais); + // And check its validity for completeness + cert.checkValidity(); + } catch (Exception ignore) { throw new PreCheckException(INVALID_GOSSIP_CA_CERTIFICATE); } } diff --git a/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/CertificatePemTest.java b/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/CertificatePemTest.java index 839088c45d53..f82f7988faca 100644 --- a/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/CertificatePemTest.java +++ b/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/test/CertificatePemTest.java @@ -24,7 +24,6 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import com.hedera.hapi.node.base.ResponseCodeEnum; @@ -73,8 +72,7 @@ void invalidCertificatePem() throws CertificateException, IOException { final var cert = readCertificatePemFile(pemFilePath); final var test = Path.of(tmpDir.getPath() + "/test"); Files.write(test, cert.getEncoded()); - final var genCert = readCertificatePemFile(test); - assertNull(genCert); + assertThrows(CertificateException.class, () -> readCertificatePemFile(test)); } @Test diff --git a/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidatorTest.java b/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidatorTest.java new file mode 100644 index 000000000000..13762c7e1859 --- /dev/null +++ b/hedera-node/hedera-addressbook-service-impl/src/test/java/com/hedera/node/app/service/addressbook/impl/validators/AddressBookValidatorTest.java @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * 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 com.hedera.node.app.service.addressbook.impl.validators; + +import static com.hedera.hapi.node.base.ResponseCodeEnum.INVALID_GOSSIP_CA_CERTIFICATE; +import static com.hedera.node.app.service.addressbook.AddressBookHelper.writeCertificatePemFile; +import static com.hedera.node.app.service.addressbook.impl.test.handlers.AddressBookTestBase.generateX509Certificates; +import static com.hedera.node.app.service.addressbook.impl.validators.AddressBookValidator.validateX509Certificate; +import static org.junit.jupiter.api.Assertions.*; + +import com.hedera.node.app.spi.workflows.PreCheckException; +import com.hedera.pbj.runtime.io.buffer.Bytes; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.security.cert.CertificateEncodingException; +import java.security.cert.X509Certificate; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +class AddressBookValidatorTest { + private static X509Certificate x509Cert; + + @BeforeAll + static void beforeAll() { + x509Cert = generateX509Certificates(1).getFirst(); + } + + @Test + void encodedCertPassesValidation() { + assertDoesNotThrow(() -> validateX509Certificate(Bytes.wrap(x509Cert.getEncoded()))); + } + + @Test + void utf8EncodingOfX509PemFailsValidation() throws CertificateEncodingException, IOException { + final var baos = new ByteArrayOutputStream(); + writeCertificatePemFile(x509Cert.getEncoded(), baos); + final var e = + assertThrows(PreCheckException.class, () -> validateX509Certificate(Bytes.wrap(baos.toByteArray()))); + assertEquals(INVALID_GOSSIP_CA_CERTIFICATE, e.responseCode()); + } +} diff --git a/hedera-node/hedera-addressbook-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java b/hedera-node/hedera-addressbook-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java index da8244a4cb8e..7db0dfdf2016 100644 --- a/hedera-node/hedera-addressbook-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java +++ b/hedera-node/hedera-addressbook-service/src/main/java/com/hedera/node/app/service/addressbook/AddressBookHelper.java @@ -27,9 +27,10 @@ import edu.umd.cs.findbugs.annotations.NonNull; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStream; import java.io.OutputStreamWriter; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.security.cert.CertificateException; @@ -71,19 +72,28 @@ public static long getNextNodeID(@NonNull final ReadableNodeStore nodeStore) { /** * Write the Certificate to a pem file. * @param pemFile to write - * @param encodes Certificate encoded byte[] + * @param x509Encoding Certificate encoded byte[] * @throws IOException if an I/O error occurs while writing the file */ - public static void writeCertificatePemFile(@NonNull final Path pemFile, @NonNull final byte[] encodes) + public static void writeCertificatePemFile(@NonNull final Path pemFile, @NonNull final byte[] x509Encoding) throws IOException { - Objects.requireNonNull(pemFile, "pemFile must not be null"); - Objects.requireNonNull(encodes, "cert must not be null"); + writeCertificatePemFile(x509Encoding, new FileOutputStream(pemFile.toFile())); + } - final PemObject pemObj = new PemObject("CERTIFICATE", encodes); - try (final var f = new FileOutputStream(pemFile.toFile()); - final var out = new OutputStreamWriter(f); - final PemWriter writer = new PemWriter(out)) { - writer.writeObject(pemObj); + /** + * Given an X509 encoded certificate, writes it as a PEM to the given output stream. + * + * @param x509Encoding the X509 encoded certificate + * @param out the output stream to write to + * @throws IOException if an I/O error occurs while writing the PEM + */ + public static void writeCertificatePemFile(@NonNull final byte[] x509Encoding, @NonNull final OutputStream out) + throws IOException { + requireNonNull(x509Encoding); + requireNonNull(out); + try (final var writer = new OutputStreamWriter(out); + final PemWriter pemWriter = new PemWriter(writer)) { + pemWriter.writeObject(new PemObject("CERTIFICATE", x509Encoding)); } } @@ -96,22 +106,31 @@ public static void writeCertificatePemFile(@NonNull final Path pemFile, @NonNull */ public static X509Certificate readCertificatePemFile(@NonNull final Path pemFile) throws IOException, CertificateException { - Objects.requireNonNull(pemFile, "pemFile must not be null"); - X509Certificate cert = null; + return readCertificatePemFile(Files.newInputStream(pemFile)); + } + + /** + * Reads a PEM-encoded X509 certificate from the given input stream. + * @param in the input stream to read from + * @return the X509Certificate + * @throws IOException if an I/O error occurs while reading the certificate + * @throws CertificateException if the file does not contain a valid X509Certificate + */ + public static X509Certificate readCertificatePemFile(@NonNull final InputStream in) + throws IOException, CertificateException { + requireNonNull(in); Object entry; - try (final PEMParser parser = - new PEMParser(new InputStreamReader(Files.newInputStream(pemFile), StandardCharsets.UTF_8))) { + try (final var parser = new PEMParser(new InputStreamReader(in))) { while ((entry = parser.readObject()) != null) { if (entry instanceof X509CertificateHolder ch) { - cert = new JcaX509CertificateConverter().getCertificate(ch); - break; + return new JcaX509CertificateConverter().getCertificate(ch); } else { throw new CertificateException( "Not X509 Certificate, it is " + entry.getClass().getSimpleName()); } } } - return cert; + throw new CertificateException("No X509 Certificate found in the PEM file"); } /**