Skip to content

crypto, mTLS: allow certificate chain for trusted_client_ca #511

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.

Sign up for GitHub

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

Jump to bottom
Merged
ansasaki merged 4 commits into from
Feb 8, 2023
Merged

crypto, mTLS: allow certificate chain for trusted_client_ca #511

ansasaki 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
aplanas
aplanas requested changes Feb 2, 2023
View reviewed changes
@keylime-bot keylime-bot assigned THS-on and unassigned ashcrow, ueno and puiterwijk Feb 2, 2023
@THS-on
crypto, mTLS: allow certificate chain for trusted_client_ca
f9446c4 
This allows for setups with intermediate CAs.

Signed-off-by: Thore Sommer <mail@thson.de>
@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 4a61ddf to f9446c4 Compare February 2, 2023 11:29
aplanas
aplanas approved these changes Feb 2, 2023
View reviewed changes
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
clippy: fix needless_borrow warnings
db2d1b4 
Signed-off-by: Thore Sommer <mail@thson.de>
@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.

aplanas
aplanas reviewed Feb 8, 2023
View reviewed changes
@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
aplanas
aplanas reviewed Feb 8, 2023
View reviewed changes
keylime-agent/src/main.rs
error!(
"Failed to load trusted CA certificate {}: {}",
ca_cert_path.display(),
e
Copy link
Contributor

@aplanas aplanas Feb 8, 2023

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

@THS-on THS-on Feb 8, 2023

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

@aplanas aplanas Feb 8, 2023

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

@THS-on THS-on Feb 8, 2023

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 added 2 commits February 8, 2023 16:20
@THS-on
clippy: fix uninlined_format_args warnings
b885bb1 
Signed-off-by: Thore Sommer <mail@thson.de>
@THS-on
algorithms: fix typo
d500481 
Signed-off-by: Thore Sommer <mail@thson.de>
@THS-on THS-on force-pushed the bug/mtls-cert-chain branch from 18bfd9a to d500481 Compare February 8, 2023 14:20
ansasaki
ansasaki approved these changes Feb 8, 2023
View reviewed changes
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

Reviewers

@aplanas aplanas aplanas approved these changes

@ansasaki ansasaki ansasaki approved these changes

@ashcrow ashcrow Awaiting requested review from ashcrow

@ueno ueno Awaiting requested review from ueno

@puiterwijk puiterwijk Awaiting requested review from puiterwijk

Assignees

@ashcrow ashcrow

@ueno ueno

@puiterwijk puiterwijk

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@THS-on @ansasaki @aplanas @ashcrow @ueno @puiterwijk