theupdateframework / tuf-conformance

TUF client conformance test suite
MIT License
7 stars 4 forks source link

Make signatures deterministic by default? #130

Closed jku closed 3 weeks ago

jku commented 1 month ago

128 seems to be an example of the flaw in current RepositorySimulator signing key & signature management:

Unfortunately ecdsa signatures are not deterministic so the combination of these two features can easily lead to issues that are difficult to debug.

Obvious options to fix this include:

  1. start explicitly signing metadata -- I would still like to avoid this if possible
  2. switch to a deterministic signature by default

I think I lean to option 2, we should at least try this with another keytype:

jku commented 1 month ago
  1. start explicitly signing metadata

the upside of this is that we could more easily test e.g. #134

AdamKorcz commented 1 month ago

I would still like to avoid this if possible

Is this because of UX? - that it is simpler to write tests?

jku commented 1 month ago

Yeah: I was expecting most tests to modify targets metadata (which is currently very easy), but I'm beginning to think the benefits may not be greater than the disadvantages... The fact that modifying root is different from other roles is already annoying (considering that a lot of tests want to modify root).

I'd be happy to see a refactoring that does explicit signing in a clean way, but I have a feeling that's a large change -- at the very least it will touch every single test, and the repository http API will change slightly (as currently we return 404 on "old" version requests but if we have the old versions stored we would logically return them when asked). Would be nice to get the hashes/length length in metafiles issue solved first (as that's currently actually broken and means our test results are partly incorrect).

We could do both options: first the second option (hard code RSA keys), and then look at a explicit signing refactoring .

Opinions @AdamKorcz ?

jku commented 1 month ago

hard code RSA keys

alternative that has less hard coded key material would be to generate 20 signers once in conftest.py and pass a copy of the list to all RepositorySimulators when they are created

jku commented 1 month ago

Just writing down some notes on the "make RepositorySimulator sign explicitly" idea

I agree it seems like a good move but seems like quite a bit of work and a source of new bugs: this is why I suggested changing the default keytype so that we would have less bugs in code to begin with

jku commented 3 weeks ago

Quick test with rsa keys shows some issue with go-tuf... see https://gist.github.com/jku/5262baefeb705663999a43a7cf412bfc

jku commented 3 weeks ago

Quick test with rsa keys shows some issue with go-tuf... see https://gist.github.com/jku/5262baefeb705663999a43a7cf412bfc

It seems go-tuf has an issue with RSA-PSS: luckily we actually wanted RSA-PKCS anyway (since we're looking for reproducible signatures)

This seems to work:

-        signer = CryptoSigner.generate_ecdsa()
+        signer = CryptoSigner.generate_rsa(scheme="rsa-pkcs1v15-sha256")
jku commented 3 weeks ago

I'll take this to try the "quick fix" (RSA keys by default): let's see about the refactor afterwards

jku commented 3 weeks ago

I changed this issue to be about the quick fix (use rsa keys) and filed #155 for the refactor