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

Trust ca certs #12513

Merged
merged 3 commits into from
Nov 15, 2023
Merged

Trust ca certs #12513

merged 3 commits into from
Nov 15, 2023

Conversation

markylaing
Copy link
Contributor

This PR cherry-picks fixes from lxc/incus#221 which prevented TLS authorization when using CA certificates.

I have added an extra commit to enforce that a user is authenticated before calling the access handler.

Closes canonical#218

Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
tomponline
tomponline previously approved these changes Nov 7, 2023
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

Thanks. Does user authorization not work with ca mode an the request is considered admin level?

stgraber and others added 2 commits November 15, 2023 10:05
The test as it was written was quite incorrect.

core.trust_ca_certificates causes valid certificates to be implictly
trusted and not get a certificate store entry.

But as the test was run immediately after a configuration where
core.trust_ca_certificates was not set, there was a leftover entry in
the trust store which the test was then checking.

This corrects the test by clearing the trust store after the initial
test and by further making sure that when using
core.trust_ca_certificates, no trust store entries get created.

Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
…ler.

Previously we ran the access handler regardless of whether a request was
authenticated. This would usually fail because there would be no
username in the request context. However we need to be careful that a
user is authenticated if predicating access on the presence of a CA
certificate.

Signed-off-by: Mark Laing <mark.laing@canonical.com>
@tomponline tomponline merged commit cbb1518 into canonical:main Nov 15, 2023
24 of 26 checks passed
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.

3 participants