theupdateframework / tuf-conformance

TUF client conformance test suite
MIT License
8 stars 6 forks source link

RepositorySimulator._compute_hashes_and_length is broken #128

Closed jku closed 2 months ago

jku commented 2 months ago

EDIT: Every test that uses RepositorySimulator._compute_hashes_and_length = True is currently broken because :

This should get fixed if any option of #130 is implemented

Original report was :


test_snapshot_rollback_with_local_snapshot_hash_mismatch() does not seem right:

I can't quite see what the issue is.

We should

jku commented 2 months ago

find the root cause here

oh, it's likely the combination of:

the hashes/length are over the whole file so include signatures.

This (the fact that making a mistake like this is so easy) is certainly a major downside to signing at request time so we may have to reconsider that decision... Still, maybe switching to a different default key type would also work. I originally only chose ecdsa because they are fast to generate (unlike rsa).

jku commented 2 months ago

I filed an issue for switching the signing keytype (or otherwise solving the underlying issue).

That said, the test is still completely wrong: it claims to test a targets rollback but does not actually try to do one.

AdamKorcz commented 2 months ago

Should this remain open, @jku?

jku commented 2 months ago

yeah, that makes sense: currently client fails as expected but for wrong reasons.

Once #131 is implemented (and #130 is fixed) we have more confidence that the failure happens for right reason

jku commented 2 months ago

test_new_targets_hash_mismatch() is broken because of this as well: If any return values had been checked that would have been visible