chore: Do stricter validation of X.509 gossip cert in DAB transactions (

#16666)

Signed-off-by: Michael Tinker <michael.tinker@swirldslabs.com>

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

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

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

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");
     }
 
     /**