Skip to content

Add documentation related to the JWT-SVID auth #75

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

hug-dev
Copy link
Member

@hug-dev hug-dev commented Nov 6, 2020

Updates the authenticator sections and the thread model.

Signed-off-by: Hugues de Valon [email protected]

@hug-dev hug-dev requested a review from ionut-arm as a code owner November 6, 2020 16:56
@hug-dev hug-dev linked an issue Nov 6, 2020 that may be closed by this pull request
@hug-dev
Copy link
Member Author

hug-dev commented Nov 6, 2020

O-14 can be deleted when we decide to do local validation of JWT-SVID tokens by fetching the trust bundle (the SPIFFE public keys) through the Bundle Endpoint which is authenticated.
It was agreed that we would do that in a second phase, and that I open a GitHub issue for that problem when the PRs are merged.

@ionut-arm
Copy link
Member

Hmm, I have one question related to the diagram - do we not consider the Spiffe implementation to be trusted like the generic "Identity Provider"? Is it not possible to combine the two within the diagram and threat model, and just mark the ones that are Spiffe or "Generic" related with their respective codes?

@hug-dev
Copy link
Member Author

hug-dev commented Nov 9, 2020

do we not consider the Spiffe implementation to be trusted like the generic "Identity Provider"? Is it not possible to combine the two within the diagram and threat model, and just mark the ones that are Spiffe or "Generic" related with their respective codes?

Yes good question, I thought about that as well. I think I preferred put them apart because ideally we should not make any assumption about the SPIFFE implementation, we should not have to trust it.

This is kind of a "temporary" solution where the admin has to trust the SPIFFE Workload API Endpoint but in the future it might be possible to not trust it completely if the validation happens locally.

@ionut-arm
Copy link
Member

ionut-arm commented Nov 9, 2020

we should not have to trust it.

I mean, we have to trust anything that's an identity provider, otherwise the model doesn't make sense. For example, if the Spiffe implementation is malicious, there is literally nothing that you can do (not as things stand) to prevent abuse by some clients.

That being said, maybe we need to create a new yellow "trusted" box with "OS" inside. We deffo need that to be trusted.

@hug-dev
Copy link
Member Author

hug-dev commented Nov 9, 2020

For example, if the Spiffe implementation is malicious, there is literally nothing that you can do (not as things stand) to prevent abuse by some clients.

Yes, in the current implementation I agree. However, the SPIFFE public keys can be fetched using another special endpoint which is authenticated and backed up by CA (as far as I know). So you still have to trust something but it is something commonly accepted

@ionut-arm
Copy link
Member

Ah, that's fair!

Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice stuff, just some nitpicky comments below 🤓

@@ -242,7 +245,7 @@ following diagram, the bytes go left to right from least significant to most sig
| Session handle | Common | 8 | Session identifier. |
| Content type | Common | 1 | Defines how the message body should be processed. The only currently-supported value is `0x01`, which indicates that the message body should be treated as a serialized protobuf message. |
| Accept type | Requests only | 1 | Defines how the service should provide its response. The only currently-supported value is `0x01`, which indicates that the service should provide a response whose body is a serialized protobuf message. |
| Auth type | Requests only | 1 | Defines how the authentication bytes should be interpreted. |
| Auth type | Requests only | 1 | Defines how the authentication bytes should be interpreted. See the [section](#authentication) above. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| Auth type | Requests only | 1 | Defines how the authentication bytes should be interpreted. See the [section](#authentication) above. |
| Auth type | Requests only | 1 | Defines how the authentication bytes should be interpreted. See the [authentication section](#authentication) above. |

?

@@ -365,5 +402,8 @@ Secure Parsec Deployment guide](../secure_deployment.md).
| 10 | *(DRCT)* A set of mutually trusted clients has restricted read-write access to the service IPC endpoint. | A Parsec Clients Unix group needs to be created for it to exclusively have read-access on the socket. |
| 11 | *(DSPC)* Before a client's Unix account is deleted, all of its Parsec keys must be deleted as well. | The OS might recycle the same UID previously used by a deleted user and give it to an attacker's account. |
| 12 | The administrator should not change Parsec authentication method while it is currently storing keys. | The keys are not namespaced with authentication type which means the authentication is as strong as the weakest used. |
| 13 | *(JWTS)* A short expiration time should be set on the JWT-SVID token so that if it is stolen, it can only be used temporarily by attackers. | The way to set the expiration time of JWT-SVID tokens is implementation-specific. The administrator should ensure it is not too long. |
| 14 | *(JWTS)* The SPIFFE Workload API endpoint must exist before Parsec is started and owned by an independant user. It must not be deleted while Parsec is running. | It must not be possible for an attacker to put its own socket where the Parsec service and clients expect it. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| 14 | *(JWTS)* The SPIFFE Workload API endpoint must exist before Parsec is started and owned by an independant user. It must not be deleted while Parsec is running. | It must not be possible for an attacker to put its own socket where the Parsec service and clients expect it. |
| 14 | *(JWTS)* The SPIFFE Workload API endpoint must exist before Parsec is started and owned by an independant user. It must not be deleted while Parsec is running. | It must not be possible for an attacker to put their own socket where the Parsec service and clients expect it. |

Copy link
Member

@ionut-arm ionut-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good overall - I still have one question, though it's more related to the service PR.

@@ -330,20 +365,22 @@ cleared.
| 4 | While Parsec is authenticated on the device, anyone can brute force a key ID to execute operations with the key. Unmitigated for the PKCS 11 Provider: all sessions share the login state; if one session logs in then all other opened sessions will also be logged in. Other sessions only need a valid key ID to execute operations with private keys. Mitigated for the TPM Provider: each key is protected by a long, random authentication value, generated by the TPM | All the client's PKCS 11 keys can be used with PKCS 11 operations. |
| 5 | *(DSPC)* Parsec does not know about the security quality of a client's Unix password. | All the client's keys can be used, and exported if allowed by the key's policy. |
| 6 | Parsec does not support using multiple authenticators concurrently ([open issue](https://github.com/parallaxsecond/parsec/issues/272)). | If there is weakness in one authenticator, all the client's keys can be used, and exported if allowed by the key's policy. |
| 7 | *(JWTS)* The validation steps should parse the JWT-SVID token and make sure the claims of the client's token are the same than the ones returned by the validation operation from the Workload API. | All the client's keys can be used, and exported if allowed by the key's policy. |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation steps should parse the JWT-SVID token and make sure the claims of the client's token are the same than the ones returned by the validation operation from the Workload API.

Isn't this something that we should be doing in the service?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this validation step should be common to all JWT-SVID token so I think it could be in rust-spiffe. This is basically to check that the SPIFFE ID that we are validating is the same as the one that is returned by the validation operation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, even though we're working on that repo too, I don't think we need to include that here. I'm sure there are lots of cases like this in other libraries we use, for other data flows

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, yeah, will remove

Updates the authenticator sections and the thread model.

Signed-off-by: Hugues de Valon <[email protected]>
@hug-dev
Copy link
Member Author

hug-dev commented Nov 26, 2020

Removed U-7 and added a link to parallaxsecond/parsec#289 as said here.

@hug-dev
Copy link
Member Author

hug-dev commented Nov 26, 2020

Merging as the only errors are "Too Many Requests" but the links are valid.

@hug-dev hug-dev merged commit bd5d0a3 into parallaxsecond:master Nov 26, 2020
@hug-dev hug-dev deleted the spiffe branch November 26, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update with the new JwtSvid authenticator
2 participants