fix(sdk): reject GMAC root signatures on read (DSPX-4703)

A ZTDF's root signature is the only thing that authenticates the manifest's
ordered list of segment hashes. AES-GCM tags bind a segment's own bytes and
nothing about its index, its neighbours, or how many segments there are, so
segment-level integrity cannot notice a truncated, reordered, or duplicated
segment list.

One signature routine served both jobs. For a segment, "GMAC" correctly means
reading back the AES-GCM tag the cipher just computed over that segment's
ciphertext. For the root it means nothing: the aggregate hash never passes
through AES-GCM, so there is no tag to recover and the code returned a copy of
the trailing bytes of its own input, i.e. the last segment hash. Manifest data
compared against manifest data, with the payload key never used.

The algorithm was read from `rootSignature.alg` in the manifest, which is not
authenticated. An attacker with no key could therefore take an HS256-rooted
TDF, rewrite the root to "GMAC" with a signature copied from the last segment
hash, and then truncate, reorder, or duplicate segments with the file still
verifying.

Changes:
  - Split the routine into segmentIntegrity (HS256 or GMAC) and rootIntegrity
    (HS256 only), so tag extraction can no longer be pointed at a non-AEAD
    input.
  - rootIntegrityAlgorithmFromManifest resolves the root algorithm against an
    allowlist and throws SDK.RootSignatureValidationException for anything
    else, instead of coercing unknown values to HS256.
  - segmentIntegrityAlgorithmFromManifest stays permissive, since both
    algorithms are meaningful over ciphertext.
  - createTDF validates the configured root algorithm too, so the SDK will not
    write a file it would refuse to read.

rootIntegrity validates its own argument in addition to its caller checking
first. The redundancy is deliberate: the check is what makes the function safe,
so it belongs with the function rather than only at today's call sites.

Tests cover truncation, reordering, GMAC in several casings, an unknown
algorithm that must not be coerced, and controls that must keep passing.
Verified by mutation: restoring the old tag-extraction branch inside
rootIntegrity turns exactly the 8 exploit tests red and leaves the other 21
green.

Refs: DSPX-4703, and the write-side controls in DSPX-4736.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>

Author: dmihalcik-virtru

sdk/src/main/java/io/opentdf/platform/sdk/TDF.java

@@ -411,7 +411,7 @@ public Manifest getManifest() {
             this.unencryptedMetadata = unencryptedMetadata;
         }
 
