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

fix: set client CAs for mTLS auth #437

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

lu1as
Copy link
Contributor

@lu1as lu1as commented Jan 21, 2024

The new TLS config introduced in #414 does not set client CAs. So when enabling mTLS authentication, client requests fail with tls: failed to verify certificate: x509: certificate signed by unknown authority.
This PR fixes this by using the root CAs also for authenticating mTLS clients, like before:

ClientCAs: rootCAs,

The new TLS config introduced in PR minio#414 does not set client CAs.
So when enabling mTLS authentication, client requests fail with
`certificate signed by unknown authority`.
This commit fixes this by using the root CAs also for
authenticating mTLS clients.

Ref: minio#414
@lu1as lu1as marked this pull request as ready for review January 21, 2024 22:37
@aead aead merged commit a275f23 into minio:master Feb 9, 2024
7 checks passed
@nicopal
Copy link

nicopal commented Feb 20, 2024

Hi, thanks for fixing this, but I think the fix is partial - the root CA must be added to OS system certificates. The error persists if KES uses a client root CA indicated by they key "ca" in the configuration file.

@xstecuc
Copy link

xstecuc commented Feb 20, 2024

Hi!
At this point, do we need an identity for a client API request upon a successful mTLS auth or can the identity of the client be omitted when defining the policies? I expected it to work without identities but I keep receiving:
{"message":"not authorized: insufficient permissions"
if I don't set one.

Thank you!

@lu1as lu1as deleted the fix-mtls-auth branch February 20, 2024 21:44
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.

4 participants