Implement revocation service from Python code #134

ashcrow · 2020-10-15T20:55:36Z

Adds:

Cargo: Add zmq and bump versions
crypto: rsa_verify function
main: run_revocation_service implementation
tests/Dockerfile: Updated dep

Note: Callback is not implemented here.

See: #92

src/main.rs

ashcrow · 2020-10-15T21:01:18Z

Will fix format issues

lkatalin

I think this makes sense, with the caveat that it should get some more sets of eyes as I'm new to Keylime.

lukehinds · 2020-12-04T08:09:21Z

The code looks good to me. Would it take much to refactor into a zmq.rs or perhaps revocation.rs might be better? I am aware I left that placeholder function for you to use, but I am now mindful that we should try to keep main.rs slimmed down when we can.

The other thing would be some unit tests, but I am understanding this might not be easy with ZMQ and might be more suited to a functional / integration test.

ashcrow · 2020-12-04T15:34:55Z

@lukehinds That's a good idea. I'll move it out into it's on file and have it utilized in main.rs

ashcrow · 2020-12-04T17:39:12Z

Updated

puiterwijk

Please fix the comments I pointed out. There's a few more .expect() uses.

src/crypto.rs

src/main.rs

src/revocation.rs

src/crypto.rs

src/revocation.rs

ashcrow · 2020-12-10T18:27:55Z

Updated based on @puiterwijk's review.

puiterwijk

A few more notes remaining, but starting to look decent!

src/crypto.rs

puiterwijk · 2020-12-10T18:33:02Z

src/revocation.rs

