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.

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

Fix handling of trusted certs in CA mode #221

Merged
merged 2 commits into from
Nov 5, 2023
Merged

Fix handling of trusted certs in CA mode #221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2bb0d8
incusd/auth: Fix handling of trusted certs in CA mode
stgraber Nov 5, 2023
9bd8330
tests: Properly test core.trust_ca_certificates
stgraber Nov 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/server/auth/driver_tls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"

"github.com/lxc/incus/internal/server/certificate"
internalUtil "github.com/lxc/incus/internal/util"
"github.com/lxc/incus/shared/api"
"github.com/lxc/incus/shared/logger"
"github.com/lxc/incus/shared/util"
Expand Down Expand Up @@ -172,5 +173,11 @@ func (t *tls) certificateDetails(fingerprint string) (certificate.Type, bool, []
return certificate.TypeMetrics, false, nil, nil
}

// If we're in a CA environment, it's possible for a certificate to be trusted despite not being present in the trust store.
// We rely on the validation of the certificate (and its potential revocation) having been done in CheckTrustState.
if util.PathExists(internalUtil.VarPath("server.ca")) {
return certificate.TypeClient, true, nil, nil
}

return -1, false, nil, api.StatusErrorf(http.StatusForbidden, "Client certificate not found")
}
12 changes: 4 additions & 8 deletions test/suites/pki.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,22 @@ test_pki() {
token="$(INCUS_DIR=${INCUS5_DIR} incus config trust add foo -q)"
incus_remote remote add pki-incus "${INCUS5_ADDR}" --accept-certificate --token "${token}"
incus_remote config trust ls pki-incus: | grep incus-client
fingerprint="$(incus_remote config trust ls pki-incus: --format csv | cut -d, -f4)"
incus_remote config trust remove pki-incus:"${fingerprint}"
incus_remote remote remove pki-incus

# Add remote using a CA-signed client certificate, and not providing a token.
# This should succeed and tests that the CA trust is working, as adding the client certificate to the trust
# store without a token would normally fail.
INCUS_DIR=${INCUS5_DIR} incus config set core.trust_ca_certificates true
incus_remote remote add pki-incus "${INCUS5_ADDR}" --accept-certificate
incus_remote config trust ls pki-incus: | grep incus-client
! incus_remote config trust ls pki-incus: | grep incus-client || false
incus_remote remote remove pki-incus

# Add remote using a CA-signed client certificate, and providing an incorrect token.
# This should succeed as is the same as the test above but with an incorrect token rather than no token.
incus_remote remote add pki-incus "${INCUS5_ADDR}" --accept-certificate --token=bar
incus_remote config trust ls pki-incus: | grep incus-client

# Try removing the fingerprint.
# This should succeed as the admin can delete all certificates.
fingerprint="$(incus_remote config trust ls pki-incus: --format csv | cut -d, -f4)"
incus_remote config trust rm pki-incus:"${fingerprint}"

! incus_remote config trust ls pki-incus: | grep incus-client || false
incus_remote remote remove pki-incus

# Replace the client certificate with a revoked certificate in the CRL.
Expand Down
Loading