Skip to content

Support multiple ctfe keys #190

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

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Support multiple ctfe keys #190

merged 1 commit into from
Oct 24, 2022

Conversation

loosebazooka
Copy link
Member

Signed-off-by: Appu Goundan [email protected]

This is kind of a temporary solution till we get tuf ready. I still think we should make some sort of "verification material provider" interface that our TUF client implements and maybe there are other implementations?. But... this fixes #188 for now.

@loosebazooka loosebazooka requested review from patflynn and vlsi October 21, 2022 20:08
@patflynn
Copy link
Collaborator

my plan was to expose a simple lookup method per client that returns the public key for a specific URL. e.g. getPublicKey(URL).

@loosebazooka
Copy link
Member Author

yeah sgtm, we can refactor for that when ready.

Resources.getResource("dev/sigstore/tuf/staging/fulcio.crt.pem"));
}

public static List<byte[]> ctfePublicKeys() throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is temporary code but why not just parse the pub keys right here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh parse them into keys? I dunno, I guess we could stop sending byte[]s around.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it might be weird for each implementation to have to parse it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lemme handle this is a followup. We'll return certs, and keys instead of just bytes everywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -13,9 +13,9 @@ We keep copies of the remote tuf repositories locally in
for interfacing with *.sigstage.dev

For this client to function we need the following keys
1. CTFE public key (`ctfe.pub` \
1. CTFE public keys (`ctfe.pub`, `ctfe_2022.pub`) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need the old key?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need the old key to verify that older SCTs embedded into older certs can still be verified (since they are not re-issued).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I need to update htis readme

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I guess we need the full history to verify old rekor entries with SCTs that were created with older keys.. I need to talk to Asra I think because I thought she said it was fine to just bundle the latest in the client.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need a full key history for all of sigstore (unless there is an expiry date for software?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeaah there's still a bit more to do here. We need to allow the client to accept multiple fulcioRoots and rekorPubKeys.

Signed-off-by: Appu Goundan <[email protected]>
@loosebazooka loosebazooka force-pushed the support-multiple-ctfe-keys branch from cd160c5 to b5798fc Compare October 21, 2022 20:22
@loosebazooka
Copy link
Member Author

@patflynn can we get this in and then followup, it'll fix the broken build.

@loosebazooka loosebazooka merged commit 2fa972d into main Oct 24, 2022
@loosebazooka loosebazooka deleted the support-multiple-ctfe-keys branch October 24, 2022 10:43
@haydentherapper
Copy link
Collaborator

@loosebazooka We also need to add the new CTFE prod key, correct?

@loosebazooka
Copy link
Member Author

Correct. Will add in soon.

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

Successfully merging this pull request may close these issues.

ctfe key rotated, we need to handle this
3 participants