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

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

@@ -411,7 +411,7 @@
             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 @@
                 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 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 @@
                         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 @@
 
         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 @@
         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");