Skip to content
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

crypto, mTLS: allow certificate chain for trusted_client_ca #511

Merged
merged 4 commits into from
Feb 8, 2023

Conversation

THS-on
Copy link
Member

@THS-on THS-on commented Feb 1, 2023

This allows for setups with intermediate CAs.

@THS-on
Copy link
Member Author

THS-on commented Feb 1, 2023

cc: @aplanas @ansasaki

@THS-on THS-on added the bug Something isn't working label Feb 1, 2023
keylime-agent/src/crypto.rs Outdated Show resolved Hide resolved
@keylime-bot keylime-bot assigned THS-on and unassigned ashcrow, ueno and puiterwijk Feb 2, 2023
This allows for setups with intermediate CAs.

Signed-off-by: Thore Sommer <[email protected]>
@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 4a61ddf to f9446c4 Compare February 2, 2023 11:29
Copy link
Contributor

@aplanas aplanas left a comment

Choose a reason for hiding this comment

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

AFAICT LGTM

@keylime-bot keylime-bot assigned ueno, ashcrow and puiterwijk and unassigned THS-on Feb 2, 2023
@ansasaki
Copy link
Contributor

ansasaki commented Feb 7, 2023

LGTM! Could you please fix that line clippy is complaining about?

@THS-on
Copy link
Member Author

THS-on commented Feb 7, 2023

@ansasaki done

@THS-on
Copy link
Member Author

THS-on commented Feb 7, 2023

It seems like that something has changed how clippy deals with references of strings. My local version of clippy (0.1.67) does not report it. I'll update the rest of issues later.

@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from afb8529 to db2d1b4 Compare February 8, 2023 13:21
@aplanas
Copy link
Contributor

aplanas commented Feb 8, 2023

It is complaining now about the string interpolation

@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 38d4000 to 144b933 Compare February 8, 2023 13:46
@THS-on
Copy link
Member Author

THS-on commented Feb 8, 2023

It is complaining now about the string interpolation

@aplanas should be fixed now.

keylime/src/algorithms.rs Outdated Show resolved Hide resolved
@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 144b933 to 0b1e9ab Compare February 8, 2023 13:53
@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 0b1e9ab to 18bfd9a Compare February 8, 2023 14:10
error!(
"Failed to load trusted CA certificate {}: {}",
ca_cert_path.display(),
e
Copy link
Contributor

Choose a reason for hiding this comment

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

clippy should complain here too, shouldn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

No because you cannot call functions currently in a fromat string, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. But I mean "e"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't know. I would keep it this way for now, because mixing the two styles looks wrong.

@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 18bfd9a to d500481 Compare February 8, 2023 14:20
Copy link
Contributor

@ansasaki ansasaki left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for going through clippy complaints and fixing them!

@ansasaki ansasaki merged commit 5ea9d76 into keylime:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants