theupdateframework / rust-tuf

Rust implementation of The Update Framework (TUF)
https://crates.io/crates/tuf
Apache License 2.0
174 stars 32 forks source link

Minimal working example #93

Closed nbigaouette-eai closed 7 years ago

nbigaouette-eai commented 7 years ago

I'm looking into tuf/rust-tuf for some update procedures.

For now I'm failing to create a minimal working example and I'm not sure if it's just me or if there is a bug in the documentation that needs fixing. rust-tuf's src/lib.rs contains an example with a function get_original_root() that simply is unimplemented!(). I'm trying to fill the gap here.

First, I have followed the TUF documentation to generate a directory structure to be served according to the documentation in https://github.com/theupdateframework/tuf/blob/develop/tuf/README.md. In a nutshell, the steps are:

  1. Generate RSA keys (root, targets, snapshot, timestamp);
  2. Create repository metadata;
  3. Create the actual repository.

Step 1. is trivial, simply calling tuf.repository_tool.generate_and_write_rsa_keypair() for each key. Step 2. uses create_new_repository() and adds the keys to the repo. Step 3. walks the repository directory and signs files using add_targets().

The problem I am facing comes from the call to rust-tuf's tuf::interchange::JsonDataInterchange::from_reader(). Since the get_original_root() function in unimplemented!(), I had to adapt it. It's returning a File, so I let it return File::open("repository/metadata/root.json").unwrap() where the json file was created by step 3 above. The from_reader() call fails though with a deserialization error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Encoding("JSON: ErrorImpl { code: Message(\"missing field `key_id`\"), line: 7, column: 3 }")', src/libcore/result.rs:859

I checked the files generated by tuf.repository_tool, and none contains the string key_id. The rust-tuf struct Signature seems to have one such field and is Deserialize so maybe this is where the error comes from?

I might be loading the improper file in tuf::interchange::JsonDataInterchange::from_reader(), but then there is no file that contains the key_id field in tuf's generated files. Am I following the proper guide to initialize a repository (https://github.com/theupdateframework/tuf/blob/develop/tuf/README.md)?

Note that I'm using rust-tuf c3321bf1295be66df46dcfb0440df491f2cf583f (branch develop) and tuf 904fa9b8df8ab8c632a210a2b05fd741e366788a (branch develop).

Thanks!

heartsucker commented 7 years ago

Hi hi @nbigaouette-eai.

The first problem with that minimal working example is that rust-tuf can't properly sign things yet. :)

Once that's done, I can clean up the lib.rs documentation to be a bit more clear. There's really two halves of this: making a root if you're doing the server side, and fetching a root if you're doing the client. This was meant to show the basic client update flow.

I'd also not use this against Python example. If you dig through the issues regarding the development of the spec, you'll see that the spec is overly restrictive on field names, formats, and other things not directly related to the security model. This will be changing to specify more the what than the how.

E.g., https://github.com/theupdateframework/tuf/issues/457

Also, Uptane, a TUF extension, only defines suggested ASN.1 modules for the metadata with the assumption that you'd implemented it how you want.

So as a result, I have taken some liberties with the data format like renaming the field _type to type because that extra underscore is an artifact of it being a common function in Python (or I suppose that's why they chose that name). Also, I used snake case eveywhere for consistency, so key_id replaces Python's keyid.

I will keep this issue open until I can get the docs more up to date with a full cycle of creating metadata, storing it, fetching, verifying, downloading, yada yada.

heartsucker commented 7 years ago

I have slightly updated the docs to use the library as it's intended in e8da3ea. This still only addresses the client side.

nbigaouette-eai commented 7 years ago

Great thanks for the hints. I'll continue my investigation.

Note that the example unfortunately does not run, probably due to

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Opaque("Hyper: Io(Error { repr: Custom(Custom { kind: InvalidInput, error: StringError(\"Invalid scheme for Http\") }) })")', src/libcore/result.rs:859

I had to add a TLS implementation (hyper-native-tls = "0.2") and using this:

extern crate hyper_native_tls;
...
    let ssl = hyper_native_tls::NativeTlsClient::new().unwrap();
    let connector = hyper::net::HttpsConnector::new(ssl);

    let mut remote = HttpRepository::new(
        Url::parse("https://static.rust-lang.org/").unwrap(),
        HttpClient::with_connector(connector),
        Some("rustup/1.4.0".into()));
...

I still get a panic, but at least it's further down the road:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: NotFound', src/libcore/result.rs:859
heartsucker commented 7 years ago

Ah, well that would be because there is no https://static.rust-lang.org/root.json yet. I guess I should have put in something like example.com. Ha! You'll have to self host for a bit. Getting that up on to static.rust-lang.org is probably a couple of months out. I'm only doing the rust/rustup/tuf stuff in my spare time.

heartsucker commented 7 years ago

Also the line Some("rustup/1.4.0".into()) is a prefix for the HTTP User-Agent header. You should put something like Some(format!("my_app/{}", env!("CARGO_PKG_VERSION"))). This generates a User-Agent string like my_app/1.2.3 (rust-tuf/0.2.0).

nbigaouette-eai commented 7 years ago

About the root.json: makes sense that it's not implemented yet for the official repo ;) I do plan to self host; it is meant for an internal tool.

Thanks about the info about the header, I'll adapt it for my use case.

When you say that rust-tuf can't sign yet, do you mean that it cannot be used to create the metadata for a repository or that it cannot be used to validate some downloads? I don't mind using the Python reference implementation to manage the repository for now, but I would like to deploy an updater in Rust... Having a full blown Rust implementation would obviously be a total win ;)

