[#344] Tamper-evident CSV audit: sign the column names and the value lengths in the row HMAC

commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/CsvSecureUtils.java

@@ -17,10 +17,13 @@
 
 package org.forgerock.audit.handlers.csv;
 
+import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Map;
+import java.util.regex.Pattern;
 
 import org.forgerock.util.encode.Base64;
 import org.slf4j.Logger;
@@ -30,6 +33,19 @@
  */
 final class CsvSecureUtils {
 
+    /**
+     * The first byte of the data that the HMAC of a row covers. UTF-8 never holds it, so these data never equal
+     * {@link #concatenatedValues(Map, String...)}, the UTF-8 bytes of a row that a previous version signed: an HMAC
+     * verifies a row in one format only.
+     */
+    private static final int DATA_TO_SIGN_MARKER = 0xFF;
+
+    /**
+     * A line break that the CSV writer writes as its end of line symbols: a CR, a CRLF, or a LF, which the CSV reader
+     * reads back as a LF. The writer writes a CR followed by a LF as one line break.
+     */
+    private static final Pattern LINE_BREAK = Pattern.compile("\r\n?");
+
     /**
      * Obtain the data to sign.
      * @param lastSignature The last signature.
@@ -71,12 +87,49 @@ private static byte[] concat(byte[]... arrays) {
     }
 
     /**
-     * Get the data to sign.
+     * Get the data to sign: the byte 0xFF, then the name and the value of each column, as the CSV reader reads them
+     * back, in UTF-8, each one after the number of its bytes as a big-endian int.
+     * <p>
+     * The CSV reader reads an empty name or value as {@code null}, so both are signed as empty, and a line break
+     * as a LF, so a CR or a CRLF is signed as a LF.
      * @param logger the value of logger
      * @param values the value of values
      * @param nameMapping the value of nameMapping
      */
     static byte[] dataToSign(Logger logger, Map<String, ?> values, String... nameMapping) {
+        ByteArrayOutputStream data = new ByteArrayOutputStream();
+        data.write(DATA_TO_SIGN_MARKER);
+        for (String h : nameMapping) {
+            final Object value = values.get(h);
+            writeLengthAndBytes(data, readBack(h));
+            writeLengthAndBytes(data, readBack(value != null ? value.toString() : null));
+        }
+        return data.toByteArray();
+    }
+
+    private static void writeLengthAndBytes(ByteArrayOutputStream data, String field) {
+        final byte[] bytes = field.getBytes(StandardCharsets.UTF_8);
+        data.write(ByteBuffer.allocate(Integer.BYTES).putInt(bytes.length).array(), 0, Integer.BYTES);
+        data.write(bytes, 0, bytes.length);
+    }
+
+    /**
+     * Returns a name or a value as the CSV reader reads it back once written: empty if it is {@code null} or empty,
+     * with each line break as a LF.
+     * @param field the name or the value, may be {@code null}
+     * @return the name or the value as the CSV reader reads it back, empty for {@code null}
+     */
+    static String readBack(String field) {
+        return field != null ? LINE_BREAK.matcher(field).replaceAll("\n") : "";
+    }
+
+    /**
+     * Get the data that the HMAC of a row covered before {@link #dataToSign(Logger, Map, String...)}: the values
+     * concatenated, which do not tell where a value ends, nor its column.
+     * @param values the value of values
+     * @param nameMapping the value of nameMapping
+     */
+    static byte[] concatenatedValues(Map<String, ?> values, String... nameMapping) {
         StringBuilder tmp = new StringBuilder();
         for (String h : nameMapping) {
             final Object value = values.get(h);

commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/CsvSecureVerifier.java

@@ -196,9 +196,11 @@ private VerificationResult newVerificationSuccessResult() {
     private boolean verifyHMAC(Map<String, String> values, String[] header) throws IOException {
         try {
             String actualHMAC = values.get(HEADER_HMAC);
-            String expectedHMAC = hmacCalculator.calculate(dataToSign(logger, values, dropExtraHeaders(header)));
-            if (!actualHMAC.equals(expectedHMAC)) {
-                logger.trace("The HMAC is not valid. Expected : {} Found : {}", expectedHMAC, actualHMAC);
+            String[] columns = dropExtraHeaders(header);
+            // A row that a previous version wrote has the HMAC of its values concatenated.
+            if (!hmacCalculator.verify(actualHMAC, dataToSign(logger, values, columns),
+                    concatenatedValues(values, columns))) {
+                logger.trace("The HMAC is not valid. Found : {}", actualHMAC);
                 return false;
             } else {
                 lastHMAC = actualHMAC;

commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/HmacCalculator.java

@@ -65,6 +65,37 @@ public void setCurrentKey(byte[] bytes) {
      * @throws SignatureException if the HMAC cannot be generated.
      */
     public String calculate(byte[] data) throws SignatureException {
+        String result = hmac(data);
+
+        // Compute the next key's iteration
+        computeNextKeyIteration();
+
+        return result;
+    }
+
+    /**
+     * Tells whether the HMAC of one of the given data is the expected one. With the current key, computes the HMAC
+     * of each data in turn until one is the expected HMAC, then computes the next key's iteration once, as
+     * {@link #calculate(byte[])} does.
+     *
+     * @param expectedHmac the expected HMAC, base64 encoded
+     * @param data the data that the expected HMAC may have been calculated for, in the order they are tried
+     * @return {@code true} if the HMAC of one of the data is the expected one.
+     * @throws SignatureException if the HMAC cannot be generated.
+     */
+    public boolean verify(String expectedHmac, byte[]... data) throws SignatureException {
+        boolean verified = false;
+        for (int i = 0; i < data.length && !verified; i++) {
+            verified = hmac(data[i]).equals(expectedHmac);
+        }
+
+        // Compute the next key's iteration
+        computeNextKeyIteration();
+
+        return verified;
+    }
+
+    private String hmac(byte[] data) throws SignatureException {
         try {
             mac.reset();
             mac.init(currentKey);
@@ -73,12 +104,7 @@ public String calculate(byte[] data) throws SignatureException {
             byte[] rawHmac = mac.doFinal(data);
 
             // base64-encode the hmac
-            String result = Base64.encode(rawHmac);
-
-            // Compute the next key's iteration
-            computeNextKeyIteration();
-
-            return result;
+            return Base64.encode(rawHmac);
         } catch (InvalidKeyException | IllegalStateException e) {
             throw new SignatureException("Failed to generate HMAC : " + e.getMessage());
         }

commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/SecureCsvWriter.java

@@ -23,6 +23,7 @@
 import static org.forgerock.audit.handlers.csv.CsvSecureConstants.HEADER_SIGNATURE;
 import static org.forgerock.audit.handlers.csv.CsvSecureConstants.SIGNATURE_ALGORITHM;
 import static org.forgerock.audit.handlers.csv.CsvSecureUtils.dataToSign;
+import static org.forgerock.audit.handlers.csv.CsvSecureUtils.readBack;
 import static org.forgerock.util.Reject.checkNotNull;
 
 import java.io.File;
@@ -169,15 +170,14 @@ class SecureCsvWriter implements CsvWriter, RolloverLifecycleHook {
                     throw new IOException("The CSV file was tampered: " + verificationResult.getFailureReason());
                 }
 
-                // Assert that the 2 headers are equal.
+                // Assert that the 2 headers are equal, as the CSV reader reads them back.
                 final String[] actualHeaders = verifier.getHeaders();
                 if (actualHeaders != null) {
                     if (actualHeaders.length != headers.length) {
                         throw new IOException("Resuming an existing CSV file but the headers do not match.");
                     }
                     for (int idx = 0; idx < actualHeaders.length; idx++) {
-                        // Super CSV reads an empty column name as null, the expected headers hold none
-                        if (!headers[idx].equals(actualHeaders[idx])) {
+                        if (!readBack(actualHeaders[idx]).equals(readBack(headers[idx]))) {
                             throw new IOException("Resuming an existing CSV file but the headers do not match.");
                         }
                     }

commons/audit/handler-csv/src/test/java/org/forgerock/audit/handlers/csv/CsvSecureUtilsTest.java

@@ -0,0 +1,69 @@
+/*
+ * The contents of this file are subject to the terms of the Common Development and
+ * Distribution License (the License). You may not use this file except in compliance with the
+ * License.
+ *
+ * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the
+ * specific language governing permission and limitations under the License.
+ *
+ * When distributing Covered Software, include this CDDL Header Notice in each file and include
+ * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL
+ * Header, with the fields enclosed by brackets [] replaced by your own identifying
+ * information: "Portions copyright [year] [name of copyright owner]".
+ *
+ * Copyright 2026 3A Systems, LLC.
+ */
+package org.forgerock.audit.handlers.csv;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.annotations.Test;
+
+@SuppressWarnings("javadoc")
+public class CsvSecureUtilsTest {
+
+    private static final Logger logger = LoggerFactory.getLogger(CsvSecureUtilsTest.class);
+
+    @Test
+    public void shouldConcatenateNonNullValuesInHeaderOrder() {
+        // the data that the HMAC of a row covered before, which the files written then still need
+        Map<String, String> values = new HashMap<>();
+        values.put("a", "x");
+        values.put("b", null);
+        values.put("c", "z");
+
+        assertThat(CsvSecureUtils.concatenatedValues(values, "c", "b", "a"))
+                .isEqualTo("zx".getBytes(StandardCharsets.UTF_8));
+    }
+
+    @Test
+    public void shouldSignUtf8LengthsAndEmptyNamesAndValues() {
+        Map<String, String> values = new HashMap<>();
+        values.put("a", "é");
+
+        assertThat(CsvSecureUtils.dataToSign(logger, values, "a", "b", null))
+                .isEqualTo(new byte[] {
+                    (byte) 0xFF,
+                    0, 0, 0, 1, 'a', 0, 0, 0, 2, (byte) 0xC3, (byte) 0xA9,
+                    0, 0, 0, 1, 'b', 0, 0, 0, 0,
+                    0, 0, 0, 0, 0, 0, 0, 0 });
+    }
+
+    @Test
+    public void shouldSignLineBreaksAsLf() {
+        Map<String, String> values = new HashMap<>();
+        values.put("a\rb", "c\r\nd\r\r\ne\nf");
+
+        assertThat(CsvSecureUtils.dataToSign(logger, values, "a\rb"))
+                .isEqualTo(new byte[] {
+                    (byte) 0xFF,
+                    0, 0, 0, 3, 'a', '\n', 'b',
+                    0, 0, 0, 8, 'c', '\n', 'd', '\n', '\n', 'e', '\n', 'f' });
+    }
+}