+            "Loading the revocation certificate from {}",
+            revocation_cert_path
+        );
+        match crypto::rsa_import_pubkey(revocation_cert_path.clone()) {


If rsa_import_pubkey had accepted an &str, you wouldn't have needed to .clone() the path here. That's exactly why that is more idiomatic 😄

Makes sense, thanks! I've updated the others but I'll leave this one be for now since it's in rsa_import_pubkey.

puiterwijk · 2020-12-10T18:34:27Z

src/revocation.rs

+
+        // Verify the message and signature with our key
+        let mut verified =
+            crypto::rsa_verify(cert_key.clone(), message, signature);


Is there any chance cert_key could be given as a reference too (&...)? That would save us from having to clone() every time we get a message.

We can make rsa_verify take a reference but once inside the function it seems like we need to clone for PKey::from_rsa though I may be wrong. Thoughts?

I am good with this now, @puiterwijk if @ashcrow answer satisfies you, we can merge this

Sure. Let's merge it like this and improve later.

ashcrow · 2020-12-11T17:55:20Z

Rebased

ashcrow · 2021-01-07T15:08:54Z

Will rebase shortly

ashcrow · 2021-01-07T16:01:54Z

Rebased!

ashcrow · 2021-01-18T15:13:37Z

Bump

lukehinds · 2021-01-18T15:25:42Z

hey @puiterwijk this needs approval from you as well (changes requested)

lukehinds · 2021-01-21T10:17:48Z

@puiterwijk ,please ack if the review request is resolved, so we can merge this. I am sure we will revisit the code again once then parts are rigged together.

puiterwijk

Sorry for forgetting to hit the Approve button. Thanks for the PR!

ashcrow · 2021-02-04T21:34:33Z

Bah! Needs another rebase!

Signed-off-by: Steve Milner <[email protected]>

Verify remote messages against signatures via rsa_verify Signed-off-by: Steve Milner <[email protected]>

lukehinds · 2021-02-08T09:05:15Z

hey @ashcrow , keen to approve this for you. It seems all we have now are some redundant clones (see CI 'Fedora tests'). If you could make another quick commit I would like to merge this and start rigging up the revocation actions.

Closes: keylime#92 Signed-off-by: Steve Milner <[email protected]>

Signed-off-by: Steve Milner <[email protected]>

lkatalin · 2021-02-08T14:35:02Z

Looks like this resolves #92 (hopefully this comment will link it)

ashcrow commented Oct 15, 2020

View reviewed changes

src/main.rs Outdated Show resolved Hide resolved

ashcrow force-pushed the revocation branch 3 times, most recently from 2c21930 to 8da03b7 Compare October 20, 2020 18:02

ashcrow force-pushed the revocation branch 6 times, most recently from c9dccd7 to e3a2016 Compare November 24, 2020 16:21

ashcrow force-pushed the revocation branch from e3a2016 to ff53883 Compare November 30, 2020 16:51

ashcrow requested review from lukehinds, puiterwijk and lkatalin November 30, 2020 16:51

ashcrow force-pushed the revocation branch from ff53883 to 0e81ad0 Compare November 30, 2020 16:57

ashcrow changed the title ~~WIP: Implement revocation service~~ Implement revocation service Nov 30, 2020

ashcrow changed the title ~~Implement revocation service~~ Implement revocation service from Python code Nov 30, 2020

keylime-bot assigned lkatalin, puiterwijk and lukehinds Nov 30, 2020

lkatalin approved these changes Dec 3, 2020

View reviewed changes

keylime-bot unassigned lkatalin Dec 3, 2020

ashcrow force-pushed the revocation branch from 0e81ad0 to b4388ae Compare December 4, 2020 17:28

lukehinds approved these changes Dec 8, 2020

View reviewed changes

keylime-bot unassigned lukehinds Dec 8, 2020

puiterwijk suggested changes Dec 10, 2020

View reviewed changes

ashcrow force-pushed the revocation branch from b4388ae to de4b7aa Compare December 10, 2020 18:26

puiterwijk reviewed Dec 10, 2020

View reviewed changes

ashcrow force-pushed the revocation branch 2 times, most recently from 5e91178 to 6bd2da8 Compare December 11, 2020 17:55

ashcrow requested a review from puiterwijk December 17, 2020 15:07

ashcrow force-pushed the revocation branch from 6bd2da8 to ab4a1aa Compare January 7, 2021 15:58

lukehinds approved these changes Jan 18, 2021

View reviewed changes

puiterwijk approved these changes Jan 21, 2021

View reviewed changes

keylime-bot unassigned puiterwijk Jan 21, 2021

ashcrow added 3 commits February 5, 2021 12:45

Cargo: Add zmq and update dep refs

06ee65a

Signed-off-by: Steve Milner <[email protected]>

error: Add zmq

9caabed

Signed-off-by: Steve Milner <[email protected]>

crypto: add rsa_verify

e7707f6

Verify remote messages against signatures via rsa_verify Signed-off-by: Steve Milner <[email protected]>

ashcrow force-pushed the revocation branch 2 times, most recently from 206e16e to af64084 Compare February 5, 2021 21:44

ashcrow added 3 commits February 8, 2021 09:20

revocation: Implement revocation service

24bbb86

Closes: keylime#92 Signed-off-by: Steve Milner <[email protected]>

main: Wire in revocation service

55f04ca

Signed-off-by: Steve Milner <[email protected]>

tests/Dockerfile: Add zmq dependency

460c826

Signed-off-by: Steve Milner <[email protected]>

ashcrow force-pushed the revocation branch from af64084 to 460c826 Compare February 8, 2021 14:20

lukehinds merged commit dccc791 into keylime:master Feb 8, 2021

lkatalin mentioned this pull request Mar 18, 2021

Implement HTTP server GET / POST #91

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement revocation service from Python code #134

Implement revocation service from Python code #134

ashcrow commented Oct 15, 2020 •

edited

Loading

ashcrow commented Oct 15, 2020

lkatalin left a comment

lukehinds commented Dec 4, 2020 •

edited

Loading

ashcrow commented Dec 4, 2020

ashcrow commented Dec 4, 2020

puiterwijk left a comment

ashcrow commented Dec 10, 2020

puiterwijk left a comment

puiterwijk Dec 10, 2020

ashcrow Dec 11, 2020

puiterwijk Dec 10, 2020

ashcrow Dec 10, 2020

ashcrow Jan 6, 2021

lukehinds Jan 7, 2021

puiterwijk Jan 7, 2021

ashcrow commented Dec 11, 2020

ashcrow commented Jan 7, 2021

ashcrow commented Jan 7, 2021

ashcrow commented Jan 18, 2021

lukehinds commented Jan 18, 2021

lukehinds commented Jan 21, 2021

puiterwijk left a comment

ashcrow commented Feb 4, 2021

lukehinds commented Feb 8, 2021 •

edited

Loading

lkatalin commented Feb 8, 2021

Implement revocation service from Python code #134

Implement revocation service from Python code #134

Conversation

ashcrow commented Oct 15, 2020 • edited Loading

ashcrow commented Oct 15, 2020

lkatalin left a comment

Choose a reason for hiding this comment

lukehinds commented Dec 4, 2020 • edited Loading

ashcrow commented Dec 4, 2020

ashcrow commented Dec 4, 2020

puiterwijk left a comment

Choose a reason for hiding this comment

ashcrow commented Dec 10, 2020

puiterwijk left a comment

Choose a reason for hiding this comment

puiterwijk Dec 10, 2020

Choose a reason for hiding this comment

ashcrow Dec 11, 2020

Choose a reason for hiding this comment

puiterwijk Dec 10, 2020

Choose a reason for hiding this comment

ashcrow Dec 10, 2020

Choose a reason for hiding this comment

ashcrow Jan 6, 2021

Choose a reason for hiding this comment

lukehinds Jan 7, 2021

Choose a reason for hiding this comment

puiterwijk Jan 7, 2021

Choose a reason for hiding this comment

ashcrow commented Dec 11, 2020

ashcrow commented Jan 7, 2021

ashcrow commented Jan 7, 2021

ashcrow commented Jan 18, 2021

lukehinds commented Jan 18, 2021

lukehinds commented Jan 21, 2021

puiterwijk left a comment

Choose a reason for hiding this comment

ashcrow commented Feb 4, 2021

lukehinds commented Feb 8, 2021 • edited Loading

lkatalin commented Feb 8, 2021

ashcrow commented Oct 15, 2020 •

edited

Loading

lukehinds commented Dec 4, 2020 •

edited

Loading

lukehinds commented Feb 8, 2021 •

edited

Loading