Regarding the field names, I still cannot Deserialize the root.json created by the (Python) tuf (v0.10.0-660-g904fa9b on branch develop). As you mentioned, some fields have a different name than what is present in the rust structs. I guess upstream is still not decided on the exact format, am I right? As such, could rust-tuf tell serde to rename the fields? That way the json can at least be attempted to be deserialized...

Here's a simple patch doing so:

diff --git a/src/crypto.rs b/src/crypto.rs
index 5ac145d..f2132c3 100644
--- a/src/crypto.rs
+++ b/src/crypto.rs
@@ -463,8 +463,11 @@ pub enum KeyFormat {
 /// A structure that contains a `Signature` and associated data for verifying it.
 #[derive(Debug, Serialize, Deserialize)]
 pub struct Signature {
+    #[serde(rename="keyid")]
     key_id: KeyId,
+    #[serde(rename="method")]
     scheme: SignatureScheme,
+    #[serde(rename="sig")]
     signature: SignatureValue,
 }

diff --git a/src/metadata.rs b/src/metadata.rs
index ffc1c62..3cbd384 100644
--- a/src/metadata.rs
+++ b/src/metadata.rs
@@ -236,8 +236,11 @@ where
 {
     signatures: Vec<Signature>,
     signed: D::RawData,
+    #[serde(skip_deserializing)]
     _interchage: PhantomData<D>,
+    #[serde(skip_deserializing)]
     _metadata: PhantomData<M>,
+    #[serde(skip_deserializing)]
     _verification: PhantomData<V>,
 }

Note that this also skip deserialization for the phantom data which should not appear in the (compiled) struct and/or the json file (it's some phantom data after all! ;) ).

Also, is there a reason why SignatureSheme uses FromStr trait instead of Deserialize from serde? I had to tell its from_str() method to accept the value found in root.json:

diff --git a/src/crypto.rs b/src/crypto.rs
index 5ac145d..b1fc4e6 100644
--- a/src/crypto.rs
+++ b/src/crypto.rs
@@ -128,6 +128,7 @@ impl FromStr for SignatureScheme {
     fn from_str(s: &str) -> ::std::result::Result<Self, Self::Err> {
         match s {
             "ed25519" => Ok(SignatureScheme::Ed25519),
+            "RSASSA-PSS" => Ok(SignatureScheme::RsaSsaPssSha512),
             "rsassa-pss-sha256" => Ok(SignatureScheme::RsaSsaPssSha256),
             "rsassa-pss-sha512" => Ok(SignatureScheme::RsaSsaPssSha512),
             typ => Err(Error::Encoding(typ.into())),

I'm definitely not sure this is valid, but at least the execution can continue ;)

After those modifications, the example continues, but still fail at the line:

let tuf = Tuf::<JsonDataInterchange>::from_root_pinned(root, &key_ids).unwrap();

with the error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Encoding("JSON: ErrorImpl { code: Message(\"missing field `type`\"), line: 0, column: 0 }")', src/libcore/result.rs:859
heartsucker commented 7 years ago

When you say that rust-tuf can't sign yet

There's a Signer struct, but some of the ser/de operations on public keys is borked at the moment. DER problems, I think.

I guess upstream is still not decided on the exact format, am I right?

Not quite. Because as the spec says, TUF is not a universal updater, there isn't any assumption that two TUF implementation need to be able to talk coherently with each other, especially since they are removing the restriction that the data interchange format is JSON.

Note that this also skip deserialization for the phantom data which should not appear in the (compiled) struct and/or the json file (it's some phantom data after all! ;) ).

I haven't written tests for the ser/de functions either, but I think some of that was covered in shims.rs. If it wasn't, can you open another ticket for that issue specifically so I don't forget?

Also, is there a reason why SignatureSheme uses FromStr trait instead of Deserialize from serde?

SignatureScheme implements both Serialize and Deserialize, and it uses ToString and FromString to perform them.

"RSASSA-PSS" => Ok(SignatureScheme::RsaSsaPssSha512),

I'm pretty sure the Python implementation does RSASSA-PSS over SHA256, not SHA512.

Message(\"missing field type\")

This is because you need to do a #[serde(rename = "_type")] on the fields typ in the signed portion of the metadata. That also might be keytype too, which I just called type because it's obviously a key.

heartsucker commented 7 years ago

There's another problem too. The Python implementation calculates key IDs as sha256(canonical_json(json_string)) and I do sha256(spki_der_bytes), so you'd have to disable the checks on key ID too.

With the amount of drift you'd have to do to get rust-tuf to work with the Python reference implementation, it'd probably be better for you to write some test cases that address the issues you've found and maybe even try to fix some of the bugs.

heartsucker commented 7 years ago

@nbigaouette-eai just letting you know that signing and verifying seems to be working on my small number of test cases. If you find any issues while playing around with this, please file issues. :)

nbigaouette-eai commented 7 years ago

There's a Signer...

I couldn't find it... at least in the develop branch.

[...] TUF is not a universal updater,

Not sure what you meant, so had to dig it up a bit. Here's the link to section 1.4. Non-goals of the specs. I believe this means that TUF is not meant to be the "updater" of the system but rather just a way to validate the software, which I think if just fine. I can write my own updater but I want to use TUF to validate the trust chain.

[...] there isn't any assumption that two TUF implementation need to be able to talk coherently with each other, especially since they are removing the restriction that the data interchange format is JSON.

Wait... what?! That's sad! I would have though the specs would define the file formats properly so that implementations could talk to each other... So while the reference implementation is in Python, I can use a great Rust implementation in my project! ;) After having listened to Justin Cappos's interview on PODCAST.INIT I though it was great that their was already multiple implementations...

So if they can't all talk to each other, I guess the same lib should be used for both the repository signing and the download validation by the client, right?

[...] but I think some of that was covered in shims.rs.

That file doesn't contain any test...

[...] can you open another ticket for that issue specifically so I don't forget?

I can open an issue for missing tests on Serialize/Deserialize traits if you want... Let me know if that is not what you meant before I start opening useless issues ;)

SignatureScheme implements both Serialize and Deserialize, and it uses ToString and FromString to perform them.

I've opened #102 regarding this.

I'll see what could be done about the serde rename attributes. I have a couple here that might be required. But before submitting them, I want to have basic functionality so I know what I'm doing is not total crap!

Because the Python reference implementation's documentation is kind of hard to follow, I've tried the go one. Its simple example is a lot simpler than the Python one.

With go-tuf, the root.json looks like this:

{"signed":{"_type":"Root","consistent_snapshot":true,"expires":"2018-07-05T20:24:43Z","keys":{"6551df4f3837b9312e88c459b1e77d40398509ba7d214253f92a4e17eb947c27":{"keytype":"ed25519","keyval":{"public":"c0cfa553f2bf974496c73deab2a5a5ad428c52c4b3534b915c8a336a4b3f4bfc"}},"72558c26bbbd89610b8a3dc15a3aee10f10b370df723bc7752b9760ec8d8e90c":{"keytype":"ed25519","keyval":{"public":"257245370ba831440eeabb7db253d2afa440e8cacb2a251ca304b5e601fd9d7d"}},"96fd5e97b0a2f912b4b8b78c3da45c98af5e2458d92db1429af6b697a745648e":{"keytype":"ed25519","keyval":{"public":"97a0f2df25914cba22d5ff70ff4cfc27ef934121f3c1ce8ca153f2cd6c4aa225"}},"c3b9005a53967778fc6a4a24bac23f87333799ab07b7a9e75f8008fd5fda7f15":{"keytype":"ed25519","keyval":{"public":"14dce612e576edc41e50d4db99f52c4b2b843c1daf797d602b6b65c1e41906e0"}}},"roles":{"root":{"keyids":["6551df4f3837b9312e88c459b1e77d40398509ba7d214253f92a4e17eb947c27"],"threshold":1},"snapshot":{"keyids":["96fd5e97b0a2f912b4b8b78c3da45c98af5e2458d92db1429af6b697a745648e"],"threshold":1},"targets":{"keyids":["72558c26bbbd89610b8a3dc15a3aee10f10b370df723bc7752b9760ec8d8e90c"],"threshold":1},"timestamp":{"keyids":["c3b9005a53967778fc6a4a24bac23f87333799ab07b7a9e75f8008fd5fda7f15"],"threshold":1}},"version":4},"signatures":[{"keyid":"6551df4f3837b9312e88c459b1e77d40398509ba7d214253f92a4e17eb947c27","method":"ed25519","sig":"64873aad168bcb63047487ec1e58130a70cd56c05b478695f72f421491d83ca8bd2f50060b43c19b0c475c9bd22569ddc6580d816ee5318a668be28368f63805"}]}

I still get an error with rust-tuf:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Encoding("JSON: ErrorImpl { code: Message(\"invalid type: map, expected a string\"), line: 0, column: 0 }")', src/libcore/result.rs:859

when calling this line:

let tuf = Tuf::<JsonDataInterchange>::from_root_pinned(root, &key_ids).unwrap();

I wish I could have a minimal working rust code to boostrap the process... :S

signing and verifying seems to be working on my small number of test cases.

That's great! So does that mean that at some point the repository files could be signed using rust-tuf to create all the metadata and that a rust client updater could be used to download the updates and validate them using rust-tuf?

heartsucker commented 7 years ago

I couldn't find it... at least in the develop branch.

Yeah, totally forgot I pulled that out in the rebase. Rushed post. Look at metadata::SignedMetadata::new and crypto::PrivateKey.

Wait... what?! That's sad!

Yes and no. At my work we're trying to switch this to DER because embedded devices can't handle the JSON as easily. Every byte counts, and DER ends up being smaller. This is also a framework which means it can be used flexibly. Even with something much more tightly spec'd, a modern webserver can't talk to an old phone, etc.

I you dislike this, you could open an upstream issue.

So if they can't all talk to each other, I guess the same lib should be used for both the repository signing and the download validation by the client, right?

Yes. That was the intention with this. Both halves would use this lib.

That file doesn't contain any test...

I mean covered as is implemented. Ser/de tests are coming with ticket #100.

I still get an error with rust-tuf:

I think that might be the public key. Python (and presumably Go) store keys like:

{
   "keytype": "ed25519 | rsa",
   "keyval": {
    "public": "hex public point | PEM encoded DER key (PKCS#1 or SPKI)"
  }
}

I store it something like this (but it's not finalized yet):

{
  "type": "ed25519 | rsa",
  "value": "PEM encded DER SPKI"
}

The only reason i include the type field (which is redundant with the DER OID) and not just using a s JSON string only is to support this future addition to TUF: https://github.com/theupdateframework/taps/issues/38

I wish I could have a minimal working rust code to boostrap the process... :S

Yeah, yeah. I'm working on it. :)

So does that mean that at some point the repository files could be signed using rust-tuf to create all the metadata and that a rust client updater could be used to download the updates and validate them using rust-tuf?

Yes. That is exactly the idea.

heartsucker commented 7 years ago

I couldn't find it... at least in the develop branch.

Yeah, totally forgot I pulled that out in the rebase. Rushed post. Look at metadata::SignedMetadata::new and crypto::PrivateKey.

Wait... what?! That's sad!

Yes and no. At my work we're trying to switch this to DER because embedded devices can't handle the JSON as easily. Every byte counts, and DER ends up being smaller. This is also a framework which means it can be used flexibly. Even with something much more tightly spec'd like TLS, a modern webserver can't talk to an old phone, etc.

If you dislike this, you could open an upstream issue.

So if they can't all talk to each other, I guess the same lib should be used for both the repository signing and the download validation by the client, right?

Yes. That was the intention with this. Both halves would use this lib.

That file doesn't contain any test...

I mean covered as is implemented. Ser/de tests are coming with ticket #100.

I still get an error with rust-tuf:

I think that might be the public key. Python (and presumably Go) store keys like:

{
   "keytype": "ed25519 | rsa",
   "keyval": {
    "public": "hex public point | PEM encoded DER key (PKCS#1 or SPKI)"
  }
}

I store it something like this (but it's not finalized yet):

{
  "type": "ed25519 | rsa",
  "value": "PEM encded DER SPKI"
}

The only reason i include the type field (which is redundant with the DER OID) and not just using a s JSON string only is to support this future addition to TUF: https://github.com/theupdateframework/taps/issues/38

I wish I could have a minimal working rust code to boostrap the process... :S

Yeah, yeah. I'm working on it. :)

So does that mean that at some point the repository files could be signed using rust-tuf to create all the metadata and that a rust client updater could be used to download the updates and validate them using rust-tuf?

Yes. That is exactly the idea.