Merge pull request #852 from sigstore/fix_tuf_dup_keys

Fix test_duplicate_sig_keyids

Author: loosebazooka

sigstore-java/src/main/java/dev/sigstore/tuf/DuplicateKeyIdsException.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright 2022 The Sigstore Authors.
+ *
+ * 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 dev.sigstore.tuf;
+
+import dev.sigstore.tuf.model.Signature;
+import java.util.List;
+import java.util.Locale;
+import java.util.stream.Collectors;
+
+/**
+ * Thrown when the metadata has signatures from the same key even if the threshold is met. <a
+ * href="https://theupdateframework.github.io/specification/latest/#file-formats-object-format">4.2.1</a>
+ */
+public class DuplicateKeyIdsException extends TufException {
+
+  private final List<Signature> signatures;
+  private final String keyId;
+
+  public DuplicateKeyIdsException(List<Signature> signatures, String keyId) {
+    super(
+        String.format(
+            Locale.ROOT,
+            "The role has multiple signatures with the same key_id. [Signatures: %s, KeyId: %s]",
+            signatures.stream()
+                .map(Signature::getSignature)
+                .collect(Collectors.joining(",", "(", ")")),
+            keyId));
+    this.signatures = signatures;
+    this.keyId = keyId;
+  }
+
+  public List<Signature> getSignatures() {
+    return signatures;
+  }
+
+  public String getKeyId() {
+    return keyId;
+  }
+}

sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java

@@ -42,6 +42,7 @@
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 import org.bouncycastle.util.encoders.DecoderException;
 import org.bouncycastle.util.encoders.Hex;
 
@@ -211,8 +212,7 @@ private boolean hasNewKeys(RootRole oldRole, RootRole newRole) {
   }
 
   void verifyDelegate(Root trustedRoot, SignedTufMeta<? extends TufMeta> delegate)
-      throws SignatureVerificationException, IOException, NoSuchAlgorithmException,
-          InvalidKeySpecException {
+      throws SignatureVerificationException, IOException {
     verifyDelegate(
         delegate.getSignatures(),
         trustedRoot.getSignedMeta().getKeys(),
@@ -228,23 +228,31 @@ void verifyDelegate(Root trustedRoot, SignedTufMeta<? extends TufMeta> delegate)
    * @param role the key ids and threshold values for role signing
    * @param verificationMaterial the contents to be verified for authenticity
    * @throws SignatureVerificationException if there are not enough verified signatures
+   * @throws IOException if an error occurred parsing a key
    */
   @VisibleForTesting
   void verifyDelegate(
       List<Signature> signatures,
       Map<String, Key> publicKeys,
       Role role,
       byte[] verificationMaterial)
-      throws InvalidKeySpecException, IOException, NoSuchAlgorithmException {
+      throws IOException {
     // use set to not count the same key multiple times towards the threshold.
     var goodSigs = new HashSet<>(role.getKeyids().size() * 4 / 3);
     // role.getKeyIds() defines the keys allowed to sign for this role.
     for (String keyid : role.getKeyids()) {
-      Optional<Signature> signatureMaybe =
-          signatures.stream().filter(sig -> sig.getKeyId().equals(keyid)).findFirst();
+      List<Signature> matchingSignatures =
+          signatures.stream()
+              .filter(sig -> sig.getKeyId().equals(keyid))
+              .collect(Collectors.toList());
+      // check for any duplicate key_ids:
+      // https://theupdateframework.github.io/specification/latest/#file-formats-object-format
+      if (matchingSignatures.size() > 1) {
+        throw new DuplicateKeyIdsException(matchingSignatures, keyid);
+      }
       // only verify if we find a signature that matches an allowed key id.
-      if (signatureMaybe.isPresent()) {
-        var signature = signatureMaybe.get();
+      if (matchingSignatures.size() == 1) {
+        var signature = matchingSignatures.get(0);
         // look for the public key that matches the key ID and use it for verification.
         var key = publicKeys.get(signature.getKeyId());
         if (key != null) {

tuf-cli/tuf-cli.xfails

@@ -1,5 +1,4 @@
 test_metadata_bytes_match
-test_duplicate_sig_keyids
 test_unusual_role_name[?]
 test_unusual_role_name[#]
 test_unusual_role_name[/delegatedrole]