Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redo staging TUF root to not contain deprecated keys #140

Open
steiza opened this issue Apr 2, 2024 · 5 comments
Open

Redo staging TUF root to not contain deprecated keys #140

steiza opened this issue Apr 2, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@steiza
Copy link
Member

steiza commented Apr 2, 2024

Description

As clients add support for --staging, they will need to support the staging TUF root, which today includes a key of type PKCS1_RSA_PKCS1V5 which is deprecated in protobuf-specs v0.3.x.

As clients adopt protobuf-specs v0.3.x, instead of having them add support for a deprecated key type, we could re-make the staging root to not use deprecated keys. We probably do want at least 2 ctlog keys, one of which is outside of its valid time range, to ensure clients are properly verifying keys that were previously valid.

@steiza steiza added the enhancement New feature or request label Apr 2, 2024
@haydentherapper
Copy link
Collaborator

SGTM. I would recommend we simply duplicate the key (example below), and instruct clients to ignore any keys whose keyDetails they do not understand so they can choose to drop support for the deprecated values. I'm not sure how clients are handling this currently, if they fail or simply ignore the keys - @woodruffw, what's python do for example?

If we choose to wipe the log, we'll modify the set of trusted keys later to remove the duplicate, or wait until all clients have been updated to support the new keyDetails values.

Extra key:

    {
      "baseUrl": "https://ctfe.sigstage.dev/test",
      "hashAlgorithm": "SHA2_256",
      "publicKey": {
        "rawBytes": "MIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA27A2MPQXm0I0v7/Ly5BIauDjRZF5Jor9vU+QheoE2UIIsZHcyYq3slHzSSHy2lLj1ZD2d91CtJ492ZXqnBmsr4TwZ9jQ05tW2mGIRI8u2DqN8LpuNYZGz/f9SZrjhQQmUttqWmtu3UoLfKz6NbNXUnoo+NhZFcFRLXJ8VporVhuiAmL7zqT53cXR3yQfFPCUDeGnRksnlhVIAJc3AHZZSHQJ8DEXMhh35TVv2nYhTI3rID7GwjXXw4ocz7RGDD37ky6p39Tl5NB71gT1eSqhZhGHEYHIPXraEBd5+3w9qIuLWlp5Ej/K6Mu4ELioXKCUimCbwy+Cs8UhHFlqcyg4AysOHJwIadXIa8LsY51jnVSGrGOEBZevopmQPNPtyfFY3dmXSS+6Z3RD2Gd6oDnNGJzpSyEk410Ag5uvNDfYzJLCWX9tU8lIxNwdFYmIwpd89HijyRyoGnoJ3entd63cvKfuuix5r+GHyKp1Xm1L5j5AWM6P+z0xigwkiXnt+adexAl1J9wdDxv/pUFEESRF4DG8DFGVtbdH6aR1A5/vD4krO4tC1QYUSeyL5Mvsw8WRqIFHcXtgybtxylljvNcGMV1KXQC8UFDmpGZVDSHx6v3e/BHMrZ7gjoCCfVMZ/cFcQi0W2AIHPYEMH/C95J2r4XbHMRdYXpovpOoT5Ca78gsCAwEAAQ==",
        "keyDetails": "PKIX_RSA_PKCS1V15_4096_SHA256",
        "validFor": {
          "start": "2021-03-14T00:00:00.000Z",
          "end": "2022-07-31T00:00:00.000Z"
        }
      },
      "logId": {
        "keyId": "G3wUKk6ZK6ffHh/FdCRUE2wVekyzHEEIpSG4savnv0w="
      }
    },

@haydentherapper
Copy link
Collaborator

Key generated with https://go.dev/play/p/O-xYbvvgCaY

@woodruffw
Copy link
Member

I'm not sure how clients are handling this currently, if they fail or simply ignore the keys - @woodruffw, what's python do for example?

I believe sigstore-python currently ignores keyDetails entirely, and blindly loads keys (the only supported types being ECDSA and RSA at the moment, both with SHA-256).

Here's where we naively load keys, whether PEM or DER:

https://github.com/sigstore/sigstore-python/blob/dc4aa82feaa18fbba93b05dcf71383751c56ba33/sigstore/_internal/trustroot.py#L113-L129

and verify checks the key's type, hardcoding the hash as SHA-256:

https://github.com/sigstore/sigstore-python/blob/dc4aa82feaa18fbba93b05dcf71383751c56ba33/sigstore/_internal/trustroot.py#L138-L170

(This is entirely non-ideal 😅 -- I think it predates TUF and the trusted root significantly. I'll look into refactoring it to make sure that we both handle keyDetails and also select the hash based on it.)

@haydentherapper
Copy link
Collaborator

FWIW, in Golang, we'll likely do the same, because the standard libraries do a great job of abstracting away details about the key algorithm that we don't need any hints. And where the standard libraries don't handle encodings like PKIX vs PKCS, we made sigstore/sigstore as another abstraction.

So I guess we need to let clients know to fail gracefully regardless if they want to look at keyDetails or blindly load in keys - either approach seems fine!

@kommendorkapten
Copy link
Member

Sigstore JavaScript respects the key type as the underlying crypto package needs the information when parsing DER encoded data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants