Implement changes for registrar call #158

lkatalin · 2021-02-04T00:28:18Z

This PR adds relevant prerequisites and then uses create_ek() to retrieve the EK cert and pubkey and adds functions to retrieve the AK pubkey as well. It sends all of this data to the registrar. Much of it was written or co-written by @lukehinds so I have included him on those commits. Supersedes #149 .

Fixes #119
Fixes #95

~~@lukehinds @puiterwijk I'm especially interested in making sure the create_ak() function is correct.~~

Currently needs to be rebased on Update error handling for revocation scripts #162 to pass CI.
TODO: I can also add unit testing with reqwest_mock.

src/main.rs

ashcrow · 2021-02-04T21:28:43Z

This looks good from my POV!

puiterwijk

Unfortunately, the types are somewhat vague, so we'll need some fixes :(.
I'll send you my fixes so you can use that.

puiterwijk · 2021-02-08T09:08:26Z

src/registrar_agent.rs

+
+#[derive(Debug, Serialize, Deserialize)]
+pub struct Register<'a> {
+    ek: &'a [u8],


This is an str, since it's PEM encoded

puiterwijk · 2021-02-08T09:08:36Z

src/registrar_agent.rs

+    ekcert: &'a [u8],
+    #[serde(serialize_with = "serialize_as_base64")]
+    ek_tpm: &'a [u8],
+    aik: &'a [u8],


This is &str as well.

puiterwijk · 2021-02-08T09:09:18Z

src/registrar_agent.rs

+        .json(&data)
+        .send()
+        .await?
+        .json()


This will fail if the server returned an error, because then code is non-200 and status is not success, and it won't have a results block.

I moved the status code check to be before this .json() is called and just used the built-in reqwest::StatusCode method.

lkatalin · 2021-02-25T17:14:27Z

@puiterwijk Let me know when you have a good idea of the changes you have / want on this one in addition to your comments above.

lkatalin · 2021-03-03T01:04:24Z

src/registrar_agent.rs

+    let resp = reqwest::Client::new().put(&addr).json(&data).send().await?;
+
+    if !resp.status().is_success() {
+        return Err(anyhow!(


Maybe it makes sense to use one of our custom error types here instead of anyhow? I could see a case for either.

I ended up using a custom type so that we can check the status codes.

lkatalin · 2021-03-03T01:04:56Z

src/main.rs

+        config_get("/etc/keylime.conf", "cloud_agent", "agent_uuid")?;
+    let agent_uuid = get_uuid(&agent_uuid_config);
+
+    {


@puiterwijk What is the purpose of this bracket? I believe this is from your changes.

that does not look like a strange scope, is the compiler not complaining about this?

So, that was a bracket I put in place to make the scope for keyblob smaller. That means that outside of the closing bracket, the keyblob is no longer valid.
Given that the Rust compiler calls drop as soon as something goes out of scope, that means that as soon as the closing bracket is hit, the keyblob is erased from memory (since it's Zeroize).

This was basically because this activation step is in the main() function, which keeps running until the entire agent terminates.
And I didn't want to have secrets lingering in memory during the full runtime.

That makes sense! Good thinking.

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>, Patrick Uiterwijk <[email protected]>

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Patrick Uiterwijk <[email protected]>

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>, Patrick Uiterwijk <[email protected]>

lkatalin · 2021-03-04T22:09:23Z

I've added some unit testing for do_register_agent and do_activate_agent. I ended up using wiremock for this, as it's better documented than reqwest_mock and seems to have more features (not just a client, etc.).

lukehinds · 2021-03-09T10:38:09Z

src/main.rs

+        config_get("/etc/keylime.conf", "cloud_agent", "agent_uuid")?;
+    let agent_uuid = get_uuid(&agent_uuid_config);
+
+    {


that does not look like a strange scope, is the compiler not complaining about this?

puiterwijk · 2021-03-09T16:00:51Z

src/main.rs

+            &auth_tag,
+        )
+        .await?;
+    }


This is the closing bracket: after we activate, we no longer need keyblob.

Yeah, I did notice it created its own scope that lasted until this line, I just wasn't sure why.

puiterwijk

LGTM. Thanks!

keylime-bot requested review from puiterwijk, lukehinds and ashcrow February 4, 2021 00:28

lkatalin force-pushed the registrar branch 2 times, most recently from 2c70672 to d7e990b Compare February 4, 2021 00:43

keylime-bot assigned puiterwijk, lukehinds and ashcrow Feb 4, 2021

ashcrow reviewed Feb 4, 2021

View reviewed changes

src/main.rs Show resolved Hide resolved

lkatalin force-pushed the registrar branch 5 times, most recently from 3fc62de to 152c848 Compare February 4, 2021 19:12

lkatalin requested a review from ashcrow February 4, 2021 19:28

lkatalin marked this pull request as ready for review February 4, 2021 19:29

puiterwijk suggested changes Feb 8, 2021

View reviewed changes

keylime-bot assigned lkatalin and unassigned ashcrow, puiterwijk and lukehinds Feb 8, 2021

lkatalin marked this pull request as draft February 10, 2021 22:33

lkatalin marked this pull request as ready for review February 25, 2021 17:12

lkatalin force-pushed the registrar branch 2 times, most recently from b914ab6 to 01e6c2a Compare February 26, 2021 01:15

lkatalin requested a review from puiterwijk February 26, 2021 01:31

keylime-bot assigned ashcrow and unassigned lkatalin Feb 26, 2021

keylime-bot assigned puiterwijk and lukehinds Feb 26, 2021

lkatalin force-pushed the registrar branch 2 times, most recently from 2f81d37 to 5fa2f9a Compare March 3, 2021 01:00

lkatalin commented Mar 3, 2021

View reviewed changes

lkatalin force-pushed the registrar branch from 5fa2f9a to 50acbf8 Compare March 3, 2021 15:26

ashcrow mentioned this pull request Mar 3, 2021

Apache license header to source files #163

Merged

lkatalin force-pushed the registrar branch from 50acbf8 to f54dcb9 Compare March 4, 2021 20:41

lkatalin and others added 5 commits March 4, 2021 16:59

Add Error types: Reqwest, Registrar, and UUID

74b50a1

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>

Add registrar_agent

6bf3718

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>, Patrick Uiterwijk <[email protected]>

Add get_uuid()

a3b7b90

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>

Add TPM functionality

a6b2979

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Patrick Uiterwijk <[email protected]>

Add send data to registrar, receive key blob, activate agent

30f0c54

Signed-off-by: Lily Sturmann <[email protected]> Co-authored-by: Luke Hinds <[email protected]>, Patrick Uiterwijk <[email protected]>

lkatalin force-pushed the registrar branch from f54dcb9 to 30f0c54 Compare March 4, 2021 22:04

lukehinds approved these changes Mar 9, 2021

View reviewed changes

keylime-bot unassigned lukehinds Mar 9, 2021

puiterwijk reviewed Mar 9, 2021

View reviewed changes

lkatalin requested a review from puiterwijk March 9, 2021 16:03

puiterwijk approved these changes Mar 9, 2021

View reviewed changes

lkatalin merged commit 23cd0d0 into keylime:master Mar 9, 2021

lkatalin mentioned this pull request Mar 18, 2021

Implement HTTP server GET / POST #91

Closed

lkatalin deleted the registrar branch July 16, 2021 17:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement changes for registrar call #158

Implement changes for registrar call #158

lkatalin commented Feb 4, 2021 •

edited

Loading

ashcrow commented Feb 4, 2021

puiterwijk left a comment

puiterwijk Feb 8, 2021

puiterwijk Feb 8, 2021

puiterwijk Feb 8, 2021

lkatalin Mar 3, 2021

lkatalin commented Feb 25, 2021

lkatalin Mar 3, 2021

lkatalin Mar 4, 2021

lkatalin Mar 3, 2021

lukehinds Mar 9, 2021

puiterwijk Mar 9, 2021

puiterwijk Mar 9, 2021

lkatalin Mar 9, 2021

lkatalin commented Mar 4, 2021 •

edited

Loading

lukehinds Mar 9, 2021

puiterwijk Mar 9, 2021

lkatalin Mar 9, 2021

puiterwijk left a comment

Implement changes for registrar call #158

Implement changes for registrar call #158

Conversation

lkatalin commented Feb 4, 2021 • edited Loading

ashcrow commented Feb 4, 2021

puiterwijk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lkatalin commented Feb 25, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lkatalin commented Mar 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puiterwijk left a comment

Choose a reason for hiding this comment

lkatalin commented Feb 4, 2021 •

edited

Loading

lkatalin commented Mar 4, 2021 •

edited

Loading