-
Notifications
You must be signed in to change notification settings - Fork 521
Rotate remote key endpoint #889
Conversation
c5b56b7 to
f9355ab
Compare
7c1b1a9 to
4da73c6
Compare
1a8ca83 to
4bf82bd
Compare
683946c to
00e92ea
Compare
| @@ -0,0 +1 @@ | |||
| DROP TABLE `timestamp_keys`; No newline at end of file | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\o/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking - this is probably a bad idea from a historical point of view, but wondering if it's better to also remove the table from 0001 and add a IF EXISTS to this drop table command, that way it doesn't have to be created and then immediately dropped.
https://dev.mysql.com/doc/refman/5.7/en/drop-table.html to this if there is no error, and modify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be more than happy to add a IF EXISTS to this drop table but I'm not sure if we should edit existing migrations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure either. I guess you really only run migrations once, so creating and then destroying a table isn't an issue.
158c542 to
2223531
Compare
461e8ea to
bde7dc7
Compare
server/timestamp/timestamp_test.go
Outdated
| crypto = signed.NewEd25519() | ||
| k3, err := GetOrCreateTimestampKey("gun", store, crypto, data.ED25519Key) | ||
| require.Nil(t, err, "Expected nil error") | ||
| require.NotEqual(t, k, k3, "Did not receive same key when attempting to recreate.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking nitpick: should this be "Received same key when..."?
Similarly with the snapshot test.
|
Thanks for all your work on this! This LGTM again aside from some minor nits on naming! |
58f9de4 to
947943c
Compare
|
LGTM. Excited to merge this! @riyazdf can you rebase just so we can get a final test run to make sure everything is good, then we can merge if it's all green :-D |
Signed-off-by: Ying Li <ying.li@docker.com>
… Get RPC calls to also return the role in order to conform with the default cryptoservice behavior Signed-off-by: Ying Li <ying.li@docker.com>
Signed-off-by: Ying Li <ying.li@docker.com>
Signed-off-by: Ying Li <ying.li@docker.com>
…ned by mark as active Signed-off-by: Ying Li <ying.li@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
extra keys until we used a pending one Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
drop mysql table Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
…s implement cryptoservice Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
…eanup, add tests Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>
947943c to
e29b1ae
Compare
|
@endophage sounds good, just rebased! |
|
Yay all green! |
Stacked on #880 to make use of the key marking functionality
Implements remote key rotation for snapshot and timestamp keys via
notary key rotate <GUN> <ROLE> -r.In case this operation is interrupted, we will attempt to sign in any "pending keys" in the database on the next remote rotation -- each role/gun pair can only have one pending key.
This PR also removes the
timestamp_keystable in the notary server database, and instead parses out public keys from the TUF metadata in thetuf_filestable. If TUF metadata does not yet exist for a repo, we attempt to create new remote keys (using the same "pending" logic described above to prevent key explosion).Closes #846, #847, #752
Supersedes #553, #880