Initial implementation of the library #1

maxlambrecht · 2021-06-06T13:52:23Z

Initial commit defining and implementing the basic interface and SPIFFE types:

workload.proto and protobuf generated code
Workload API client with one-shot call methods
Certificate and PrivateKey types
X.509 SVID and bundle types
JWT SVID and bundle types
TrustDomain and SpiffeId types

Signed-off-by: Max Lambrecht [email protected]

…FE types: - Workload API client with one-shot call methods - Certificate and PrivateKey types - X.509 SVID and bundle types - JWT SVID and bundle types - TrustDomain and SpiffeId types Signed-off-by: Max Lambrecht <[email protected]>

Signed-off-by: Max Lambrecht <[email protected]>

…ode and enum variants closer to where they belong. Signed-off-by: Max Lambrecht <[email protected]>

Signed-off-by: Max Lambrecht <[email protected]>

hug-dev

This is impressive work 👏 The amount of tests is really nice. I did not look at it in all details as I think this is pretty good for a first baseline release and provides the basis to use it correctly. Even more than that! We would be happy to try that with the Parsec project.

My knowledge of SPIFFE is also not too good for a thorough review but if you wanted I could take some time to analyse it properly.

hug-dev · 2021-06-14T16:11:53Z

README.md

+let spiffe_id = SpiffeId::try_from("spiffe://example.org/my-service")?;
+
+// fetch a jwt token for the provided SPIFFE-ID and with the target audience `service1.com`
+let jwt_token = client.fetch_jwt_token(&["audience1", "audience2"], Some(&spiffe_id))?;


Maybe my SPIFFE knowledge is a bit rusted by now but why is the SPIFFE ID needed as a parameter here? Could it be the case that the workload does not know its SPIFFE ID if its set by another entity during registration?

edit: ah I see, this is an Option in case you want the sub claim to be populated. In our case, we mainly use the fetch method for the workload to know which is its SPIFFE ID

The usual case would be to just pass None in the SPIFFE ID parameter and the Workload API will return a JWT token with the 'sub' claim set with the SPIFFE ID corresponding to the default identity (the first one).

If the workload has multiple identities mapped to it, there is the possibility to select which SPIFFE ID to request the token for.

In case it's requested a token with a SPIFFE ID that the workload it's not mapped with, the Workload API will return an empty response.

Makes sense?

That makes sense! Thank you for the explanation 👍

hug-dev · 2021-06-14T16:17:45Z

src/proto/workload.proto

@@ -0,0 +1,152 @@
+syntax = "proto3";


Maybe instead of having a copy of the Workload API we could have spiffe/spiffe repo as a submodule and use spiffe/main/standards/workloadapi.proto directly!

Not so sure about this, I'm not a big fan of git submodules 😁 I'd leave as it is and put more thinking on this later on.

hug-dev · 2021-06-14T16:18:39Z

src/proto/workload.rs

@@ -0,0 +1,2283 @@
+// This file is generated by rust-protobuf 2.23.0. Do not edit


Since we generate it here, maybe it's not needed to have a build file? Would save some compilation time. We can re-generate it manually when the Workload API gets updated.

I removed the build file.

hug-dev · 2021-06-14T16:23:35Z

src/spiffe_id/mod.rs

+    /// assert_eq!("example.org", spiffe_id.trust_domain().to_string());
+    /// assert_eq!("/path", spiffe_id.path());
+    /// ```
+    pub fn new(id: &str) -> Result<Self, SpiffeIdError> {


Thanks for adding the parsing and the validation of it as well 🙏

hug-dev · 2021-06-14T16:42:50Z

src/workload_api/client.rs

+            None => Err(ClientError::EmptyResponse),
+        }
+    }
+}


Would be very nice to have an abstraction for the ValidateJWTSVID call as well but I am happy to add it once this is merged 👌

hug-dev

forgot a few

hug-dev · 2021-06-15T07:51:21Z

Cargo.toml

@@ -0,0 +1,37 @@
+[package]
+name = "rust-spiffe"


The spiffe name is free on crates.io so I guess this could just be "spiffe", when it will be used it will always be in a Rust context :)

Good idea, it makes total sense!

hug-dev · 2021-06-15T07:58:42Z

Cargo.toml

+description = "Rust client library implementation for SPIFFE"
+
+[dependencies]
+grpcio = { version = "0.6", default-features = false, features = ["protobuf-codec"] }


Do you want to bump that to at least 0.8 in order to include tikv/grpc-rs#499? That would boost compilation time and also help us in Parsec 👍

Sure! Bumped it to 0.9. For some reason it needs grpcio-sys = "0.9.0" specified as build-dependency, otherwise the build throws an error.

Bump 'grpcio' dependency to 0.9. Signed-off-by: Max Lambrecht <[email protected]>

Signed-off-by: Max Lambrecht <[email protected]>

hug-dev

Thanks 🌟 For me this is good enough to get started, make a first release so that we can try it and improve it incrementally!

maxlambrecht · 2021-06-15T16:26:14Z

Thanks 🌟 For me this is good enough to get started, make a first release so that we can try it and improve it incrementally!

Great! Thanks for your review, I really appreciate it! I'll merge this and publish the first release.

maxlambrecht added 3 commits June 6, 2021 10:48

Add line to README and missing new lines.

665e8c0

Signed-off-by: Max Lambrecht <[email protected]>

Minor change in README and docs.

13d1fdd

Signed-off-by: Max Lambrecht <[email protected]>

maxlambrecht requested a review from hug-dev June 6, 2021 18:57

maxlambrecht added 3 commits June 6, 2021 16:28

Remove unused dependency.

fc9f277

Signed-off-by: Max Lambrecht <[email protected]>

Refactor Certificate and X509Svid errors and validations moving the c…

1c95485

…ode and enum variants closer to where they belong. Signed-off-by: Max Lambrecht <[email protected]>

Some error and trait refactors.

64e3a8c

Signed-off-by: Max Lambrecht <[email protected]>

hug-dev approved these changes Jun 14, 2021

View reviewed changes

hug-dev reviewed Jun 15, 2021

View reviewed changes

maxlambrecht added 2 commits June 15, 2021 10:20

Rename crate to 'spiffe'.

7a7be00

Bump 'grpcio' dependency to 0.9. Signed-off-by: Max Lambrecht <[email protected]>

Remove 'build.rs'.

f31b7f1

Signed-off-by: Max Lambrecht <[email protected]>

hug-dev approved these changes Jun 15, 2021

View reviewed changes

maxlambrecht merged commit dabc35e into main Jun 15, 2021

maxlambrecht deleted the pre/0.1.0 branch August 12, 2023 20:26

		@@ -0,0 +1,2283 @@
		// This file is generated by rust-protobuf 2.23.0. Do not edit

Initial implementation of the library #1

Initial implementation of the library #1

Uh oh!

Conversation

maxlambrecht commented Jun 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hug-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxlambrecht Jun 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hug-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hug-dev left a comment

Choose a reason for hiding this comment

Uh oh!

maxlambrecht commented Jun 15, 2021

Uh oh!

Uh oh!

maxlambrecht commented Jun 6, 2021 •

edited

Loading

maxlambrecht Jun 14, 2021 •

edited

Loading