-        public void readPayload(OutputStream outputStream) throws SDK.SegmentSignatureMismatch, IOException {
+        public void readPayload(OutputStream outputStream) throws SDK.TamperException, IOException {
 
             MessageDigest digest = null;
             try {
@@ -438,13 +438,10 @@ public void readPayload(OutputStream outputStream) throws SDK.SegmentSignatureMi
                 var isLegacyTdf = manifest.tdfVersion == null || manifest.tdfVersion.isEmpty();
 
                 if (manifest.payload.isEncrypted) {
-                    String segHashAlg = manifest.encryptionInformation.integrityInformation.segmentHashAlg;
-                    Config.IntegrityAlgorithm sigAlg = Config.IntegrityAlgorithm.HS256;
-                    if (segHashAlg.compareToIgnoreCase(kGmacIntegrityAlgorithm) == 0) {
-                        sigAlg = Config.IntegrityAlgorithm.GMAC;
-                    }
+                    var sigAlg = segmentIntegrityAlgorithmFromManifest(
+                            manifest.encryptionInformation.integrityInformation.segmentHashAlg);
 
-                    var payloadSig = calculateSignature(readBuf, payloadKey, sigAlg);
+                    var payloadSig = segmentIntegrity(readBuf, payloadKey, sigAlg);
                     if (isLegacyTdf) {
                         payloadSig = Hex.encodeHexString(payloadSig).getBytes(StandardCharsets.UTF_8);
                     }
@@ -470,22 +467,164 @@ public void readPayload(OutputStream outputStream) throws SDK.SegmentSignatureMi
         public PolicyObject readPolicyObject() {
             return tdfReader.readPolicyObject();
         }
-    }
 
-    private static byte[] calculateSignature(byte[] data, byte[] secret, Config.IntegrityAlgorithm algorithm) {
-        if (algorithm == Config.IntegrityAlgorithm.HS256) {
-            return CryptoUtils.CalculateSHA256Hmac(secret, data);
+        /**
+         * Resolves {@code segmentHashAlg} as read from the manifest. Both algorithms are
+         * allowed: a GMAC segment hash proves nothing by itself, but unlike the root it is
+         * bracketed by keyed checks that do (see {@link TDF#aeadTag}). An unrecognized name
+         * is still refused rather than defaulted. Contrast
+         * {@link TDF#rootIntegrityAlgorithmFromManifest}, where only HS256 is meaningful.
+         */
+        private static Config.IntegrityAlgorithm segmentIntegrityAlgorithmFromManifest(String declared) {
+            if (declared != null) {
+                String name = declared.trim();
+                if (kGmacIntegrityAlgorithm.equalsIgnoreCase(name)) {
+                    return Config.IntegrityAlgorithm.GMAC;
+                }
+                if (kHmacIntegrityAlgorithm.equalsIgnoreCase(name)) {
+                    return Config.IntegrityAlgorithm.HS256;
+                }
+            }
+            // Not a SegmentSignatureMismatch: no signature was compared. Still a
+            // TamperException, because segmentHashAlg is not covered by the root
+            // signature and so is something an attacker can freely rewrite.
+            throw new SDK.TamperException("unsupported segment integrity algorithm: " + declared);
         }
+    }
 
-        if (kGMACPayloadLength > data.length) {
+    /**
+     * Recovers the trailing AES-GCM authentication tag from a segment's ciphertext.
+     * <p>
+     * Recovering a tag is not verifying one. These are bytes whoever supplied the input
+     * already holds, so comparing them against a manifest value is keyless and on its own
+     * proves nothing — an attacker can re-chunk a payload and write each chunk's own
+     * trailing sixteen bytes into its {@code segment.hash}. What makes a GMAC segment hash
+     * trustworthy is the keyed checks around it: {@code loadTDF} has already validated the
+     * whole list of segment hashes against the HS256 root signature, and {@code readPayload}
+     * follows the comparison with a real AES-GCM tag check under the payload key.
+     * <p>
+     * The root signature has neither backstop — it is the outermost check, so a "GMAC root"
+     * is a keyless comparison with nothing behind it. The asymmetry is therefore structural,
+     * not a property of the bytes, and it is why {@link #rootIntegrity} does not offer this
+     * algorithm.
+     */
+    private static byte[] aeadTag(byte[] ciphertext) {
+        if (kGMACPayloadLength > ciphertext.length) {
             throw new IllegalArgumentException("tried to calculate GMAC on too small a payload. payload is "
-                    + data.length + "bytes while GMAC is " + kGMACPayloadLength + " bytes");
+                    + ciphertext.length + " bytes while GMAC is " + kGMACPayloadLength + " bytes");
+        }
+
+        return Arrays.copyOfRange(ciphertext, ciphertext.length - kGMACPayloadLength, ciphertext.length);
+    }
+
+    /**
+     * The integrity value recorded in a segment's {@code hash}.
+     *
+     * @param ciphertext the AES-GCM output for this segment, whole and unmodified
+     * @param key        the payload key
+     * @param algorithm  {@code GMAC} to reuse the segment's own AEAD tag, or
+     *                   {@code HS256} to HMAC the segment ciphertext
+     * @throws IllegalArgumentException if {@code algorithm} is null or unsupported
+     */
+    static byte[] segmentIntegrity(byte[] ciphertext, byte[] key, Config.IntegrityAlgorithm algorithm) {
+        requireSupportedSegmentIntegrityAlgorithm(algorithm);
+        switch (algorithm) {
+            case HS256:
+                return CryptoUtils.CalculateSHA256Hmac(key, ciphertext);
+            case GMAC:
+                return aeadTag(ciphertext);
+            default:
+                throw new IllegalArgumentException("unsupported segment integrity algorithm: " + algorithm);
+        }
+    }
+
+    /**
+     * The integrity value recorded in {@code rootSignature.sig}, over the concatenated
+     * segment hashes.
+     * <p>
+     * HS256 only. The aggregate hash never passes through the AEAD, so there is no tag
+     * to recover from it; a "GMAC" root signature is just a copy of the last segment
+     * hash, which is attacker-controlled manifest data. Accepting one would let anyone
+     * truncate, reorder, duplicate or drop segments without holding a key, since nothing
+     * else binds a segment to its index or to the segment count.
+     *
+     * @throws IllegalArgumentException if {@code algorithm} is anything but HS256
+     */
+    static byte[] rootIntegrity(byte[] aggregateHash, byte[] key, Config.IntegrityAlgorithm algorithm) {
+        requireSupportedRootIntegrityAlgorithm(algorithm);
+        return CryptoUtils.CalculateSHA256Hmac(key, aggregateHash);
+    }
+
+    /**
+     * The segment counterpart to {@link #requireSupportedRootIntegrityAlgorithm}. Both
+     * algorithms are legal in this position, so this exists to reject {@code null} and any
+     * future enum value in {@code createTDF} rather than partway through the payload:
+     * TDFConfig's fields are public, so a field left unset arrives here as {@code null} and
+     * would otherwise surface as a {@link NullPointerException} at the switch below, after
+     * segments had already been written to the output stream.
+     *
+     * @throws IllegalArgumentException if {@code algorithm} cannot hash a segment
+     */
+    static void requireSupportedSegmentIntegrityAlgorithm(Config.IntegrityAlgorithm algorithm) {
+        if (algorithm != Config.IntegrityAlgorithm.HS256 && algorithm != Config.IntegrityAlgorithm.GMAC) {
+            throw new IllegalArgumentException("unsupported segment integrity algorithm: " + algorithm);
+        }
+    }
+
+    /**
+     * The write-path gate, and a second checkpoint inside {@link #rootIntegrity}.
+     * <p>
+     * An {@link IllegalArgumentException} rather than a {@link SDK.TamperException}, on
+     * purpose. Reading, this is unreachable: {@link #rootIntegrityAlgorithmFromManifest}
+     * has already narrowed the manifest's declaration to HS256 or thrown
+     * {@link SDK.RootSignatureValidationException} trying. So if it ever does fire on a
+     * read, the cause is a bug in this class rather than a hostile file, and it should
+     * escape {@code loadTDF} uncaught instead of being reported to callers as tamper —
+     * fail loud, and do not let a defect hide inside an exception type that callers
+     * routinely handle.
+     *
+     * @throws IllegalArgumentException if {@code algorithm} cannot authenticate a root
+     *                                  signature
+     */
+    static void requireSupportedRootIntegrityAlgorithm(Config.IntegrityAlgorithm algorithm) {
+        if (algorithm != Config.IntegrityAlgorithm.HS256) {
+            throw new IllegalArgumentException("unsupported root integrity algorithm: " + algorithm
+                    + "; the root signature must be " + kHmacIntegrityAlgorithm);
         }
+    }
 
-        return Arrays.copyOfRange(data, data.length - kGMACPayloadLength, data.length);
+    /**
+     * Resolves {@code rootSignature.alg} as read from the (unauthenticated) manifest.
+     * <p>
+     * An allowlist, deliberately: anything other than HS256 — GMAC, an unknown name, an
+     * empty string — is refused rather than being defaulted to HS256. Defaulting would
+     * validate a downgraded manifest against an algorithm it does not declare.
+     */
+    private static Config.IntegrityAlgorithm rootIntegrityAlgorithmFromManifest(String declared) {
+        if (declared != null && kHmacIntegrityAlgorithm.equalsIgnoreCase(declared.trim())) {
+            return Config.IntegrityAlgorithm.HS256;
+        }
+        throw new SDK.RootSignatureValidationException("unsupported root integrity algorithm: " + declared
+                + "; the root signature must be " + kHmacIntegrityAlgorithm);
     }
 
+    /**
+     * @throws IllegalArgumentException if {@code tdfConfig} selects an integrity algorithm
+     *                                  that cannot be written. Unchecked and not an
+     *                                  {@link SDKException}, matching how the config layer
+     *                                  already reports out-of-range values (see
+     *                                  {@link Config#withSegmentSize}): this is a caller
+     *                                  mistake to fix in code, not a condition to handle
+     *                                  alongside I/O and tamper failures.
+     */
     TDFObject createTDF(InputStream payload, OutputStream outputStream, Config.TDFConfig tdfConfig) throws SDKException, IOException {
+        // Checked before anything is written so an unusable algorithm cannot produce a
+        // partial TDF. There are no setters for these -- the config defaults to an HS256
+        // root and GMAC segments -- but TDFConfig's fields are public, so re-check what
+        // was actually set.
+        requireSupportedRootIntegrityAlgorithm(tdfConfig.integrityAlgorithm);
+        requireSupportedSegmentIntegrityAlgorithm(tdfConfig.segmentIntegrityAlgorithm);
+
         Planner planner = new Planner(tdfConfig, services, Autoconfigure::createGranter);
         Map<String, List<KASInfo>> splits = planner.getSplits();
 
@@ -526,7 +665,7 @@ TDFObject createTDF(InputStream payload, OutputStream outputStream, Config.TDFCo
                         readBuf, 0, readThisLoop);
                 payloadOutput.write(cipherData);
 
-                segmentSig = calculateSignature(cipherData, tdfObject.payloadKey, tdfConfig.segmentIntegrityAlgorithm);
+                segmentSig = segmentIntegrity(cipherData, tdfObject.payloadKey, tdfConfig.segmentIntegrityAlgorithm);
                 if (tdfConfig.hexEncodeRootAndSegmentHashes) {
                     segmentSig = Hex.encodeHexString(segmentSig).getBytes(StandardCharsets.UTF_8);
                 }
@@ -542,18 +681,17 @@ TDFObject createTDF(InputStream payload, OutputStream outputStream, Config.TDFCo
 
         Manifest.RootSignature rootSignature = new Manifest.RootSignature();
 
-        byte[] rootSig = calculateSignature(aggregateHash.toByteArray(), tdfObject.payloadKey,
+        byte[] rootSig = rootIntegrity(aggregateHash.toByteArray(), tdfObject.payloadKey,
                 tdfConfig.integrityAlgorithm);
         byte[] encodedRootSig = tdfConfig.hexEncodeRootAndSegmentHashes
                 ? Hex.encodeHexString(rootSig).getBytes(StandardCharsets.UTF_8)
                 : rootSig;
         rootSignature.signature = Base64.getEncoder().encodeToString(encodedRootSig);
 
-        String alg = kGmacIntegrityAlgorithm;
-        if (tdfConfig.integrityAlgorithm == Config.IntegrityAlgorithm.HS256) {
-            alg = kHmacIntegrityAlgorithm;
-        }
-        rootSignature.algorithm = alg;
+        // Unconditional. createTDF refuses any other root algorithm before a byte is
+        // written and rootIntegrity would refuse it again; selecting on tdfConfig here
+        // would leave a path that emits alg="GMAC" should either check ever be relaxed.
+        rootSignature.algorithm = kHmacIntegrityAlgorithm;
 
         tdfObject.manifest.encryptionInformation.integrityInformation.rootSignature = rootSignature;
         tdfObject.manifest.encryptionInformation.integrityInformation.segmentSizeDefault = tdfConfig.defaultSegmentSize;
@@ -757,17 +895,21 @@ Reader loadTDF(SeekableByteChannel tdf, Config.TDFReaderConfig tdfReaderConfig)
         String rootSigValue;
         boolean isLegacyTdf = manifest.tdfVersion == null || manifest.tdfVersion.isEmpty();
         if (manifest.payload.isEncrypted) {
-            Config.IntegrityAlgorithm sigAlg = Config.IntegrityAlgorithm.HS256;
-            if (rootAlgorithm.compareToIgnoreCase(kGmacIntegrityAlgorithm) == 0) {
-                sigAlg = Config.IntegrityAlgorithm.GMAC;
-            }
+            var sigAlg = rootIntegrityAlgorithmFromManifest(rootAlgorithm);
 
-            var sig = calculateSignature(aggregateHash.toByteArray(), payloadKey, sigAlg);
+            var sig = rootIntegrity(aggregateHash.toByteArray(), payloadKey, sigAlg);
             if (isLegacyTdf) {
                 sig = Hex.encodeHexString(sig).getBytes();
             }
             rootSigValue = Base64.getEncoder().encodeToString(sig);
         } else {
+            // KNOWN GAP, untouched by this change and tracked separately: this branch is a
+            // bare SHA-256, so it authenticates nothing. `payload.isEncrypted` is itself
+            // unauthenticated manifest data, so flipping it to false selects a keyless
+            // verification path that anyone can satisfy -- and readPayload then emits the
+            // segment bytes without decrypting them. Same downgrade shape as a GMAC root.
+            // Left alone here because closing it means deciding whether unencrypted TDFs
+            // are supported at all, which is a wider question than this fix.
             MessageDigest digest;
             try {
                 digest = MessageDigest.getInstance("SHA-256");