empty (unused) signatures left in metadata

[jku]

during the root-signing-staging import (https://github.com/sigstore/root-signing-staging/pull/21) the registry.npmjs.org role was changed:

• key 314ae73abd3012fc73bfcc3783e31d03852716597642b891d6a33155c4baf600 was removed
• key 762cb22caca65de5e9b7b6baecb84ca989d337280ce6914b6440aea95769ad93 was added

After signing the metadata looked like this:

 "signatures": [
  {
   "keyid": "314ae73abd3012fc73bfcc3783e31d03852716597642b891d6a33155c4baf600",
   "sig": ""
  },
  {
   "keyid": "762cb22caca65de5e9b7b6baecb84ca989d337280ce6914b6440aea95769ad93",
   "sig": "3045022100dbcf90a4fe0364cd40017c6150c945492c6d8e775586ad9a6f0b719567a767d8022001beaa7f370a6154d9e9597bdfb424004575595e4e9b0399d81d11ded9ff8620"
  }
 ],

That first empty signature should not be there: the metadata is valid but that should not happen

[jku]

I think this might actually happen everytime a signer is removed. It cannot be too hard to figure out some improvement here: Options seem to be:

1. remove the sigs when the key is removed from the delegating metadata. this means the delegated metadata changes even if there are no delegated metadata content changes... there are two options:
   • either there are still enough sigs: this does not require any action from delegated role signers assuming the signature removal is smart enough (and does not bump the metadata version). I don't think we have code this clever
     • or there are no longer enough signatures: in this case it makes sense to require more signatures -- IMO also makes sense to bump the version at this point even if that might not be 100% required
2. when signing, check that existing sigs are valid sigs (remove any that are not)
   • this does not ensure that delegated metadata is still valid after signer removal (because delegated metadata is not signed when delegation is modified)

So the difference is mostly, do we want to make sure delegation changes don't break delegated metadata signatures (and how do we do it), see #95. Case 2 (we don't make those checks) is likely a lot easier

[jku]

I think what I want is:

• when delegations change, we should possibly validate existing delegated metadata and require re-signing invlaid delegated metadata in the same signing event: this is #95. I think always creating a new delegated version is appropriate (even if in some cases just resigning would theoretically work): I don't trust that all clients work correctly with resigned metadata versions
• when signing, make sure signatures dict contains sigs for the current keys only (taking into account root being a special case)
  • this is already handled in the case when metadata content changes happen: here the signatures get wiped and "rebuilt" for the current signers
  • this is not done when "just signing" (iow no content changes)... will have to investigate if this is incorrect

[jku]

yes we're hitting this:

• delegation changes (a signer is removed and another is added)
• this currently does not trigger delegated metadata edit (version bumps etc) as it should
• but the new signer happens to be asked for a signature and signs so the result is a valid repository

I believe this same thing happens in almost every case of this bug, and the issue will resolve itself on the next metadata version... so I'm inclined to not do workarounds at least at this point.

[jku]

After some more thought:

• there is a "legitimate" way to have empty signatures in metadata with tuf-on-ci:
  • if threshold is smaller than number of signers, and the signing event is finished when threshold is reached: in this case the remaining (non-signed) signers will have sig: "" in the metadata
  • it would be possible to remove those lines but I dont think it's a good idea (would break snapshot file hashes to begin with -- it's not a feature tuf-on-ci currently uses but it could be)
  • I believe it is fine for those lines to be in the metadata: spec does not require all signatures to match, it requires threshold signatures to match
• the example case in this issue is still a bug as 314ae73abd3012fc73bfcc3783e31d03852716597642b891d6a33155c4baf600 is no longer a signer for this role

My opinion is that clients should be able to process metadata with invalid signatures -- and if there are enough valid signatures, must consider the metadata valid.

[kommendorkapten]

My opinion is that clients should be able to process metadata with invalid signatures -- and if there are enough valid signatures, must consider the metadata valid.

Agree!