Rotate remote key endpoint #889

riyazdf · 2016-07-29T23:14:50Z

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_keys table in the notary server database, and instead parses out public keys from the TUF metadata in the tuf_files table. 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

cyli · 2016-08-02T22:42:25Z

migrations/server/mysql/0004_drop_timestamp_key.up.sql

@@ -0,0 +1 @@
+DROP TABLE `timestamp_keys`;


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

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?

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.

cyli · 2016-08-09T06:19:12Z

server/timestamp/timestamp_test.go

+	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.")


Non-blocking nitpick: should this be "Received same key when..."?

Similarly with the snapshot test.

cyli · 2016-08-09T06:21:27Z

Thanks for all your work on this! This LGTM again aside from some minor nits on naming!

endophage · 2016-08-09T23:15:14Z

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>

…ned by mark as active Signed-off-by: Ying Li <ying.li@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>

…s implement cryptoservice 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>

riyazdf · 2016-08-09T23:26:38Z

@endophage sounds good, just rebased!

cyli · 2016-08-10T00:44:52Z

Yay all green!

GordonTheTurtle added status/0-triage labels Jul 29, 2016

riyazdf force-pushed the rotate-key-endpoint branch from c5b56b7 to f9355ab Compare July 29, 2016 23:19

riyazdf added this to the Notary 0.4 milestone Jul 29, 2016

riyazdf force-pushed the rotate-key-endpoint branch 8 times, most recently from 7c1b1a9 to 4da73c6 Compare August 1, 2016 22:52

riyazdf changed the title ~~WIP: Rotate remote key endpoint~~ Rotate remote key endpoint Aug 1, 2016

riyazdf force-pushed the rotate-key-endpoint branch 2 times, most recently from 1a8ca83 to 4bf82bd Compare August 2, 2016 17:00

cyli mentioned this pull request Aug 2, 2016

Signer key management refactor #880

Closed

riyazdf force-pushed the rotate-key-endpoint branch 3 times, most recently from 683946c to 00e92ea Compare August 2, 2016 22:17

cyli mentioned this pull request Aug 2, 2016

WIP: Client rotate remote key #553

Closed

cyli reviewed Aug 2, 2016
View reviewed changes

riyazdf force-pushed the rotate-key-endpoint branch 4 times, most recently from 158c542 to 2223531 Compare August 3, 2016 01:33

riyazdf force-pushed the rotate-key-endpoint branch 3 times, most recently from 461e8ea to bde7dc7 Compare August 9, 2016 02:29

cyli reviewed Aug 9, 2016
View reviewed changes

riyazdf force-pushed the rotate-key-endpoint branch 2 times, most recently from 58f9de4 to 947943c Compare August 9, 2016 17:11

cyli and others added 15 commits August 9, 2016 16:26

Respect the GUN and Role passed to the signer

f623705

Signed-off-by: Ying Li <ying.li@docker.com>

Update the signer RPC interface to take a role and a gun, and for the…

6efbfde

… 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>

Add LastUsed field so we can tell when a key was last active

8ff52f4

Signed-off-by: Ying Li <ying.li@docker.com>

Mark a signer key as active if it is used to sign

5815f53

Signed-off-by: Ying Li <ying.li@docker.com>

Do not surface MarkActive to the rpc api - make the private key retur…

7ffd28e

…ned by mark as active Signed-off-by: Ying Li <ying.li@docker.com>

Introduce rotate remote key server endpoint

d28e0f2

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Rip out timestamp_keys public key table

1d2e832

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Wire up pending key method to use on signer create, do not create

ccbfd28

extra keys until we used a pending one Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Use keyinfo instead of strings, bring back tests, and add migration to

8cda3cd

drop mysql table Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Adding tests to client, server, and httpstore. Update docs

8f83315

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

More testing, cleanup

66f35f8

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Do not bubble up pending key logic to RPC API, instead make signer DB…

d75fcde

…s implement cryptoservice Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Streamline tests and address review

1d97f3f

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Fix timestamp update logic

b491b6e

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

Address comments from group review: 0.3 rotation compatibility and cl…

e29b1ae

…eanup, add tests Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

riyazdf force-pushed the rotate-key-endpoint branch from 947943c to e29b1ae Compare August 9, 2016 23:26

cyli merged commit b4d4ab6 into master Aug 10, 2016

cyli deleted the rotate-key-endpoint branch August 10, 2016 00:45

riyazdf mentioned this pull request Aug 10, 2016

Provide endpoint to rotate server-managed keys #347

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rotate remote key endpoint #889

Rotate remote key endpoint #889

riyazdf commented Jul 29, 2016 •

edited

Loading

cyli Aug 2, 2016

cyli Aug 2, 2016

riyazdf Aug 3, 2016

cyli Aug 3, 2016

cyli Aug 9, 2016 •

edited

Loading

cyli commented Aug 9, 2016

endophage commented Aug 9, 2016

riyazdf commented Aug 9, 2016

cyli commented Aug 10, 2016

Rotate remote key endpoint #889

Rotate remote key endpoint #889

Conversation

riyazdf commented Jul 29, 2016 • edited Loading

cyli Aug 2, 2016

Choose a reason for hiding this comment

cyli Aug 2, 2016

Choose a reason for hiding this comment

riyazdf Aug 3, 2016

Choose a reason for hiding this comment

cyli Aug 3, 2016

Choose a reason for hiding this comment

cyli Aug 9, 2016 • edited Loading

Choose a reason for hiding this comment

cyli commented Aug 9, 2016

endophage commented Aug 9, 2016

riyazdf commented Aug 9, 2016

cyli commented Aug 10, 2016

riyazdf commented Jul 29, 2016 •

edited

Loading

cyli Aug 9, 2016 •

edited

Loading