diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index dd6b9fe68..880370ede 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -207,6 +207,43 @@ jobs: # regression test, and need no frontend bundle. run: CGO_ENABLED=1 go test -timeout 5m ./client/ui/authsession/... ./client/ui/i18n/... ./client/ui/preferences/... ./client/ui/services/... + test_client_pkcs11: + name: "Client PKCS#11 / Unit" + # The deb and rpm packages ship the client built with the pkcs11 tag, which loads + # PKCS#11 modules through purego. The other jobs build without the tag, so this one + # compiles that driver and signs with a real SoftHSM token, the build release ships. + runs-on: ubuntu-22.04 + steps: + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + persist-credentials: false + + - name: Install Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version-file: "go.mod" + cache: false + + - name: Install SoftHSM + run: sudo apt update && sudo apt install -y -q softhsm2 + + - name: Initialise token + run: | + mkdir -p "$RUNNER_TEMP/softhsm/tokens" + printf 'directories.tokendir = %s\nobjectstore.backend = file\n' "$RUNNER_TEMP/softhsm/tokens" > "$RUNNER_TEMP/softhsm/softhsm2.conf" + echo "SOFTHSM2_CONF=$RUNNER_TEMP/softhsm/softhsm2.conf" >> "$GITHUB_ENV" + SOFTHSM2_CONF="$RUNNER_TEMP/softhsm/softhsm2.conf" softhsm2-util --init-token --free --label netbird --pin 1234 --so-pin 1234 + + - name: Vet + run: CGO_ENABLED=0 go vet -tags pkcs11 ./client/internal/pkcs11/... ./client/internal/certproof/... + + - name: Test + env: + NB_TEST_PKCS11_URI: "pkcs11:token=netbird?module-path=/usr/lib/softhsm/libsofthsm2.so&pin-value=1234" + NB_TEST_PKCS11_DISPOSABLE: "1" + run: CGO_ENABLED=0 go test -tags pkcs11 -timeout 5m ./client/internal/pkcs11/... ./client/internal/certproof/... + test_client_on_docker: name: "Client (Docker) / Unit" needs: [build-cache] diff --git a/.goreleaser.yaml b/.goreleaser.yaml index a16bd8cf5..6bb4e9e8a 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -333,6 +333,9 @@ nfpms: - src: release_files/netbird.sysconfig dst: /etc/sysconfig/netbird type: config|noreplace + # May hold the PKCS#11 token PIN (NB_TPM_PIN), so readable by root alone. + file_info: + mode: 0600 scripts: postinstall: "release_files/post_install.sh" preremove: "release_files/pre_remove.sh" diff --git a/client/internal/certproof/README.md b/client/internal/certproof/README.md deleted file mode 100644 index 9f7831bc0..000000000 --- a/client/internal/certproof/README.md +++ /dev/null @@ -1,277 +0,0 @@ -# Certificate posture proofs - -A peer answers a management certificate challenge by signing the challenge nonce with a -private key it holds, and sending back the certificate chain. Management verifies the -chain against the CAs configured on the check and verifies the signature, which proves -the peer holds the key rather than merely a copy of the certificate. - -The signature covers `netbird-posture-cert-v1 || nonce || peerKey`, so a proof is bound -to one WireGuard peer key and cannot be replayed by another peer. - -## Where certificates come from - -| Platform | Store | Read by | -| --- | --- | --- | -| macOS | System keychain | the daemon, directly | -| macOS | console user's login keychain | a helper in that user's desktop session | -| Windows | `LocalMachine\MY` | the service, directly | -| Windows | signed-in user's `CurrentUser\MY` | a helper launched with that session's token | -| Linux and others | PEM directory: `CertStoreDir` in the profile config, else `NB_CERT_STORE_DIR`, else `/etc/netbird/certs` | the daemon, directly | -| Linux | a `TSS2 PRIVATE KEY` file in that directory, signed by the TPM | the daemon, through `/dev/tpmrm0` | -| Linux | a PKCS#11 token, tpm2-pkcs11 for one, enabled by `CertPKCS11PIN` in the profile config | the daemon, through the token's module, in builds with the `pkcs11` tag | - -macOS and Windows both keep per-user certificates out of reach of a privileged daemon, -and both are handled the same way: the daemon reads the machine store itself and -launches `netbird posture cert-proof` as the signed-in user for the rest. Only the -signature and the chain come back. The helper, the request and response types and the -subcommand are shared; only the way the child is launched differs. - -## macOS: why the daemon cannot read a login keychain - -The daemon runs as root from a LaunchDaemon. Its keychain search list is the System -keychain, which is where MDM installs device identities, and nothing else. A user's -login keychain is out of reach for reasons that are not about privilege: - -- `login.keychain-db` is unlocked by `securityd` **in the user's session**. The daemon - lives in a different Mach bootstrap namespace, so from where it stands the keychain is - locked no matter which uid it runs as. -- Every private key carries an ACL naming the applications allowed to use it. A process - that is not listed causes a consent prompt *in the user's session*. A daemon has no - session to show one in, so it receives `errSecInteractionNotAllowed (-25308)` instead. - -Dropping to the user's uid with `SysProcAttr.Credential` does **not** fix this: uid is -not what selects the securityd instance, the bootstrap namespace is. The process has to -enter the user's session, which is what `launchctl asuser` does. - -## macOS: the console user helper - -When a certificate challenge arrives and the daemon is root, it: - -1. Reads the System keychain itself, so MDM device identities are answered with no user - session involved. -2. Resolves the console user with `SCDynamicStoreCopyConsoleUser`. -3. Launches itself as that user with - `launchctl asuser sudo -u -H netbird posture cert-proof`, writing the - challenges to the child's stdin as JSON and reading proofs from its stdout. -4. Merges both sets of proofs, dropping a leaf that both keychains hold. - -The child runs `RunHelper`, which uses the ordinary `KeychainStore` — inside the user's -session it simply works. **The private key never crosses the boundary; only the -signature and the certificate chain come back.** - -`-H` matters: it sets `HOME`, which is how the login keychain path is resolved. - -`netbird posture cert-proof` is hidden and not meant to be run by hand. It writes proofs -to stdout and every log line to stderr, so stdout stays parseable. - -## Windows: the service and the signed-in user - -`LocalMachine\MY` is what the service reads, and it is where AD and Intune enrol device -certificates. `CurrentUser\MY` lives in the signed-in user's registry hive with private -keys protected by DPAPI against their profile, so it is only readable while running as -that user. - -The failure mode differs from macOS in an important way: a service that opens -`CURRENT_USER` does **not** get an error. "Current user" resolves to the service -account's own hive, `HKU\S-1-5-18`, so it silently reads an empty and irrelevant store. -There is nothing to log. That is why the service only ever opens `LocalMachine` and asks -a helper for the rest. - -Windows does let a privileged service assume a user identity, which macOS does not for -keychains, so no external tooling is involved: - -```go -windows.WTSQueryUserToken(session, &token) -cmd.SysProcAttr = &syscall.SysProcAttr{Token: syscall.Token(token), CreationFlags: windows.CREATE_NO_WINDOW} -``` - -`CREATE_NO_WINDOW` matters: without it a console window flashes on the user's desktop on -every sync. - -Session selection prefers the physical console, then falls back to any active session, -so remote desktop and VDI hosts work. `WTSQueryUserToken` needs `SE_TCB_NAME`, which -LocalSystem holds and an ordinary process does not, so a user-run `netbird up` skips the -helper and reads the machine store alone. - -In-process impersonation would also work, but it is per-OS-thread while goroutines -migrate freely, so it would need `runtime.LockOSThread` around every key operation. The -child process avoids that class of bug entirely. - -Unlike macOS, the Windows store acquires keys with `CRYPT_ACQUIRE_SILENT_FLAG`, so a key -that would need a prompt fails immediately instead of blocking. That also means a -smartcard PIN can never be satisfied this way. - -## Linux: keys held by the TPM - -Enrollment tooling on Linux keeps a TPM-resident key as a `TSS2 PRIVATE KEY` PEM file, -the format of draft-bottomley-tpm2-keys that tpm2-openssl, tpm2-tss-engine and -`tpm2_encodeobject` write. The file holds the key wrapped by its parent; the TPM is the -only thing that can use it. Drop it next to the certificate as usual: - -``` -openssl genpkey -provider tpm2 -algorithm EC -pkeyopt group:P-256 -out /etc/netbird/certs/device.key -openssl req -provider tpm2 -provider default -new -key /etc/netbird/certs/device.key -subj /CN=device -out device.csr -``` - -Sign the CSR with the organisation CA and store the result as `device.pem`. The store -parses the key file without touching the TPM, so the certificate is listed as a -candidate like any other, and every signature opens `/dev/tpmrm0`, loads the key under -its parent, signs, flushes and closes again. `NB_TPM_DEVICE` overrides the device path. - -What the key file may look like: - -- **Parent.** A persistent handle such as `0x81000001` is used as is. The owner - hierarchy, which both tpm2-openssl and tpm2-tss-engine default to, means the key was - created under a transient primary from the TCG default ECC P-256 template, and that - same primary is derived again before loading. -- **No authorization value.** A key created with a password needs someone to type it, - which the daemon cannot arrange, so the certificate is skipped with a log line rather - than blocking on a TPM auth failure. -- **RSA-2048 or P-256, sometimes P-384.** Those are what the PC Client profile requires - of a TPM; P-384 depends on the chip. The TPM chooses the RSA-PSS salt itself, which is - why management verifies PSS proofs with `rsa.PSSSaltLengthAuto`. - -Windows needs none of this: a certificate enrolled into the TPM sits behind the Microsoft -Platform Crypto Provider and the CNG path above signs with it unchanged. macOS has no -TPM; its Secure Enclave keys are reachable only through the keychain path. - -To exercise the path without hardware, run a software TPM and point the end-to-end test -at it: - -``` -swtpm socket --tpm2 --server type=unixio,path=/tmp/swtpm.sock --ctrl type=unixio,path=/tmp/swtpm.ctrl --flags not-need-init,startup-clear -NB_TPM_DEVICE=/tmp/swtpm.sock go test ./client/internal/certproof/ -run TestCollect_TPMKeyEndToEnd -v -``` - -## Linux: keys behind a PKCS#11 token - -Distributions that follow Red Hat's guidance reach the TPM through tpm2-pkcs11, a PKCS#11 -module whose token holds both the key and, after `tpm2_ptool addcert`, the certificate. -The store reads that token when the profile config, `/etc/netbird/config.json` by default, -carries the token's user PIN: - -```json -"CertPKCS11PIN": "1234" -``` - -That alone opens the first token the p11-kit proxy exposes, which is tpm2-pkcs11 on a -stock setup that has registered it. `CertPKCS11URI`, an RFC 7512 URI, narrows that down -on a host with several tokens or without p11-kit: - -```json -"CertPKCS11URI": "pkcs11:token=netbird?module-path=/usr/lib/x86_64-linux-gnu/libtpm2_pkcs11.so" -``` - -`token` selects the token by label, or the first token present when absent. `module-path` -names the library to load; `module-name=tpm2_pkcs11` resolves to `libtpm2_pkcs11.so` on -the loader's search path, and with neither the p11-kit proxy is loaded, which exposes every -module the system has registered. The URI may carry the PIN itself, as `pin-value` inline -or `pin-source` naming a file, and `CertPKCS11PIN` takes precedence over both. Without any -PIN no login happens, and tpm2-pkcs11 then shows no private keys at all. Every other -attribute is ignored. - -The certificate may live on the token or in the PEM directory: `CertStoreDir` in the -profile config, else `NB_CERT_STORE_DIR`, else `/etc/netbird/certs`. On the token, -certificates and private keys are paired by `CKA_ID`, -which is what `tpm2_ptool addcert` and `pkcs11-tool` set. In the directory, a certificate -file without a key of its own is paired with the token key whose public key it carries, so -`device.pem` alone next to a key that only the TPM holds is enough; the token's public key -object, which `tpm2_ptool addkey` and `import` create alongside the private one, is what -the store compares against. Chains are completed from the certificates on the token and in -the directory together, so intermediates may sit in either place. - -Each operation opens a session, logs in, works, logs out and closes, so no token handle -outlives a call, and the PEM directory keeps working when the token does not: the two are -queried together and a failing token is logged rather than hiding file certificates. - -Two consequences of the PIN are worth knowing. It is a secret on disk, which the profile -config already is: it holds the WireGuard private key and is written readable by root -alone, and the debug bundle's config dump leaves `CertPKCS11PIN` out. And a wrong PIN -counts against the TPM's dictionary-attack lockout, which is shared with everything else -on the machine that uses the TPM. - -The module is loaded at runtime without cgo, through `purego`, which means the binary is -dynamically linked against libc. The store is therefore compiled in only with `-tags pkcs11` -on linux/amd64 and linux/arm64: the deb and rpm packages are built that way, since they -target glibc distributions, while the release tarballs and the Alpine-based container -images keep the fully static build. Without the tag, setting `CertPKCS11PIN` logs that -the build lacks the support. - -To exercise the path without hardware, initialise a SoftHSM token and run the end-to-end -test, which imports a key and certificate itself: - -``` -softhsm2-util --init-token --free --label netbird --pin 1234 --so-pin 1234 -NB_TEST_PKCS11_URI='pkcs11:token=netbird?module-path=/usr/lib/softhsm/libsofthsm2.so&pin-value=1234' \ - go test -tags pkcs11 ./client/internal/certproof/ -run PKCS11 -v -``` - -## Only the signed-in user can be validated - -This is the central limitation of the design, and it is deliberate. - -A proof from a user store can only ever be produced for **the user whose session is -currently open**. Consequences worth designing around: - -- **At the sign-in screen there is no user proof.** macOS reports no console user or - attributes the console to root, and `CurrentConsoleUser` returns false for both. - Windows reports no active session with a token. Only machine proofs are sent, so a - posture check that demands a user certificate fails on a machine nobody has signed - into yet. -- **Signing out changes the answer.** Posture can flip between compliant and - non-compliant across a sign-out, so management should treat "no proof" as its own - state rather than as a failed check, or users get disconnected at the sign-in screen. -- **One session is asked, not all of them.** macOS asks the console user, so other - fast-user-switched accounts are skipped even though their keychains are unlocked. - Windows prefers the console and otherwise takes the first active session. If you ever - need every signed-in user, both platforms would have to enumerate sessions and ask - each one. -- **A locked keychain still blocks signing.** A user can be logged in with their - keychain locked (locked on sleep, or manually). The helper then needs an unlock prompt - and may block, which is why the spawn has a 30s timeout and a failure is reported as - "no proof" rather than an error. -- **The first signature prompts.** The user sees "netbird wants to use your confidential - information stored in ...". Choosing *Always Allow* records the helper's designated - requirement in the key's ACL, so it persists across restarts and updates while the - signing identity is stable. Unsigned or ad-hoc development builds re-prompt every run. - -## What a user proof does and does not attest - -It attests: *some process in that user's session had ACL permission to use a private key -whose certificate chains to CA X, and signed a nonce bound to this peer key*. - -It does not attest that the daemon controls the key, that the key is hardware-bound, or -that a particular binary produced the signature. Any code running in that user's session -with an existing ACL grant can produce the same signature by calling -`SecKeyCreateSignature` directly — the proof format is not a secret. The helper does not -create that capability, it only packages it. - -If you need a stronger guarantee, use a device identity that never involves a user -session (MDM into the System keychain, which the daemon reads directly), or a key that -requires user presence for each signature (Secure Enclave or a PIV token). - -## Reading the logs - -Everything in this path logs at info. A healthy macOS run shows, in order: - -``` -certificate posture: answering N certificate challenges from store *certproof.KeychainStore -macOS Security framework loaded for certificate posture, running as uid=0 euid=0 -keychain search list contains 2 keychains -keychain search list[0]: /Library/Keychains/System.keychain -keychain identity query returned N items -certificate posture: asking the desktop session of "user" (uid 501) to answer N challenges -certificate posture: desktop session of "user" returned N proofs -peer meta carries N certificate posture proofs -``` - -Common outcomes and what they mean: - -| Log line | Meaning | -| --- | --- | -| `keychain identity query returned errSecItemNotFound (-25300)` | The keychain is readable and holds no identity of that class. Any other OSStatus is a real access failure. | -| `holds no identities usable for certificate posture, but N readable certificates` | Reading works; the certificate is present without its private key, or is not there at all. | -| `no console user is logged in` | Login window. Device proofs only. | -| `has no issuer in the keychain` | The chain ships leaf-only and verifies only if the challenge supplies that exact root. | -| `challenge N rejected "..." : x509: unhandled critical extension` | The chain is fine but Go refuses an extension in it, which is common for Apple-issued certificates. | -| `challenge N matched none of the M candidates` | Every candidate was rejected; the preceding lines give the reason for each. | diff --git a/client/internal/certproof/collect.go b/client/internal/certproof/collect.go index d2b5d87b0..65affca71 100644 --- a/client/internal/certproof/collect.go +++ b/client/internal/certproof/collect.go @@ -2,10 +2,13 @@ package certproof import ( "context" + "crypto" "crypto/sha256" + "crypto/x509" "time" log "github.com/sirupsen/logrus" + "golang.zx2c4.com/wireguard/wgctrl/wgtypes" "github.com/netbirdio/netbird/shared/management/certposture" "github.com/netbirdio/netbird/shared/management/proto" @@ -25,14 +28,24 @@ func Collect(ctx context.Context, store Store, checks []*proto.Checks, peerKey [ func logNoChallenges(checks []*proto.Checks) { if len(checks) > 0 { - log.Infof("certificate posture: %d posture checks received, none carries a certificate challenge", len(checks)) + log.Debugf("certificate posture: %d posture checks received, none carries a certificate challenge", len(checks)) } } // CollectChallenges answers challenges already extracted from the posture checks, so a // caller that ships them across a process boundary reuses the same matching and signing. +// Only nonces of the size management issues are signed, for a peer key of the size of +// ours, so the keys behind the store never sign arbitrary caller-chosen data. func CollectChallenges(ctx context.Context, store Store, challenges []*proto.CertificateChallenge, peerKey []byte) []certposture.Proof { - log.Infof("certificate posture: answering %d certificate challenges from store %T", len(challenges), store) + if len(peerKey) != wgtypes.KeyLen { + log.Warnf("certificate posture: refusing to sign for a %d byte peer key", len(peerKey)) + return nil + } + challenges = wellFormed(challenges) + if len(challenges) == 0 { + return nil + } + log.Debugf("certificate posture: answering %d certificate challenges from store %T", len(challenges), store) candidates, err := store.Candidates(ctx) if err != nil { @@ -40,10 +53,10 @@ func CollectChallenges(ctx context.Context, store Store, challenges []*proto.Cer return nil } if len(candidates) == 0 { - log.Info("certificate posture: certificate store holds no candidates, no proof will be sent") + log.Debug("certificate posture: certificate store holds no candidates, no proof will be sent") return nil } - log.Infof("certificate posture: store holds %d candidate certificates", len(candidates)) + log.Debugf("certificate posture: store holds %d candidate certificates", len(candidates)) now := time.Now() proven := make(map[[sha256.Size]byte]struct{}) @@ -54,7 +67,7 @@ func CollectChallenges(ctx context.Context, store Store, challenges []*proto.Cer log.Warnf("skipping certificate challenge with invalid CA certificates: %v", err) continue } - log.Infof("certificate posture: challenge %d accepts %d CA certificates, nonce is %d bytes", i, len(challenge.GetCaCertificates()), len(challenge.GetNonce())) + log.Debugf("certificate posture: challenge %d accepts %d CA certificates, nonce is %d bytes", i, len(challenge.GetCaCertificates()), len(challenge.GetNonce())) matched := false for _, candidate := range candidates { @@ -62,53 +75,84 @@ func CollectChallenges(ctx context.Context, store Store, challenges []*proto.Cer continue } leaf := candidate.Chain[0] - if err := certposture.VerifyChain(candidate.Chain, roots, now); err != nil { - log.Infof("certificate posture: challenge %d rejected %q issued by %q, chain of %d: %v", i, leaf.Subject, leaf.Issuer, len(candidate.Chain), err) + chain, err := certposture.VerifiedChain(leaf, candidate.issuers(), roots, now) + if err != nil { + log.Debugf("certificate posture: challenge %d rejected %q issued by %q: %v", i, leaf.Subject, leaf.Issuer, err) continue } matched = true - fingerprint := sha256.Sum256(leaf.Raw) + // The same leaf can chain to different CAs for different challenges, and + // management checks each chain against each check's CAs, so a proof is + // deduplicated by its whole chain rather than by its leaf. + fingerprint := chainFingerprint(chain) if _, done := proven[fingerprint]; done { - log.Infof("certificate posture: challenge %d matched %q, already proven for an earlier challenge", i, leaf.Subject) + log.Debugf("certificate posture: challenge %d matched %q, already proven for an earlier challenge", i, leaf.Subject) break } - proof, err := prove(candidate, challenge.GetNonce(), peerKey) + proof, err := prove(candidate.Signer, chain, challenge.GetNonce(), peerKey) if err != nil { log.Warnf("failed signing certificate proof for %s: %v", leaf.Subject, err) continue } - log.Infof("certificate posture: challenge %d proven by %q with %s, signature %d bytes, chain of %d", i, leaf.Subject, proof.SigAlg, len(proof.Signature), len(proof.Chain)) + log.Debugf("certificate posture: challenge %d proven by %q with %s, signature %d bytes, chain of %d", i, leaf.Subject, proof.SigAlg, len(proof.Signature), len(proof.Chain)) proven[fingerprint] = struct{}{} proofs = append(proofs, proof) break } if !matched { - log.Infof("certificate posture: challenge %d matched none of the %d candidates", i, len(candidates)) + log.Debugf("certificate posture: challenge %d matched none of the %d candidates", i, len(candidates)) } } - log.Infof("certificate posture: %d challenges produced %d proofs", len(challenges), len(proofs)) + log.Debugf("certificate posture: %d challenges produced %d proofs", len(challenges), len(proofs)) return proofs } +// HasChallenges reports whether any of checks asks for a certificate proof. +func HasChallenges(checks []*proto.Checks) bool { + return len(certificateChallenges(checks)) > 0 +} + func certificateChallenges(checks []*proto.Checks) []*proto.CertificateChallenge { var challenges []*proto.CertificateChallenge for _, check := range checks { - if challenge := check.GetCertificateChallenge(); challenge != nil && len(challenge.GetNonce()) > 0 { + if challenge := check.GetCertificateChallenge(); challenge != nil { challenges = append(challenges, challenge) } } - return challenges + return wellFormed(challenges) } -func prove(candidate Candidate, nonce, peerKey []byte) (certposture.Proof, error) { - sigAlg, sig, err := certposture.Sign(candidate.Signer, nonce, peerKey) +// wellFormed drops challenges whose nonce is not one management could have issued. +func wellFormed(challenges []*proto.CertificateChallenge) []*proto.CertificateChallenge { + var kept []*proto.CertificateChallenge + for _, challenge := range challenges { + if len(challenge.GetNonce()) != certposture.NonceSize { + log.Debugf("certificate posture: skipping challenge with a %d byte nonce", len(challenge.GetNonce())) + continue + } + kept = append(kept, challenge) + } + return kept +} + +func prove(signer crypto.Signer, chain []*x509.Certificate, nonce, peerKey []byte) (certposture.Proof, error) { + sigAlg, sig, err := certposture.Sign(signer, nonce, peerKey) if err != nil { return certposture.Proof{}, err } - chain := make([][]byte, 0, len(candidate.Chain)) - for _, cert := range candidate.Chain { - chain = append(chain, cert.Raw) + der := make([][]byte, 0, len(chain)) + for _, cert := range chain { + der = append(der, cert.Raw) } - return certposture.Proof{Nonce: nonce, Chain: chain, SigAlg: sigAlg, Signature: sig}, nil + return certposture.Proof{Nonce: nonce, Chain: der, SigAlg: sigAlg, Signature: sig}, nil +} + +func chainFingerprint(chain []*x509.Certificate) [sha256.Size]byte { + buf := make([]byte, 0, len(chain)*sha256.Size) + for _, cert := range chain { + certHash := sha256.Sum256(cert.Raw) + buf = append(buf, certHash[:]...) + } + return sha256.Sum256(buf) } diff --git a/client/internal/certproof/collect_darwin.go b/client/internal/certproof/collect_darwin.go index b52f00e50..92f0471e7 100644 --- a/client/internal/certproof/collect_darwin.go +++ b/client/internal/certproof/collect_darwin.go @@ -1,14 +1,15 @@ +//go:build !ios + package certproof import ( - "bytes" "context" - "encoding/json" + "errors" "fmt" "os" "os/exec" + "path/filepath" "strconv" - "strings" "time" log "github.com/sirupsen/logrus" @@ -19,12 +20,15 @@ import ( const helperTimeout = 30 * time.Second +// userHelperBackoff holds off asking a user's keychain again after it proved nothing. +var userHelperBackoff = newHelperBackoff() + // CollectProofs answers the certificate challenges in checks from every store this Mac // can reach. The root daemon reads the System keychain itself, which is where MDM // installs device identities, and reaches the console user's login keychain only by // launching a helper into that user's session. A Mac sitting at the login window // therefore yields device proofs alone. -func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, _ Config) []certposture.Proof { +func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, cfg Config) []certposture.Proof { challenges := certificateChallenges(checks) if len(challenges) == 0 { logNoChallenges(checks) @@ -38,59 +42,96 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, } proofs := CollectChallenges(ctx, DefaultStore(), challenges, peerKey) + if cfg.OwnerUnknown { + return proofs + } - userProofs, err := collectAsConsoleUser(ctx, challenges, peerKey) + userProofs, err := collectAsConsoleUser(ctx, cfg.ProfileOwner, challenges, peerKey) if err != nil { - log.Infof("certificate posture: console user keychain unavailable: %v", err) + log.Debugf("certificate posture: console user keychain unavailable: %v", err) } return mergeProofs(proofs, userProofs) } +// UserContext identifies the user whose keychain a collection would include: the console +// user when it owns the active profile, or empty when no user keychain would be asked. A +// change means a collection made earlier no longer reflects what this Mac can prove. +func UserContext(cfg Config) string { + if os.Geteuid() != 0 || cfg.OwnerUnknown { + return "" + } + user, ok := CurrentConsoleUser() + if !ok || !user.isOwner(cfg.ProfileOwner) { + return "" + } + return strconv.FormatUint(uint64(user.UID), 10) + ":" + user.Name +} + // collectAsConsoleUser runs the helper inside the desktop session of the logged-in // user. Dropping to their uid is not enough: keychain access is an XPC call to a // per-session securityd, so the helper has to enter their Mach bootstrap namespace, // which is what launchctl asuser does. -func collectAsConsoleUser(ctx context.Context, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) { +func collectAsConsoleUser(ctx context.Context, owner string, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) { user, ok := CurrentConsoleUser() if !ok { return nil, nil } + if !user.isOwner(owner) { + log.Debugf("certificate posture: console user %s does not own the active profile, no user keychain is asked", user.Name) + return nil, nil + } + + uid := strconv.FormatUint(uint64(user.UID), 10) + backoffKey := helperBackoffKey(uid, challenges) + if !userHelperBackoff.allow(backoffKey, time.Now()) { + log.Debugf("certificate posture: the keychain of uid %s proved nothing recently, not asking again yet", uid) + return nil, nil + } binary, err := os.Executable() if err != nil { return nil, fmt.Errorf("resolve own binary: %w", err) } - payload, err := json.Marshal(helperRequest(challenges, peerKey)) - if err != nil { - return nil, fmt.Errorf("encode helper request: %w", err) - } - + parent := ctx ctx, cancel := context.WithTimeout(ctx, helperTimeout) defer cancel() - uid := strconv.FormatUint(uint64(user.UID), 10) - cmd := exec.CommandContext(ctx, "launchctl", "asuser", uid, "sudo", "-u", user.Name, "-H", binary, "posture", "cert-proof") - cmd.Stdin = bytes.NewReader(payload) - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr + // Absolute paths, because the daemon's PATH is configurable through the service + // environment, and sudo selects the user by uid so the name never has to round-trip. + cmd := exec.CommandContext(ctx, "/bin/launchctl", "asuser", uid, "/usr/bin/sudo", "-u", "#"+uid, "-H", "--", binary, "posture", "cert-proof") + killHelperGroupOnCancel(cmd) - log.Infof("certificate posture: asking the desktop session of %q (uid %s) to answer %d challenges", user.Name, uid, len(challenges)) - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("run helper as %s: %w: %s", user.Name, err, strings.TrimSpace(stderr.String())) + log.Debugf("certificate posture: asking the desktop session of uid %s to answer %d challenges", uid, len(challenges)) + proofs, err := runHelperCmd(cmd, helperRequest(challenges, peerKey)) + // A run that completed, or ran into the timeout waiting on a prompt nobody answered, + // tells whether the keychain proves anything. A launch or output failure, or a run the + // caller cut short, says nothing about it and must not hold off the next one. + timedOut := errors.Is(ctx.Err(), context.DeadlineExceeded) && parent.Err() == nil + if err == nil || timedOut { + userHelperBackoff.record(backoffKey, err == nil && len(proofs) > 0, time.Now()) } - - var resp HelperResponse - if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { - return nil, fmt.Errorf("decode helper response: %w", err) + if err != nil { + return nil, fmt.Errorf("run helper as uid %s: %w", uid, err) } - log.Infof("certificate posture: desktop session of %q returned %d proofs", user.Name, len(resp.Proofs)) - return resp.Proofs, nil + log.Debugf("certificate posture: desktop session of uid %s returned %d proofs", uid, len(proofs)) + return proofs, nil } -// helperStore is the store the helper reads. On macOS the keychain search list of the -// user's own session already is that user's keychain, so the platform default is right. +// helperStore is the store the helper reads: the user's login keychain alone. The +// session's search list also holds the System keychain, which the daemon reads itself, +// and using a System keychain key from the user's session would ask for an +// administrator's approval. func helperStore() Store { - return DefaultStore() + home, err := os.UserHomeDir() + if err != nil { + log.Debugf("certificate posture: no home directory, searching the default keychain list: %v", err) + return NewKeychainStore() + } + login := filepath.Join(home, "Library", "Keychains", "login.keychain-db") + if _, err := os.Stat(login); err != nil { + // Keychains created before macOS 10.12 keep the old file name. + login = filepath.Join(home, "Library", "Keychains", "login.keychain") + } + return NewKeychainStore(login) } diff --git a/client/internal/certproof/collect_js.go b/client/internal/certproof/collect_js.go new file mode 100644 index 000000000..fd7c4c690 --- /dev/null +++ b/client/internal/certproof/collect_js.go @@ -0,0 +1,31 @@ +//go:build js + +package certproof + +import ( + "context" + + "github.com/netbirdio/netbird/shared/management/certposture" + "github.com/netbirdio/netbird/shared/management/proto" +) + +// CollectProofs proves nothing in a browser: it has no TPM, certificate store or file +// store to sign with, so the stores stay out of the WebAssembly build. +func CollectProofs(context.Context, []*proto.Checks, []byte, Config) []certposture.Proof { + return nil +} + +// UserContext identifies the user whose certificates a collection would include. A +// browser has no per-user store, so it never changes. +func UserContext(Config) string { + return "" +} + +// DefaultStore is empty in a browser. +func DefaultStore() Store { + return Stores{} +} + +func helperStore() Store { + return DefaultStore() +} diff --git a/client/internal/certproof/collect_other.go b/client/internal/certproof/collect_other.go index d9c49c807..785d3db65 100644 --- a/client/internal/certproof/collect_other.go +++ b/client/internal/certproof/collect_other.go @@ -1,4 +1,4 @@ -//go:build !darwin && !windows +//go:build ((!darwin && !windows) || ios) && !js package certproof @@ -17,8 +17,15 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, return Collect(ctx, storeWithToken(cfg), checks, peerKey) } +// UserContext identifies the user whose certificates a collection would include. These +// platforms have no per-user store, so it never changes. +func UserContext(Config) string { + return "" +} + // helperStore is the store the helper reads. Nothing launches a helper on these -// platforms, so it is the platform default. +// platforms, so it is the store the daemon reads, configured from the same environment, +// which lets an administrator check a setup by running the helper by hand. func helperStore() Store { - return DefaultStore() + return storeWithToken(Config{PKCS11: PKCS11FromEnv()}) } diff --git a/client/internal/certproof/collect_test.go b/client/internal/certproof/collect_test.go index 16768b974..e16ee3668 100644 --- a/client/internal/certproof/collect_test.go +++ b/client/internal/certproof/collect_test.go @@ -2,6 +2,9 @@ package certproof import ( "context" + "crypto/rand" + "crypto/x509" + "math/big" "os" "path/filepath" "testing" @@ -22,7 +25,7 @@ func TestCollect_ProvesOneMatchingCertificatePerChallenge(t *testing.T) { otherCA := certtest.NewCA(t, "other-root") unrelatedCA := certtest.NewCA(t, "unrelated-root") - dir := t.TempDir() + dir := storeDir(t) deviceKey := certtest.ECDSAKey(t) device := corpCA.Issue(t, deviceKey, "device") writeFile(t, dir, "device.pem", certtest.CertPEM(device)+certtest.KeyPEM(t, deviceKey)) @@ -61,7 +64,7 @@ func TestCollect_ProvesOneMatchingCertificatePerChallenge(t *testing.T) { } func TestCollect_NothingToProve(t *testing.T) { - dir := t.TempDir() + dir := storeDir(t) key := certtest.ECDSAKey(t) ca := certtest.NewCA(t, "root") writeFile(t, dir, "device.pem", certtest.CertPEM(ca.Issue(t, key, "device"))+certtest.KeyPEM(t, key)) @@ -74,7 +77,7 @@ func TestCollect_NothingToProve(t *testing.T) { {"no checks", NewFileStore(dir), nil}, {"files only", NewFileStore(dir), []*proto.Checks{{Files: []string{"/bin/x"}}}}, {"challenge without nonce", NewFileStore(dir), []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{CaCertificates: []string{ca.PEM}}}}}, - {"missing store dir", NewFileStore(filepath.Join(dir, "missing")), []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{Nonce: []byte{1}, CaCertificates: []string{ca.PEM}}}}}, + {"missing store dir", NewFileStore(filepath.Join(dir, "missing")), []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{Nonce: make([]byte, certposture.NonceSize), CaCertificates: []string{ca.PEM}}}}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -89,7 +92,7 @@ func TestFileStore_ChainWithIntermediate(t *testing.T) { key := certtest.ECDSAKey(t) leaf := intermediate.Issue(t, key, "device") - dir := t.TempDir() + dir := storeDir(t) writeFile(t, dir, "device.pem", certtest.CertPEM(leaf)+certtest.CertPEM(intermediate.Cert)+certtest.KeyPEM(t, key)) candidates, err := NewFileStore(dir).Candidates(context.Background()) @@ -102,7 +105,154 @@ func TestFileStore_ChainWithIntermediate(t *testing.T) { assert.NoError(t, certposture.VerifyChain(candidates[0].Chain, roots, time.Now())) } +// storeDir is a PEM directory the store accepts: t.TempDir follows the umask, which +// leaves the directory group-writable on systems with a user-private group umask. +func storeDir(t *testing.T) string { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.Chmod(dir, 0o700)) + return dir +} + func writeFile(t *testing.T, dir, name, content string) { t.Helper() require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600)) } + +func TestCollectChallenges_RefusesMalformedInput(t *testing.T) { + ca := certtest.NewCA(t, "corp-root") + dir := storeDir(t) + key := certtest.ECDSAKey(t) + writeFile(t, dir, "device.pem", certtest.CertPEM(ca.Issue(t, key, "device"))+certtest.KeyPEM(t, key)) + store := NewFileStore(dir) + nonce := certposture.NewChallenger([]byte("secret")).Nonce(peerKey, time.Now()) + + tests := []struct { + name string + nonce []byte + peerKey []byte + want int + }{ + {"issued nonce and peer key are signed", nonce, peerKey, 1}, + {"short nonce is not signed", nonce[:8], peerKey, 0}, + {"oversized nonce is not signed", append(append([]byte{}, nonce...), 0), peerKey, 0}, + {"short peer key is not signed", nonce, peerKey[:16], 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + challenges := []*proto.CertificateChallenge{{Nonce: tt.nonce, CaCertificates: []string{ca.PEM}}} + assert.Len(t, CollectChallenges(context.Background(), store, challenges, tt.peerKey), tt.want, + "the device key signs only what management could have issued") + }) + } +} + +func TestFileStore_SkipsKeyOfAnotherCertificate(t *testing.T) { + ca := certtest.NewCA(t, "corp-root") + dir := storeDir(t) + + // A stale key next to a renewed certificate, sorted before the good pair, must not + // produce a proof that management rejects and stop the search there. + writeFile(t, dir, "a-renewed.crt", certtest.CertPEM(ca.Issue(t, certtest.ECDSAKey(t), "renewed"))) + writeFile(t, dir, "a-renewed.key", certtest.KeyPEM(t, certtest.ECDSAKey(t))) + goodKey := certtest.ECDSAKey(t) + good := ca.Issue(t, goodKey, "good") + writeFile(t, dir, "b-good.pem", certtest.CertPEM(good)+certtest.KeyPEM(t, goodKey)) + + candidates, err := NewFileStore(dir).Candidates(context.Background()) + require.NoError(t, err) + require.Len(t, candidates, 1, "only the certificate whose key matches is a candidate") + assert.True(t, good.Equal(candidates[0].Chain[0]), "the matching pair is kept") + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + proofs := CollectChallenges(context.Background(), NewFileStore(dir), []*proto.CertificateChallenge{{Nonce: nonce, CaCertificates: []string{ca.PEM}}}, peerKey) + require.Len(t, proofs, 1) + _, err = challenger.Verify(proofs[0], peerKey, now) + assert.NoError(t, err, "the proof sent is one management accepts") +} + +// staticStore hands out fixed candidates, for scenarios no on-disk layout can express. +type staticStore []Candidate + +func (s staticStore) Candidates(context.Context) ([]Candidate, error) { return s, nil } + +func TestCollectChallenges_RoutesAroundExpiredCopyOfRenewedIntermediate(t *testing.T) { + root := certtest.NewCA(t, "root") + intermediate := certtest.NewIntermediate(t, root, "issuing-ca") + + // Renewing a CA with the same key pair leaves two certificates with the same + // subject and key in the store. The expired one sorts first here. + expiredTmpl := *intermediate.Cert + expiredTmpl.SerialNumber = big.NewInt(1) + expiredTmpl.NotBefore = time.Now().Add(-72 * time.Hour) + expiredTmpl.NotAfter = time.Now().Add(-48 * time.Hour) + der, err := x509.CreateCertificate(rand.Reader, &expiredTmpl, root.Cert, intermediate.Key.Public(), root.Key) + require.NoError(t, err) + expired, err := x509.ParseCertificate(der) + require.NoError(t, err) + + key := certtest.ECDSAKey(t) + leaf := intermediate.Issue(t, key, "device") + pool := []*x509.Certificate{expired, intermediate.Cert} + + chain := buildChain(leaf, pool) + require.Len(t, chain, 2) + require.True(t, expired.Equal(chain[1]), "precondition: the first-match chain runs through the expired copy") + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + store := staticStore{{Chain: chain, Signer: key, Intermediates: pool}} + + proofs := CollectChallenges(context.Background(), store, []*proto.CertificateChallenge{{Nonce: nonce, CaCertificates: []string{root.PEM}}}, peerKey) + + require.Len(t, proofs, 1, "a valid path through the renewed intermediate exists, so the challenge is answered") + verified, err := challenger.Verify(proofs[0], peerKey, now) + require.NoError(t, err) + assert.True(t, intermediate.Cert.Equal(verified[1]), "the proof carries the valid intermediate, not the expired copy") + assert.True(t, certposture.ChainMatchesCAs(certposture.EncodeChainPEM(verified), []string{root.PEM}, now), + "management's own check accepts the chain the proof carries") +} + +func TestCollectChallenges_ProvesALeafOncePerDistinctChain(t *testing.T) { + rootA := certtest.NewCA(t, "root-a") + rootB := certtest.NewCA(t, "root-b") + issuer := certtest.NewIntermediate(t, rootA, "issuing-ca") + + // The same issuing CA cross-signed by a second root: one leaf, two valid paths. + crossTmpl := *issuer.Cert + crossTmpl.SerialNumber = big.NewInt(2) + der, err := x509.CreateCertificate(rand.Reader, &crossTmpl, rootB.Cert, issuer.Key.Public(), rootB.Key) + require.NoError(t, err) + cross, err := x509.ParseCertificate(der) + require.NoError(t, err) + + key := certtest.ECDSAKey(t) + leaf := issuer.Issue(t, key, "device") + pool := []*x509.Certificate{issuer.Cert, cross} + store := staticStore{{Chain: buildChain(leaf, pool), Signer: key, Intermediates: pool}} + + challenger := certposture.NewChallenger([]byte("secret")) + now := time.Now() + nonce := challenger.Nonce(peerKey, now) + challenges := []*proto.CertificateChallenge{ + {Nonce: nonce, CaCertificates: []string{rootA.PEM}}, + {Nonce: nonce, CaCertificates: []string{rootB.PEM}}, + {Nonce: nonce, CaCertificates: []string{rootA.PEM}}, + } + + proofs := CollectChallenges(context.Background(), store, challenges, peerKey) + + require.Len(t, proofs, 2, "one proof per distinct chain, the repeated root-a challenge reuses the first") + for _, root := range []*certtest.CA{rootA, rootB} { + matched := false + for _, p := range proofs { + chain, err := challenger.Verify(p, peerKey, now) + require.NoError(t, err) + matched = matched || certposture.ChainMatchesCAs(certposture.EncodeChainPEM(chain), []string{root.PEM}, now) + } + assert.True(t, matched, "management can match a chain for the check that trusts %s", root.Cert.Subject.CommonName) + } +} diff --git a/client/internal/certproof/collect_windows.go b/client/internal/certproof/collect_windows.go index 074662fdb..8e5695539 100644 --- a/client/internal/certproof/collect_windows.go +++ b/client/internal/certproof/collect_windows.go @@ -1,13 +1,10 @@ package certproof import ( - "bytes" "context" - "encoding/json" "fmt" "os" "os/exec" - "strings" "syscall" "time" @@ -25,7 +22,7 @@ const helperTimeout = 30 * time.Second // Intune enrol device certificates, and reaches the signed-in user's store by launching // a helper with that session's token. A machine at the sign-in screen therefore proves // device certificates alone. -func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, _ Config) []certposture.Proof { +func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, cfg Config) []certposture.Proof { challenges := certificateChallenges(checks) if len(challenges) == 0 { logNoChallenges(checks) @@ -36,17 +33,32 @@ func CollectProofs(ctx context.Context, checks []*proto.Checks, peerKey []byte, // The helper already runs as the signed-in user, and an ordinary process has no // right to a session token, so only the service goes looking for one. - if !runningAsLocalSystem() { + if !runningAsLocalSystem() || cfg.OwnerUnknown { return proofs } - userProofs, err := collectAsDesktopUser(ctx, challenges, peerKey) + userProofs, err := collectAsDesktopUser(ctx, cfg.ProfileOwner, challenges, peerKey) if err != nil { - log.Infof("certificate posture: user certificate store unavailable: %v", err) + log.Debugf("certificate posture: user certificate store unavailable: %v", err) } return mergeProofs(proofs, userProofs) } +// UserContext identifies the session whose store a collection would include: a session +// of the profile owner, or empty when no user store would be asked. A change means a +// collection made earlier no longer reflects what this machine can prove. +func UserContext(cfg Config) string { + if !runningAsLocalSystem() || cfg.OwnerUnknown { + return "" + } + user, ok := CurrentDesktopUser(cfg.ProfileOwner) + if !ok { + return "" + } + defer user.Close() + return fmt.Sprintf("%d:%s", user.Session, user.Name) +} + // helperStore is the store the helper reads. It runs as the signed-in user, so it wants // that user's store rather than the machine store the service already read. func helperStore() Store { @@ -56,8 +68,8 @@ func helperStore() Store { // collectAsDesktopUser runs the helper inside the interactive session of the signed-in // user. Unlike a keychain on macOS, a Windows service can assume a user identity // directly, so the session token goes straight into the child process. -func collectAsDesktopUser(ctx context.Context, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) { - user, ok := CurrentDesktopUser() +func collectAsDesktopUser(ctx context.Context, owner string, challenges []*proto.CertificateChallenge, peerKey []byte) ([]certposture.Proof, error) { + user, ok := CurrentDesktopUser(owner) if !ok { return nil, nil } @@ -68,34 +80,29 @@ func collectAsDesktopUser(ctx context.Context, challenges []*proto.CertificateCh return nil, fmt.Errorf("resolve own binary: %w", err) } - payload, err := json.Marshal(helperRequest(challenges, peerKey)) + // The user's own environment, not the service's: the service environment may carry + // secrets such as a setup key that the signed-in user must not be able to read. + env, err := user.Token.Environ(false) if err != nil { - return nil, fmt.Errorf("encode helper request: %w", err) + return nil, fmt.Errorf("build environment of %s: %w", user.Name, err) } ctx, cancel := context.WithTimeout(ctx, helperTimeout) defer cancel() cmd := exec.CommandContext(ctx, binary, "posture", "cert-proof") + cmd.Env = env cmd.SysProcAttr = &syscall.SysProcAttr{ Token: syscall.Token(user.Token), HideWindow: true, CreationFlags: windows.CREATE_NO_WINDOW, } - cmd.Stdin = bytes.NewReader(payload) - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - log.Infof("certificate posture: asking the session of %q (session %d) to answer %d challenges", user.Name, user.Session, len(challenges)) - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("run helper as %s: %w: %s", user.Name, err, strings.TrimSpace(stderr.String())) + log.Debugf("certificate posture: asking session %d to answer %d challenges", user.Session, len(challenges)) + proofs, err := runHelperCmd(cmd, helperRequest(challenges, peerKey)) + if err != nil { + return nil, fmt.Errorf("run helper in session %d: %w", user.Session, err) } - - var resp HelperResponse - if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { - return nil, fmt.Errorf("decode helper response: %w", err) - } - log.Infof("certificate posture: session of %q returned %d proofs", user.Name, len(resp.Proofs)) - return resp.Proofs, nil + log.Debugf("certificate posture: session %d returned %d proofs", user.Session, len(proofs)) + return proofs, nil } diff --git a/client/internal/certproof/collector.go b/client/internal/certproof/collector.go new file mode 100644 index 000000000..abba4bf61 --- /dev/null +++ b/client/internal/certproof/collector.go @@ -0,0 +1,85 @@ +package certproof + +import ( + "context" + "sync/atomic" + "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/shared/management/certposture" + "github.com/netbirdio/netbird/shared/management/proto" +) + +const collectTimeout = 45 * time.Second + +// collecting is the single-flight flag Collectors share by default. It outlives the +// engine, since a collection abandoned in a token, TPM or keychain call keeps running +// after the engine that started it has stopped, and the next engine must not start +// another one on top of it. +var collecting atomic.Bool + +// Collector runs CollectProofs with a deadline and at most one collection at a time in +// the process. Token, TPM and keychain calls cannot be interrupted, so a collection that +// overruns is abandoned rather than awaited, and a new one is refused until it has +// finished. The zero value is ready to use. +type Collector struct { + // busy overrides the process-wide single-flight flag when set. + busy *atomic.Bool + // timeout overrides collectTimeout when set. + timeout time.Duration +} + +// Collect answers the certificate challenges in checks, returning no proofs when there +// are no challenges, when a previous collection is still running, or when this one does +// not finish in time. Missing proofs fail the certificate check on management. +func (c *Collector) Collect(ctx context.Context, checks []*proto.Checks, peerKey []byte, cfg Config) []certposture.Proof { + return c.collect(ctx, checks, func(ctx context.Context) []certposture.Proof { + return CollectProofs(ctx, checks, peerKey, cfg) + }) +} + +func (c *Collector) collect(ctx context.Context, checks []*proto.Checks, run func(context.Context) []certposture.Proof) []certposture.Proof { + if len(certificateChallenges(checks)) == 0 { + return nil + } + busy := c.flag() + if !busy.CompareAndSwap(false, true) { + log.Warnf("certificate posture: previous proof collection is still running, sending no proofs") + return nil + } + + ctx, cancel := context.WithTimeout(ctx, c.deadline()) + defer cancel() + + done := make(chan []certposture.Proof, 1) + go func() { + // The slot is freed before the result is delivered, so a caller that starts the + // next collection right after this one returned is not turned away. + proofs := run(ctx) + busy.Store(false) + done <- proofs + }() + + select { + case proofs := <-done: + return proofs + case <-ctx.Done(): + log.Warnf("certificate posture: proof collection did not finish within %s, sending no proofs", c.deadline()) + return nil + } +} + +func (c *Collector) flag() *atomic.Bool { + if c.busy != nil { + return c.busy + } + return &collecting +} + +func (c *Collector) deadline() time.Duration { + if c.timeout > 0 { + return c.timeout + } + return collectTimeout +} diff --git a/client/internal/certproof/collector_test.go b/client/internal/certproof/collector_test.go new file mode 100644 index 000000000..d5108eb22 --- /dev/null +++ b/client/internal/certproof/collector_test.go @@ -0,0 +1,126 @@ +package certproof + +import ( + "context" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/shared/management/certposture" + "github.com/netbirdio/netbird/shared/management/proto" +) + +var challengeChecks = []*proto.Checks{{CertificateChallenge: &proto.CertificateChallenge{ + Nonce: certposture.NewChallenger([]byte("secret")).Nonce(peerKey, time.Now()), +}}} + +func TestCollector_SkipsChecksWithoutChallenges(t *testing.T) { + var c Collector + called := false + + proofs := c.collect(context.Background(), []*proto.Checks{{Files: []string{"/bin/agent"}}}, func(context.Context) []certposture.Proof { + called = true + return nil + }) + + assert.Nil(t, proofs) + assert.False(t, called, "no store is touched when no check carries a challenge") +} + +func TestCollector_ReturnsProofs(t *testing.T) { + var c Collector + want := []certposture.Proof{{Nonce: []byte("nonce")}} + + proofs := c.collect(context.Background(), challengeChecks, func(context.Context) []certposture.Proof { return want }) + + assert.Equal(t, want, proofs, "a collection that finishes in time is returned as is") +} + +func TestCollector_AbandonsStuckCollection(t *testing.T) { + c := Collector{timeout: 50 * time.Millisecond, busy: new(atomic.Bool)} + release := make(chan struct{}) + finished := make(chan struct{}) + + // A token or keychain call that ignores its context and blocks well past the deadline. + stuck := func(context.Context) []certposture.Proof { + defer close(finished) + <-release + return []certposture.Proof{{Nonce: []byte("late")}} + } + + start := time.Now() + proofs := c.collect(context.Background(), challengeChecks, stuck) + assert.Nil(t, proofs, "an overrunning collection yields no proofs") + assert.Less(t, time.Since(start), time.Second, "the caller is released at the deadline, not when the call returns") + + called := false + proofs = c.collect(context.Background(), challengeChecks, func(context.Context) []certposture.Proof { + called = true + return nil + }) + assert.Nil(t, proofs) + assert.False(t, called, "no second collection starts while the first is still running") + + close(release) + <-finished + require.Eventually(t, func() bool { return !c.busy.Load() }, time.Second, 5*time.Millisecond, "the collector frees up once the stuck call returns") + + want := []certposture.Proof{{Nonce: []byte("nonce")}} + assert.Equal(t, want, c.collect(context.Background(), challengeChecks, func(context.Context) []certposture.Proof { return want }), + "collection works again after the stuck call returned") +} + +func TestCollector_CancelsContextAtDeadline(t *testing.T) { + c := Collector{timeout: 20 * time.Millisecond, busy: new(atomic.Bool)} + cancelled := make(chan struct{}) + + c.collect(context.Background(), challengeChecks, func(ctx context.Context) []certposture.Proof { + <-ctx.Done() + close(cancelled) + return nil + }) + + select { + case <-cancelled: + case <-time.After(time.Second): + t.Fatal("a collection that honours its context, like the helper process, must see it cancelled") + } +} + +// TestCollector_SingleFlightAcrossCollectors: each engine has its own Collector, and a +// collection stuck in a token call outlives the engine that started it, so the next +// engine's Collector must not start another one until it has finished. +func TestCollector_SingleFlightAcrossCollectors(t *testing.T) { + shared := new(atomic.Bool) + first := Collector{timeout: 20 * time.Millisecond, busy: shared} + second := Collector{timeout: time.Second, busy: shared} + + release := make(chan struct{}) + stuck := func(context.Context) []certposture.Proof { + <-release + return nil + } + assert.Nil(t, first.collect(context.Background(), challengeChecks, stuck), "the first collection is abandoned at its deadline") + + called := false + proofs := second.collect(context.Background(), challengeChecks, func(context.Context) []certposture.Proof { + called = true + return []certposture.Proof{{}} + }) + assert.Nil(t, proofs, "no proofs while the abandoned collection still runs") + assert.False(t, called, "a second collection does not start on top of the abandoned one") + + close(release) + require.Eventually(t, func() bool { return !shared.Load() }, time.Second, 5*time.Millisecond) + assert.Len(t, second.collect(context.Background(), challengeChecks, func(context.Context) []certposture.Proof { return []certposture.Proof{{}} }), 1, + "collections resume once the abandoned one finished") +} + +// TestCollector_ZeroValuesShareTheProcessFlag: the zero value uses the process-wide flag. +func TestCollector_ZeroValuesShareTheProcessFlag(t *testing.T) { + var a, b Collector + assert.Same(t, a.flag(), b.flag(), "separate Collectors share one single-flight flag") +} diff --git a/client/internal/certproof/consoleuser_darwin.go b/client/internal/certproof/consoleuser_darwin.go index f0489d605..a5182c7fd 100644 --- a/client/internal/certproof/consoleuser_darwin.go +++ b/client/internal/certproof/consoleuser_darwin.go @@ -1,8 +1,11 @@ +//go:build !ios + package certproof import ( "bytes" "fmt" + "strconv" "sync" "github.com/ebitengine/purego" @@ -37,21 +40,21 @@ type ConsoleUser struct { // or attributes the session to root, and neither has a login keychain to offer. func CurrentConsoleUser() (ConsoleUser, bool) { if err := loadConsoleUser(); err != nil { - log.Infof("console user lookup unavailable: %v", err) + log.Debugf("console user lookup unavailable: %v", err) return ConsoleUser{}, false } var uid, gid uint32 name := scDynamicStoreCopyConsoleUser(0, &uid, &gid) if name == 0 { - log.Info("no console user is logged in, no login keychain is reachable") + log.Debug("no console user is logged in, no login keychain is reachable") return ConsoleUser{}, false } - defer cfRelease(name) + defer release(name) user := ConsoleUser{Name: cfString(name), UID: uid, GID: gid} if !user.hasDesktop() { - log.Infof("console session belongs to %q uid=%d, which is not a desktop login, no login keychain is reachable", user.Name, user.UID) + log.Debugf("console session belongs to %q uid=%d, which is not a desktop login, no login keychain is reachable", user.Name, user.UID) return ConsoleUser{}, false } return user, true @@ -67,6 +70,13 @@ func (u ConsoleUser) hasDesktop() bool { return u.UID != 0 } +// isOwner reports whether the console user is owner, the account of the active profile, +// which is recorded as a short user name or, for an account without one, a numeric uid. +// With no owner the console user counts, as macOS has a single console user. +func (u ConsoleUser) isOwner(owner string) bool { + return owner == "" || owner == u.Name || owner == strconv.FormatUint(uint64(u.UID), 10) +} + func cfString(str uintptr) string { buf := make([]byte, consoleNameBufSize) if !cfStringGetCString(str, &buf[0], len(buf), encodingUTF8) { diff --git a/client/internal/certproof/consoleuser_darwin_test.go b/client/internal/certproof/consoleuser_darwin_test.go index a3ecba217..9c1aaa22c 100644 --- a/client/internal/certproof/consoleuser_darwin_test.go +++ b/client/internal/certproof/consoleuser_darwin_test.go @@ -1,3 +1,5 @@ +//go:build !ios + package certproof import ( @@ -37,3 +39,13 @@ func TestCurrentConsoleUser_AgreesWithItself(t *testing.T) { assert.NotZero(t, user.UID, "a desktop session never belongs to uid 0") assert.True(t, user.hasDesktop(), "a reported console user must be a desktop session") } + +func TestConsoleUser_IsOwner(t *testing.T) { + user := ConsoleUser{Name: "maycon", UID: 501, GID: 20} + + assert.True(t, user.isOwner(""), "a profile without owner accepts the single console user") + assert.True(t, user.isOwner("maycon"), "the owner by short name") + assert.True(t, user.isOwner("501"), "the owner recorded as a numeric uid") + assert.False(t, user.isOwner("viktor"), "another account's profile must not read this user's keychain") + assert.False(t, user.isOwner("502"), "another uid is another account") +} diff --git a/client/internal/certproof/desktopuser_windows.go b/client/internal/certproof/desktopuser_windows.go index ae6a7221b..6f50aae4f 100644 --- a/client/internal/certproof/desktopuser_windows.go +++ b/client/internal/certproof/desktopuser_windows.go @@ -2,6 +2,7 @@ package certproof import ( "fmt" + "strings" "unsafe" log "github.com/sirupsen/logrus" @@ -10,11 +11,14 @@ import ( const ( noActiveSession = 0xFFFFFFFF + servicesSession = 0 - // wtsCurrentServer is WTS_CURRENT_SERVER_HANDLE and wtsActive is WTSActive of - // WTS_CONNECTSTATE_CLASS. Neither is exported by x/sys/windows. + // wtsCurrentServer is WTS_CURRENT_SERVER_HANDLE; wtsActive and wtsDisconnected are + // WTSActive and WTSDisconnected of WTS_CONNECTSTATE_CLASS. None is exported by + // x/sys/windows. wtsCurrentServer = windows.Handle(0) wtsActive = 0 + wtsDisconnected = 4 ) // DesktopUser is an interactive session and the account signed into it. The user's @@ -33,35 +37,82 @@ func (u DesktopUser) Close() { } } -// CurrentDesktopUser returns a token for the interactive user whose certificate store -// should be asked. The physical console comes first, and an active remote desktop -// session is used when nobody is at the console, which is how servers and VDI hosts are -// normally reached. The second return is false at the sign-in screen, where no -// interactive session exists and only machine certificates can be proven. +// CurrentDesktopUser returns a token for the session whose certificate store should be +// asked: a session of owner, the account the active profile belongs to, with the +// physical console preferred over remote sessions. With no owner only the console user +// counts. Picking any signed-in user instead would let whoever else is logged in to a +// terminal server or VDI host decide the result. The second return is false when no +// such session exists, and only machine certificates can then be proven. // // Obtaining the token needs SE_TCB_NAME, which the LocalSystem service has and an // ordinary process does not. -func CurrentDesktopUser() (DesktopUser, bool) { - if session := windows.WTSGetActiveConsoleSessionId(); session != noActiveSession { - if user, ok := desktopUser(session); ok { - return user, true - } - log.Infof("console session %d has nobody signed in, looking for an active remote session", session) +func CurrentDesktopUser(owner string) (DesktopUser, bool) { + console := windows.WTSGetActiveConsoleSessionId() + if owner == "" { + return consoleUser(console) } - sessions, err := activeSessions() + sessions, err := userSessions(console) if err != nil { - log.Infof("cannot enumerate terminal sessions: %v", err) + log.Debugf("cannot enumerate terminal sessions: %v", err) return DesktopUser{}, false } + var found DesktopUser + var ok bool for _, session := range sessions { - if user, ok := desktopUser(session); ok { - return user, true + user, signedIn := desktopUser(session) + if !signedIn { + continue + } + switch { + case !sameAccountName(user.Name, owner): + user.Close() + case !ok: + found, ok = user, true + case strings.EqualFold(user.Name, found.Name): + // Another session of the same account; the first one in preference order wins. + user.Close() + default: + // An owner recorded without a domain matches accounts of several domains here. + // Picking one would let another domain's user answer for the owner. + log.Debugf("certificate posture: profile owner %s matches both %s and %s, no user certificate store is used", owner, found.Name, user.Name) + user.Close() + found.Close() + return DesktopUser{}, false } } + if !ok { + log.Debugf("certificate posture: profile owner %s has no signed-in session, no user certificate store is reachable", owner) + } + return found, ok +} - log.Info("no interactive session is signed in, no user certificate store is reachable") - return DesktopUser{}, false +func consoleUser(console uint32) (DesktopUser, bool) { + if console == noActiveSession || console == servicesSession { + log.Debug("no console session, no user certificate store is reachable") + return DesktopUser{}, false + } + user, ok := desktopUser(console) + if !ok { + log.Debugf("console session %d has nobody signed in, no user certificate store is reachable", console) + } + return user, ok +} + +// sameAccountName compares DOMAIN\account names case-insensitively, as Windows does, and an +// owner given without a domain against the account part alone. The session side comes from +// the session token's own SID, which Windows resolves from its cache of signed-in users. +// Resolving the owner name to a SID instead would ask the domain controller, which on a +// laptop that cannot reach it yet blocks for tens of seconds, past the collection deadline. +func sameAccountName(sessionName, owner string) bool { + if strings.EqualFold(sessionName, owner) { + return true + } + if strings.Contains(owner, `\`) { + return false + } + _, account, found := strings.Cut(sessionName, `\`) + return found && strings.EqualFold(account, owner) } func desktopUser(session uint32) (DesktopUser, bool) { @@ -73,7 +124,7 @@ func desktopUser(session uint32) (DesktopUser, bool) { name, err := tokenAccount(token) if err != nil { - log.Infof("session %d token has no readable account: %v", session, err) + log.Debugf("session %d token has no readable account: %v", session, err) if closeErr := token.Close(); closeErr != nil { log.Debugf("failed closing session token: %v", closeErr) } @@ -97,7 +148,10 @@ func tokenAccount(token windows.Token) (string, error) { return domain + `\` + account, nil } -func activeSessions() ([]uint32, error) { +// userSessions lists the sessions a user can be signed in to, console first, then +// active remote sessions, then disconnected ones, whose user is still signed in. Session +// 0 is skipped: it hosts services and never belongs to an interactive user. +func userSessions(console uint32) ([]uint32, error) { var info *windows.WTS_SESSION_INFO var count uint32 if err := windows.WTSEnumerateSessions(wtsCurrentServer, 0, 1, &info, &count); err != nil { @@ -105,13 +159,19 @@ func activeSessions() ([]uint32, error) { } defer windows.WTSFreeMemory(uintptr(unsafe.Pointer(info))) - var active []uint32 - for _, session := range unsafe.Slice(info, count) { - if session.State == wtsActive { - active = append(active, session.SessionID) + var sessions []uint32 + if console != noActiveSession && console != servicesSession { + sessions = append(sessions, console) + } + for _, state := range []uint32{wtsActive, wtsDisconnected} { + for _, session := range unsafe.Slice(info, count) { + if session.SessionID == servicesSession || session.SessionID == console || session.State != state { + continue + } + sessions = append(sessions, session.SessionID) } } - return active, nil + return sessions, nil } // runningAsLocalSystem reports whether this process is the service. The helper runs as diff --git a/client/internal/certproof/desktopuser_windows_test.go b/client/internal/certproof/desktopuser_windows_test.go new file mode 100644 index 000000000..95d29eb5f --- /dev/null +++ b/client/internal/certproof/desktopuser_windows_test.go @@ -0,0 +1,61 @@ +package certproof + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSameAccountName(t *testing.T) { + tests := []struct { + session, owner string + want bool + }{ + {`CORP\alice`, `CORP\alice`, true}, + {`CORP\alice`, `corp\ALICE`, true}, + {`CORP\alice`, `alice`, true}, + {`CORP\alice`, `OTHER\alice`, false}, + {`CORP\alice`, `bob`, false}, + {`alice`, `alice`, true}, + {`CORP\alice`, `CORP\alic`, false}, + } + for _, tt := range tests { + assert.Equal(t, tt.want, sameAccountName(tt.session, tt.owner), "session %q, owner %q", tt.session, tt.owner) + } +} + +// CurrentDesktopUser against the real session manager: an owner no session belongs to +// must never yield a session, whoever else is signed in. +func TestCurrentDesktopUser_UnknownOwnerHasNoSession(t *testing.T) { + user, ok := CurrentDesktopUser(`NO-SUCH-DOMAIN\no-such-user-netbird`) + if ok { + user.Close() + } + assert.False(t, ok, "a profile owner without a session must not borrow another user's store") +} + +// TestCurrentDesktopUser_FindsTheOwnersSession runs against a real machine as LocalSystem, +// with NB_TEST_DESKTOP_OWNER naming an account that is signed in. That account's session +// must be found, and only that account's. +func TestCurrentDesktopUser_FindsTheOwnersSession(t *testing.T) { + owner := os.Getenv("NB_TEST_DESKTOP_OWNER") + if owner == "" { + t.Skip("set NB_TEST_DESKTOP_OWNER to a signed-in account and run as LocalSystem") + } + user, ok := CurrentDesktopUser(owner) + require.True(t, ok, "the signed-in owner %s must have a session", owner) + defer user.Close() + t.Logf("owner %s resolved to session %d as %s", owner, user.Session, user.Name) + assert.True(t, sameAccountName(user.Name, owner), "the session found belongs to %s, not %s", owner, user.Name) + assert.NotZero(t, user.Session, "session 0 hosts services and never belongs to the owner") + + console, consoleOK := CurrentDesktopUser("") + if consoleOK { + defer console.Close() + t.Logf("without an owner the console user counts: session %d as %s", console.Session, console.Name) + } else { + t.Log("without an owner nobody counts: no user at the console") + } +} diff --git a/client/internal/certproof/doc.go b/client/internal/certproof/doc.go new file mode 100644 index 000000000..c8dc62f0c --- /dev/null +++ b/client/internal/certproof/doc.go @@ -0,0 +1,26 @@ +// Package certproof answers certificate posture challenges: it finds the certificates a +// peer holds a private key for and signs the challenge nonce, bound to the peer's +// WireGuard key, with that key. Management verifies the signature and the chain against +// the CAs of the check, so only a holder of the key passes and a proof made for one peer +// cannot be replayed by another. Keys in the OS stores, the TPM and a PKCS#11 token are +// used through the platform, which signs; a plain PEM key file is the exception, parsed +// and used in the daemon's memory. +// +// Machine stores are read by the daemon itself: the Windows LocalMachine store, the macOS +// System keychain, and on Linux a directory of PEM files, TSS2 key files the TPM signs +// with, and a PKCS#11 token. On Linux the directory and the token URI come from the +// daemon's environment (NB_CERT_STORE_DIR, NB_CERT_PKCS11_URI), as does the token PIN +// (NB_TPM_PIN); none of them is read from the profile config. +// +// User stores cannot be read by a privileged daemon, so it starts this binary as +// "netbird posture cert-proof" inside the user's session and receives only signatures +// and chains on its stdout. On macOS uid is not what unlocks a login keychain, the +// session's bootstrap namespace is, which is why the helper enters it through launchctl +// asuser. On Windows a service opening CURRENT_USER silently reads its own empty hive, so +// the helper runs with the session's token instead. Its output is untrusted: sizes are +// capped and only proofs for requested nonces are kept. +// +// A user proof shows that some process in that user's session could use a key whose +// certificate chains to the CA. It does not show which binary signed, or that the key is +// hardware-bound; device stores are the stronger signal. +package certproof diff --git a/client/internal/certproof/helper.go b/client/internal/certproof/helper.go index 26d719044..81a72ac70 100644 --- a/client/internal/certproof/helper.go +++ b/client/internal/certproof/helper.go @@ -57,7 +57,7 @@ func runHelper(ctx context.Context, store Store, in io.Reader, out io.Writer) er if len(challenges) > 0 { proofs = CollectChallenges(ctx, store, challenges, req.PeerKey) } - log.Infof("certificate posture helper: answering %d challenges with %d proofs", len(challenges), len(proofs)) + log.Debugf("certificate posture helper: answering %d challenges with %d proofs", len(challenges), len(proofs)) if err := json.NewEncoder(out).Encode(HelperResponse{Proofs: proofs}); err != nil { return fmt.Errorf("encode helper response: %w", err) diff --git a/client/internal/certproof/helper_backoff.go b/client/internal/certproof/helper_backoff.go new file mode 100644 index 000000000..3372b026b --- /dev/null +++ b/client/internal/certproof/helper_backoff.go @@ -0,0 +1,78 @@ +package certproof + +import ( + "crypto/sha256" + "encoding/hex" + "slices" + "strings" + "sync" + "time" + + "github.com/netbirdio/netbird/shared/management/proto" +) + +// helperQuietPeriod is how long a user whose helper proved nothing is not asked again +// for the same CAs. On macOS each helper run may show a keychain prompt, so retrying on +// every collection would put up a new one each time the user ignored or denied the last. +const helperQuietPeriod = time.Hour + +// helperBackoff remembers, per user and set of CAs asked about, until when the helper +// is not launched again because its last run proved nothing. +type helperBackoff struct { + mu sync.Mutex + until map[string]time.Time +} + +func newHelperBackoff() *helperBackoff { + return &helperBackoff{until: map[string]time.Time{}} +} + +// allow reports whether the helper may be launched for key now. +func (b *helperBackoff) allow(key string, now time.Time) bool { + b.mu.Lock() + defer b.mu.Unlock() + return !now.Before(b.until[key]) +} + +// record stores the outcome of a helper run for key: one that proved something clears +// the back-off, one that proved nothing starts it. Expired entries are dropped, so the +// map holds only users currently held off. +func (b *helperBackoff) record(key string, proven bool, now time.Time) { + b.mu.Lock() + defer b.mu.Unlock() + for other, until := range b.until { + if !now.Before(until) { + delete(b.until, other) + } + } + if proven { + delete(b.until, key) + return + } + b.until[key] = now.Add(helperQuietPeriod) +} + +// helperBackoffKey identifies a user and the set of CAs the challenges accept, in any +// order, leaving out the nonces, which rotate without changing what the user is asked to +// prove. +func helperBackoffKey(user string, challenges []*proto.CertificateChallenge) string { + sets := make([]string, 0, len(challenges)) + for _, challenge := range challenges { + cas := make([]string, 0, len(challenge.GetCaCertificates())) + for _, ca := range challenge.GetCaCertificates() { + sum := sha256.Sum256([]byte(ca)) + cas = append(cas, hex.EncodeToString(sum[:])) + } + slices.Sort(cas) + sets = append(sets, strings.Join(cas, ",")) + } + slices.Sort(sets) + + h := sha256.New() + h.Write([]byte(user)) + for _, set := range sets { + h.Write([]byte{0}) + h.Write([]byte(set)) + } + return hex.EncodeToString(h.Sum(nil)) +} diff --git a/client/internal/certproof/helper_backoff_test.go b/client/internal/certproof/helper_backoff_test.go new file mode 100644 index 000000000..0d3cafea1 --- /dev/null +++ b/client/internal/certproof/helper_backoff_test.go @@ -0,0 +1,69 @@ +package certproof + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "github.com/netbirdio/netbird/shared/management/proto" +) + +// TestHelperBackoff: a user whose helper proved nothing, for one ignoring or denying a +// keychain prompt, is not asked again for the same CAs within the quiet period, while a +// success or other CAs are asked at once. +func TestHelperBackoff(t *testing.T) { + now := time.Now() + b := newHelperBackoff() + key := helperBackoffKey("501", []*proto.CertificateChallenge{{CaCertificates: []string{"ca-a"}}}) + + assert.True(t, b.allow(key, now), "a user never asked is asked") + + b.record(key, false, now) + assert.False(t, b.allow(key, now.Add(helperQuietPeriod-time.Second)), "no new prompt within the quiet period") + assert.True(t, b.allow(key, now.Add(helperQuietPeriod)), "asked again once the quiet period passed") + + other := helperBackoffKey("501", []*proto.CertificateChallenge{{CaCertificates: []string{"ca-b"}}}) + assert.True(t, b.allow(other, now), "a check for other CAs is asked at once") + assert.True(t, b.allow(helperBackoffKey("502", []*proto.CertificateChallenge{{CaCertificates: []string{"ca-a"}}}), now), "another user is asked at once") + + b.record(key, true, now) + assert.True(t, b.allow(key, now), "a run that proved something clears the quiet period") +} + +// TestHelperBackoffKey_IgnoresNonces: nonces rotate every window without changing what +// the user is asked to prove, so they do not reset the quiet period. +func TestHelperBackoffKey_IgnoresNonces(t *testing.T) { + a := helperBackoffKey("501", []*proto.CertificateChallenge{{Nonce: []byte("one"), CaCertificates: []string{"ca"}}}) + b := helperBackoffKey("501", []*proto.CertificateChallenge{{Nonce: []byte("two"), CaCertificates: []string{"ca"}}}) + assert.Equal(t, a, b, "the key does not depend on the nonce") +} + +// TestHelperBackoffKey_IgnoresOrder: the same CAs, listed in another order or with the +// challenges reordered, are the same question to the user. +func TestHelperBackoffKey_IgnoresOrder(t *testing.T) { + a := helperBackoffKey("501", []*proto.CertificateChallenge{ + {CaCertificates: []string{"ca-1", "ca-2"}}, + {CaCertificates: []string{"ca-3"}}, + }) + b := helperBackoffKey("501", []*proto.CertificateChallenge{ + {CaCertificates: []string{"ca-3"}}, + {CaCertificates: []string{"ca-2", "ca-1"}}, + }) + assert.Equal(t, a, b, "the key does not depend on the order of CAs or challenges") + + c := helperBackoffKey("501", []*proto.CertificateChallenge{{CaCertificates: []string{"ca-1", "ca-2", "ca-3"}}}) + assert.NotEqual(t, a, c, "the same CAs grouped into other challenges are another question") +} + +// TestHelperBackoff_PrunesExpired: entries whose quiet period passed are dropped when the +// next outcome is recorded, so the map does not grow with every user and CA set seen. +func TestHelperBackoff_PrunesExpired(t *testing.T) { + now := time.Now() + b := newHelperBackoff() + b.record("old", false, now) + b.record("new", false, now.Add(helperQuietPeriod)) + + assert.NotContains(t, b.until, "old", "an expired entry is pruned") + assert.Contains(t, b.until, "new", "a current entry is kept") +} diff --git a/client/internal/certproof/helper_kill_other.go b/client/internal/certproof/helper_kill_other.go new file mode 100644 index 000000000..2ececb27e --- /dev/null +++ b/client/internal/certproof/helper_kill_other.go @@ -0,0 +1,10 @@ +//go:build !unix && !windows + +package certproof + +import "os/exec" + +// startHelper starts cmd. +func startHelper(cmd *exec.Cmd) (func(), error) { + return func() {}, cmd.Start() +} diff --git a/client/internal/certproof/helper_kill_unix.go b/client/internal/certproof/helper_kill_unix.go new file mode 100644 index 000000000..e434d4962 --- /dev/null +++ b/client/internal/certproof/helper_kill_unix.go @@ -0,0 +1,35 @@ +//go:build unix + +package certproof + +import ( + "errors" + "os/exec" + "syscall" +) + +// killHelperGroupOnCancel puts cmd in a process group of its own and kills the whole +// group when cmd's context ends. The helper may run below launchers such as launchctl +// and sudo, which do not pass a kill on, and it keeps the output pipes open, so killing +// only the direct child would leave the helper running and Wait blocked until the +// helper exits on its own, for one waiting on a keychain prompt nobody answers. +func killHelperGroupOnCancel(cmd *exec.Cmd) { + if cmd.SysProcAttr == nil { + cmd.SysProcAttr = &syscall.SysProcAttr{} + } + cmd.SysProcAttr.Setpgid = true + cmd.Cancel = func() error { + err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + if errors.Is(err, syscall.ESRCH) { + return nil + } + return err + } + cmd.WaitDelay = helperWaitDelay +} + +// startHelper starts cmd. Killing the helper with everything below it is arranged by +// killHelperGroupOnCancel where the helper runs under a launcher. +func startHelper(cmd *exec.Cmd) (func(), error) { + return func() {}, cmd.Start() +} diff --git a/client/internal/certproof/helper_kill_windows.go b/client/internal/certproof/helper_kill_windows.go new file mode 100644 index 000000000..1e143c132 --- /dev/null +++ b/client/internal/certproof/helper_kill_windows.go @@ -0,0 +1,77 @@ +//go:build windows + +package certproof + +import ( + "fmt" + "os/exec" + "unsafe" + + log "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" +) + +// startHelper starts cmd inside a job object that is terminated when cmd's context ends +// and closed, killing whatever is left in it, once the helper has been awaited. Killing +// only the helper on Windows leaves its descendants running, and one that holds the +// output pipe would block Wait until it exits, so WaitDelay bounds that wait as well. +func startHelper(cmd *exec.Cmd) (func(), error) { + job, err := newKillOnCloseJob() + if err != nil { + return nil, err + } + closeJob := func() { + if err := windows.CloseHandle(job); err != nil { + log.Debugf("failed to close certificate proof helper job: %v", err) + } + } + + cmd.Cancel = func() error { + return windows.TerminateJobObject(job, 1) + } + cmd.WaitDelay = helperWaitDelay + if err := cmd.Start(); err != nil { + closeJob() + return nil, err + } + + // A helper outside the job is still killed directly and bounded by WaitDelay; only + // its descendants would outlive it. + if err := assignToJob(job, cmd.Process.Pid); err != nil { + log.Debugf("failed to put certificate proof helper %d in its job: %v", cmd.Process.Pid, err) + cmd.Cancel = func() error { return cmd.Process.Kill() } + } + return closeJob, nil +} + +// newKillOnCloseJob creates a job object whose processes are killed when its last handle +// is closed. +func newKillOnCloseJob() (windows.Handle, error) { + job, err := windows.CreateJobObject(nil, nil) + if err != nil { + return 0, fmt.Errorf("create job object: %w", err) + } + info := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ + BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ + LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, + } + if _, err := windows.SetInformationJobObject(job, windows.JobObjectExtendedLimitInformation, + uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil { + _ = windows.CloseHandle(job) + return 0, fmt.Errorf("set job object limits: %w", err) + } + return job, nil +} + +// assignToJob puts the process pid into job. +func assignToJob(job windows.Handle, pid int) error { + process, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE, false, uint32(pid)) + if err != nil { + return fmt.Errorf("open process: %w", err) + } + defer func() { + _ = windows.CloseHandle(process) + }() + return windows.AssignProcessToJobObject(job, process) +} diff --git a/client/internal/certproof/helper_kill_windows_test.go b/client/internal/certproof/helper_kill_windows_test.go new file mode 100644 index 000000000..4778d253b --- /dev/null +++ b/client/internal/certproof/helper_kill_windows_test.go @@ -0,0 +1,29 @@ +//go:build windows + +package certproof + +import ( + "context" + "os/exec" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestRunHelperCmd_KillsHelperTree: the helper's child holds stdout and runs for a minute, +// like a helper stuck below a launcher. Killing only the helper would leave Wait blocked +// on the child's pipe until WaitDelay gives up; terminating the job ends both at once. +func TestRunHelperCmd_KillsHelperTree(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + cmd := exec.CommandContext(ctx, "cmd.exe", "/c", "ping -n 60 127.0.0.1") + + start := time.Now() + _, err := runHelperCmd(cmd, HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}}) + + require.Error(t, err) + assert.Less(t, time.Since(start), helperWaitDelay, "the whole tree is killed at the timeout, not left for WaitDelay") +} diff --git a/client/internal/certproof/helper_run.go b/client/internal/certproof/helper_run.go new file mode 100644 index 000000000..52c9db775 --- /dev/null +++ b/client/internal/certproof/helper_run.go @@ -0,0 +1,126 @@ +package certproof + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os/exec" + "strings" + "time" + + log "github.com/sirupsen/logrus" + + "github.com/netbirdio/netbird/shared/management/certposture" +) + +const ( + maxHelperStdout = 1 << 20 + maxHelperStderr = 4 << 10 + + // helperWaitDelay bounds how long a helper's output is awaited after it was killed. + helperWaitDelay = 2 * time.Second +) + +var errHelperOutputTooLarge = errors.New("helper output exceeds the size limit") + +// runHelperCmd feeds req to the helper process cmd and returns the proofs it answered. +// The helper runs as an unprivileged user who can control its output, so both streams +// are capped and only proofs for a nonce req asked about are kept, one per challenge. +func runHelperCmd(cmd *exec.Cmd, req HelperRequest) ([]certposture.Proof, error) { + payload, err := json.Marshal(req) + if err != nil { + return nil, fmt.Errorf("encode helper request: %w", err) + } + + stdout := &cappedBuffer{limit: maxHelperStdout} + stderr := &cappedBuffer{limit: maxHelperStderr} + cmd.Stdin = bytes.NewReader(payload) + cmd.Stdout = stdout + cmd.Stderr = stderr + + release, err := startHelper(cmd) + if err != nil { + return nil, fmt.Errorf("start helper: %w", err) + } + defer release() + if err := cmd.Wait(); err != nil { + return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) + } + logHelperStderr(stderr.String()) + if stdout.truncated { + return nil, errHelperOutputTooLarge + } + + var resp HelperResponse + if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { + return nil, fmt.Errorf("decode helper response: %w", err) + } + return requestedProofs(req, resp.Proofs), nil +} + +// logHelperStderr records what a helper that succeeded wrote to stderr, such as a user +// certificate it could not sign with, which is otherwise only visible in the user's +// session. Each line is quoted because the helper's user controls its content. +func logHelperStderr(stderr string) { + for _, line := range strings.Split(strings.TrimSpace(stderr), "\n") { + if line = strings.TrimSpace(line); line != "" { + log.Debugf("certificate posture helper: %q", line) + } + } +} + +// requestedProofs keeps the proofs whose nonce belongs to one of req's challenges, at +// most as many as req has challenges. +func requestedProofs(req HelperRequest, proofs []certposture.Proof) []certposture.Proof { + var kept []certposture.Proof + for _, proof := range proofs { + if len(kept) == len(req.Challenges) { + break + } + if !req.asked(proof.Nonce) { + log.Debugf("certificate posture: dropping helper proof for a nonce that was not requested") + continue + } + kept = append(kept, proof) + } + return kept +} + +func (r HelperRequest) asked(nonce []byte) bool { + for _, challenge := range r.Challenges { + if len(challenge.Nonce) > 0 && bytes.Equal(challenge.Nonce, nonce) { + return true + } + } + return false +} + +// cappedBuffer keeps the first limit bytes written to it and discards the rest, so a +// misbehaving child cannot grow the parent's memory without bound. The buffer is a +// named field rather than embedded: an embedded bytes.Buffer would promote ReadFrom, +// which io.Copy prefers over Write, bypassing the cap. +type cappedBuffer struct { + buf bytes.Buffer + limit int + truncated bool +} + +func (b *cappedBuffer) Write(p []byte) (int, error) { + if room := b.limit - b.buf.Len(); room < len(p) { + b.truncated = true + if room > 0 { + b.buf.Write(p[:room]) + } + return len(p), nil + } + return b.buf.Write(p) +} + +func (b *cappedBuffer) Bytes() []byte { + return b.buf.Bytes() +} + +func (b *cappedBuffer) String() string { + return b.buf.String() +} diff --git a/client/internal/certproof/helper_run_test.go b/client/internal/certproof/helper_run_test.go new file mode 100644 index 000000000..ea1246e93 --- /dev/null +++ b/client/internal/certproof/helper_run_test.go @@ -0,0 +1,119 @@ +//go:build !windows && !js + +package certproof + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "strings" + "testing" + "time" + + log "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/shared/management/certposture" +) + +// fakeHelper is a child process that drains its stdin and then prints script's output, +// standing in for a helper running in a user session the daemon cannot trust. +func fakeHelper(t *testing.T, script string) *exec.Cmd { + t.Helper() + return exec.Command("/bin/sh", "-c", "cat >/dev/null; "+script) +} + +func printJSON(t *testing.T, v any) string { + t.Helper() + out, err := json.Marshal(v) + require.NoError(t, err) + return fmt.Sprintf("printf '%%s' '%s'", out) +} + +func TestRunHelperCmd_KeepsOnlyRequestedProofs(t *testing.T) { + req := HelperRequest{PeerKey: peerKey, Challenges: []HelperChallenge{{Nonce: []byte("asked")}}} + resp := HelperResponse{Proofs: []certposture.Proof{ + {Nonce: []byte("injected"), Signature: []byte("x")}, + {Nonce: []byte("asked"), Signature: []byte("first")}, + {Nonce: []byte("asked"), Signature: []byte("second")}, + }} + + proofs, err := runHelperCmd(fakeHelper(t, printJSON(t, resp)), req) + + require.NoError(t, err) + require.Len(t, proofs, 1, "a helper may not return more proofs than challenges or proofs for nonces it was not asked about") + assert.Equal(t, []byte("first"), proofs[0].Signature, "the first proof for the requested nonce is kept") +} + +func TestRunHelperCmd_RejectsOversizedOutput(t *testing.T) { + req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}} + script := fmt.Sprintf("head -c %d /dev/zero", maxHelperStdout+1) + + _, err := runHelperCmd(fakeHelper(t, script), req) + + assert.ErrorIs(t, err, errHelperOutputTooLarge) +} + +func TestRunHelperCmd_CapsStderrInError(t *testing.T) { + req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}} + script := fmt.Sprintf("head -c %d /dev/zero | tr '\\0' 'a' >&2; exit 3", 10*maxHelperStderr) + + _, err := runHelperCmd(fakeHelper(t, script), req) + + require.Error(t, err) + assert.LessOrEqual(t, len(err.Error()), maxHelperStderr+100, "a chatty helper must not blow up the daemon's error or log line") + assert.True(t, strings.Contains(err.Error(), "exit status 3"), "the exit status is kept: %v", err) +} + +// TestRunHelperCmd_LogsStderrOfSuccessfulHelper covers a helper that answers but warns, +// for one about a certificate it could not sign with: the warning reaches the daemon's +// log, quoted so a helper cannot forge log lines with embedded newlines. +func TestRunHelperCmd_LogsStderrOfSuccessfulHelper(t *testing.T) { + var logged bytes.Buffer + log.SetOutput(&logged) + level := log.GetLevel() + log.SetLevel(log.DebugLevel) + t.Cleanup(func() { + log.SetOutput(os.Stderr) + log.SetLevel(level) + }) + + req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}} + script := "printf 'failed signing certificate proof for CN=user\\nforged line\\n' >&2; " + printJSON(t, HelperResponse{}) + + _, err := runHelperCmd(fakeHelper(t, script), req) + + require.NoError(t, err) + assert.Contains(t, logged.String(), "failed signing certificate proof for CN=user", "the helper's warning reaches the daemon log") + assert.NotContains(t, logged.String(), "\nforged line", "helper output cannot start a log line of its own") +} + +// TestRunHelperCmd_KillsHelperBelowLauncher stands in for launchctl and sudo starting the +// helper: the direct child spawns a grandchild that holds stdout and never exits, like a +// helper waiting on a keychain prompt. The timeout must end both, not wait for the +// grandchild. +func TestRunHelperCmd_KillsHelperBelowLauncher(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + cmd := exec.CommandContext(ctx, "/bin/sh", "-c", "sleep 30 & cat >/dev/null; wait") + killHelperGroupOnCancel(cmd) + + start := time.Now() + _, err := runHelperCmd(cmd, HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}}) + + require.Error(t, err) + assert.Less(t, time.Since(start), 5*time.Second, "the helper is killed at the timeout, not awaited until it exits") +} + +func TestRunHelperCmd_RejectsGarbage(t *testing.T) { + req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}} + + _, err := runHelperCmd(fakeHelper(t, "printf 'not json'"), req) + + assert.ErrorContains(t, err, "decode helper response") +} diff --git a/client/internal/certproof/helper_spawn.go b/client/internal/certproof/helper_spawn.go index ccf0567a5..968cc3679 100644 --- a/client/internal/certproof/helper_spawn.go +++ b/client/internal/certproof/helper_spawn.go @@ -1,4 +1,4 @@ -//go:build darwin || windows +//go:build (darwin && !ios) || windows package certproof @@ -56,8 +56,8 @@ func mergeProofs(device, user []certposture.Proof) []certposture.Proof { func logUserProof(proof certposture.Proof) { leaf, err := x509.ParseCertificate(proof.Chain[0]) if err != nil { - log.Infof("certificate posture: user proof carries an unparsable leaf: %v", err) + log.Debugf("certificate posture: user proof carries an unparsable leaf: %v", err) return } - log.Infof("certificate posture: signed-in user proved %q issued by %q", leaf.Subject, leaf.Issuer) + log.Debugf("certificate posture: signed-in user proved %q issued by %q", leaf.Subject, leaf.Issuer) } diff --git a/client/internal/certproof/helper_spawn_test.go b/client/internal/certproof/helper_spawn_test.go index 5356a9b00..338a5f66a 100644 --- a/client/internal/certproof/helper_spawn_test.go +++ b/client/internal/certproof/helper_spawn_test.go @@ -1,4 +1,4 @@ -//go:build darwin || windows +//go:build (darwin && !ios) || windows package certproof diff --git a/client/internal/certproof/helper_test.go b/client/internal/certproof/helper_test.go index b70bd71f3..37dadead2 100644 --- a/client/internal/certproof/helper_test.go +++ b/client/internal/certproof/helper_test.go @@ -16,7 +16,7 @@ import ( func TestRunHelper_ProofSurvivesTheProcessBoundary(t *testing.T) { ca := certtest.NewCA(t, "corp-root") - dir := t.TempDir() + dir := storeDir(t) key := certtest.ECDSAKey(t) writeFile(t, dir, "device.pem", certtest.CertPEM(ca.Issue(t, key, "device"))+certtest.KeyPEM(t, key)) @@ -47,7 +47,7 @@ func TestRunHelper_NoChallengesYieldsEmptyResponse(t *testing.T) { require.NoError(t, err) var stdout bytes.Buffer - require.NoError(t, runHelper(context.Background(), NewFileStore(t.TempDir()), bytes.NewReader(request), &stdout)) + require.NoError(t, runHelper(context.Background(), NewFileStore(storeDir(t)), bytes.NewReader(request), &stdout)) var resp HelperResponse require.NoError(t, json.Unmarshal(stdout.Bytes(), &resp), "an empty request must still emit valid JSON") @@ -56,7 +56,7 @@ func TestRunHelper_NoChallengesYieldsEmptyResponse(t *testing.T) { func TestRunHelper_RejectsMalformedRequest(t *testing.T) { var stdout bytes.Buffer - err := runHelper(context.Background(), NewFileStore(t.TempDir()), bytes.NewReader([]byte("not json")), &stdout) + err := runHelper(context.Background(), NewFileStore(storeDir(t)), bytes.NewReader([]byte("not json")), &stdout) require.Error(t, err, "a malformed request must fail rather than emit an empty proof set") assert.Empty(t, stdout.String(), "nothing should be written to stdout on a decode failure") diff --git a/client/internal/certproof/keychain_darwin.go b/client/internal/certproof/keychain_darwin.go index d53c15b27..2bffdc81e 100644 --- a/client/internal/certproof/keychain_darwin.go +++ b/client/internal/certproof/keychain_darwin.go @@ -1,3 +1,5 @@ +//go:build !ios + package certproof import ( @@ -20,9 +22,17 @@ const ( securityFramework = "/System/Library/Frameworks/Security.framework/Security" coreFoundationFramework = "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation" - errSecItemNotFound = -25300 + errSecItemNotFound = -25300 + errSecInteractionNotAllowed = -25308 ) +// errKeyNeedsApproval reports a key whose access list does not include netbird, so using +// it needs the user's approval, which a daemon has no UI to ask for. +var errKeyNeedsApproval = errors.New("the key's access control requires user approval for netbird; " + + "import the identity with netbird allowed (security import -k " + + "-T /Applications/NetBird.app/Contents/MacOS/netbird), " + + "or set AllowAllAppsAccess in the MDM certificate payload, which allows every application") + var ( keychainOnce sync.Once keychainErr error @@ -33,8 +43,10 @@ var ( secCertificateCopyData func(cert uintptr) uintptr secKeyCreateSignature func(key, algorithm, data uintptr, err *uintptr) uintptr + secKeychainOpen func(path *byte, keychain *uintptr) int32 secKeychainCopySearchList func(searchList *uintptr) int32 secKeychainGetPath func(keychain uintptr, pathLength *uint32, path *byte) int32 + cfArrayCreate func(alloc uintptr, values *uintptr, count int, callBacks uintptr) uintptr cfDictionaryCreate func(alloc uintptr, keys, values *uintptr, count int, keyCallBacks, valueCallBacks uintptr) uintptr cfArrayGetCount func(array uintptr) int @@ -45,37 +57,56 @@ var ( cfErrorGetCode func(err uintptr) int cfRelease func(ref uintptr) - kSecClass, kSecClassIdentity, kSecClassCertificate, kSecMatchLimit, kSecMatchLimitAll, kSecReturnRef uintptr - kSecKeyAlgorithmECDSASHA256, kSecKeyAlgorithmECDSASHA384, kSecKeyAlgorithmRSAPSSSHA256 uintptr - kCFBooleanTrue, kCFTypeDictionaryKeyCallBacks, kCFTypeDictionaryValueCallBacks uintptr + kSecClass, kSecClassIdentity, kSecClassCertificate, kSecMatchLimit, kSecMatchLimitAll, kSecReturnRef uintptr + kSecMatchSearchList uintptr + kSecKeyAlgorithmECDSASHA256, kSecKeyAlgorithmECDSASHA384, kSecKeyAlgorithmRSAPSSSHA256 uintptr + kCFBooleanTrue, kCFTypeDictionaryKeyCallBacks, kCFTypeDictionaryValueCallBacks, kCFTypeArrayCallBacks uintptr ) -// DefaultStore is the keychain search list of the daemon, which for the root daemon is -// the System keychain where MDM installs device identities. +// systemKeychain is where MDM installs device identities. +const systemKeychain = "/Library/Keychains/System.keychain" + +// DefaultStore is the System keychain for the root daemon, so it never touches a user's +// keychain, whose keys would ask for approval in a session the daemon has no UI in. Run +// by an ordinary user, it is that user's keychain search list. func DefaultStore() Store { + if os.Geteuid() == 0 { + return NewKeychainStore(systemKeychain) + } return NewKeychainStore() } -// KeychainStore yields the identities of the process's keychain search list, reached -// through purego so the client keeps building with CGO_ENABLED=0. -type KeychainStore struct{} +// KeychainStore yields the identities of a set of keychains, or of the process's keychain +// search list when none is named, reached through purego so the client keeps building +// with CGO_ENABLED=0. +type KeychainStore struct { + keychains []string +} -func NewKeychainStore() *KeychainStore { - return &KeychainStore{} +// NewKeychainStore returns a store that searches the keychain files at paths, or the +// process's keychain search list when no path is given. +func NewKeychainStore(paths ...string) *KeychainStore { + return &KeychainStore{keychains: paths} } func (s *KeychainStore) Candidates(_ context.Context) ([]Candidate, error) { if err := loadKeychain(); err != nil { return nil, err } + searchList, done, err := searchListOf(s.keychains) + if err != nil { + return nil, err + } + defer done() + var leaves []*x509.Certificate - err := eachIdentity(func(_ uintptr, der []byte) (bool, error) { + err = eachIdentity(searchList, func(_ uintptr, der []byte) (bool, error) { cert, err := x509.ParseCertificate(der) if err != nil { log.Warnf("skipping keychain identity: %v", err) return false, nil } - log.Infof("keychain identity: subject=%q issuer=%q serial=%s expires=%s", cert.Subject, cert.Issuer, cert.SerialNumber, cert.NotAfter) + log.Debugf("keychain identity: subject=%q issuer=%q serial=%s expires=%s", cert.Subject, cert.Issuer, cert.SerialNumber, cert.NotAfter) leaves = append(leaves, cert) return false, nil }) @@ -84,24 +115,24 @@ func (s *KeychainStore) Candidates(_ context.Context) ([]Candidate, error) { } // The certificate query runs even without identities: it separates a keychain that is // readable but holds no identity from one the process cannot read at all. - pool, err := keychainCertificates() + pool, err := keychainCertificates(searchList) if err != nil { return nil, err } if len(leaves) == 0 { - log.Infof("keychain search list holds no identities usable for certificate posture, but %d readable certificates: an identity needs its private key in the same keychain", len(pool)) + log.Debugf("keychain search list holds no identities usable for certificate posture, but %d readable certificates: an identity needs its private key in the same keychain", len(pool)) return nil, nil } - log.Infof("keychain search list holds %d identities and %d certificates for chain building", len(leaves), len(pool)) + log.Debugf("keychain search list holds %d identities and %d certificates for chain building", len(leaves), len(pool)) candidates := make([]Candidate, 0, len(leaves)) for _, leaf := range leaves { chain := buildChain(leaf, pool) - log.Infof("keychain candidate %q issued by %q built a chain of %d certificates", leaf.Subject, leaf.Issuer, len(chain)) + log.Debugf("keychain candidate %q issued by %q built a chain of %d certificates", leaf.Subject, leaf.Issuer, len(chain)) if len(chain) == 1 && leaf.CheckSignatureFrom(leaf) != nil { - log.Infof("keychain candidate %q has no issuer in the keychain, its proof carries the leaf alone and only verifies if the challenge supplies %q", leaf.Subject, leaf.Issuer) + log.Debugf("keychain candidate %q has no issuer in the keychain, its proof carries the leaf alone and only verifies if the challenge supplies %q", leaf.Subject, leaf.Issuer) } - candidates = append(candidates, Candidate{Chain: chain, Signer: &keychainSigner{leaf: leaf}}) + candidates = append(candidates, Candidate{Chain: chain, Signer: &keychainSigner{leaf: leaf, keychains: s.keychains}, Intermediates: pool}) } return candidates, nil } @@ -109,7 +140,8 @@ func (s *KeychainStore) Candidates(_ context.Context) ([]Candidate, error) { // keychainSigner holds only the certificate; the identity is looked up again at signing // time so no keychain references outlive a call. type keychainSigner struct { - leaf *x509.Certificate + leaf *x509.Certificate + keychains []string } func (s *keychainSigner) Public() crypto.PublicKey { @@ -121,11 +153,17 @@ func (s *keychainSigner) Sign(_ io.Reader, digest []byte, opts crypto.SignerOpts if err != nil { return nil, err } - log.Infof("signing certificate posture challenge with keychain key of %q", s.leaf.Subject) + log.Debugf("signing certificate posture challenge with keychain key of %q", s.leaf.Subject) + + searchList, done, err := searchListOf(s.keychains) + if err != nil { + return nil, err + } + defer done() algorithm := keychainAlgorithm(scheme) var signature []byte - err = eachIdentity(func(identity uintptr, der []byte) (bool, error) { + err = eachIdentity(searchList, func(identity uintptr, der []byte) (bool, error) { if !bytes.Equal(der, s.leaf.Raw) { return false, nil } @@ -138,7 +176,7 @@ func (s *keychainSigner) Sign(_ io.Reader, digest []byte, opts crypto.SignerOpts if signature == nil { return nil, errors.New("certificate is no longer in the keychain") } - log.Infof("keychain signed certificate posture challenge for %q, %d bytes", s.leaf.Subject, len(signature)) + log.Debugf("keychain signed certificate posture challenge for %q, %d bytes", s.leaf.Subject, len(signature)) return signature, nil } @@ -158,37 +196,94 @@ func signWithIdentity(identity, algorithm uintptr, digest []byte) ([]byte, error if status := secIdentityCopyPrivateKey(identity, &key); status != 0 { return nil, fmt.Errorf("SecIdentityCopyPrivateKey: %d", status) } - defer cfRelease(key) + defer release(key) data := cfDataCreate(0, &digest[0], len(digest)) - defer cfRelease(data) + if data == 0 { + return nil, errors.New("CFDataCreate returned NULL") + } + defer release(data) var cfErr uintptr signature := secKeyCreateSignature(key, algorithm, data, &cfErr) if signature == 0 { - defer cfRelease(cfErr) - return nil, fmt.Errorf("SecKeyCreateSignature: CFError %d", cfErrorGetCode(cfErr)) + if cfErr == 0 { + return nil, errors.New("SecKeyCreateSignature failed without a CFError") + } + defer release(cfErr) + code := cfErrorGetCode(cfErr) + if code == errSecInteractionNotAllowed { + return nil, fmt.Errorf("SecKeyCreateSignature: CFError %d: %w", code, errKeyNeedsApproval) + } + return nil, fmt.Errorf("SecKeyCreateSignature: CFError %d", code) } - defer cfRelease(signature) + defer release(signature) return dataBytes(signature), nil } -func eachIdentity(fn func(identity uintptr, der []byte) (bool, error)) error { - return eachMatching(kSecClassIdentity, "identity", func(identity uintptr) (bool, error) { +// searchListOf opens the keychain files at paths as a CFArray for kSecMatchSearchList, +// or yields 0, the process's own search list, when paths is empty. The caller calls done +// once it no longer uses the list. +func searchListOf(paths []string) (searchList uintptr, done func(), err error) { + if len(paths) == 0 { + return 0, func() {}, nil + } + list, err := openSearchList(paths) + if err != nil { + return 0, nil, err + } + return list, func() { release(list) }, nil +} + +// openSearchList opens the keychain files at paths, at least one, as a CFArray. The +// caller releases the array. +func openSearchList(paths []string) (uintptr, error) { + refs := make([]uintptr, 0, len(paths)) + defer func() { + for _, ref := range refs { + release(ref) + } + }() + for _, path := range paths { + cpath := append([]byte(path), 0) + var keychain uintptr + if status := secKeychainOpen(&cpath[0], &keychain); status != 0 { + return 0, fmt.Errorf("SecKeychainOpen %s: %d", path, status) + } + refs = append(refs, keychain) + } + // The array retains the keychains, so the references opened here are released. + list := cfArrayCreate(0, &refs[0], len(refs), kCFTypeArrayCallBacks) + if list == 0 { + return 0, errors.New("CFArrayCreate returned NULL") + } + return list, nil +} + +// eachIdentity calls fn with every identity in searchList, or in the process's search +// list when it is 0, and its certificate. An identity whose certificate cannot be read is +// skipped rather than ending the walk. +func eachIdentity(searchList uintptr, fn func(identity uintptr, der []byte) (bool, error)) error { + return eachMatching(searchList, kSecClassIdentity, "identity", func(identity uintptr) (bool, error) { var cert uintptr if status := secIdentityCopyCertificate(identity, &cert); status != 0 { - return true, fmt.Errorf("SecIdentityCopyCertificate: %d", status) + log.Debugf("skipping keychain identity: SecIdentityCopyCertificate: %d", status) + return false, nil } der := certificateDER(cert) - cfRelease(cert) + release(cert) + if der == nil { + log.Debug("skipping keychain identity whose certificate has no DER data") + return false, nil + } return fn(identity, der) }) } -func keychainCertificates() ([]*x509.Certificate, error) { +func keychainCertificates(searchList uintptr) ([]*x509.Certificate, error) { var certs []*x509.Certificate var unparsable int - err := eachMatching(kSecClassCertificate, "certificate", func(item uintptr) (bool, error) { + err := eachMatching(searchList, kSecClassCertificate, "certificate", func(item uintptr) (bool, error) { if cert, err := x509.ParseCertificate(certificateDER(item)); err == nil { certs = append(certs, cert) return false, nil @@ -196,30 +291,34 @@ func keychainCertificates() ([]*x509.Certificate, error) { unparsable++ return false, nil }) - log.Infof("keychain holds %d parsable certificates, %d unparsable", len(certs), unparsable) + log.Debugf("keychain holds %d parsable certificates, %d unparsable", len(certs), unparsable) return certs, err } -func eachMatching(class uintptr, name string, fn func(item uintptr) (bool, error)) error { +func eachMatching(searchList, class uintptr, name string, fn func(item uintptr) (bool, error)) error { keys := []uintptr{kSecClass, kSecMatchLimit, kSecReturnRef} values := []uintptr{class, kSecMatchLimitAll, kCFBooleanTrue} + if searchList != 0 { + keys = append(keys, kSecMatchSearchList) + values = append(values, searchList) + } query := cfDictionaryCreate(0, &keys[0], &values[0], len(keys), kCFTypeDictionaryKeyCallBacks, kCFTypeDictionaryValueCallBacks) - defer cfRelease(query) + defer release(query) var items uintptr switch status := secItemCopyMatching(query, &items); status { case 0: case errSecItemNotFound: - log.Infof("keychain %s query returned errSecItemNotFound (%d): the search list holds no item of this class", name, errSecItemNotFound) + log.Debugf("keychain %s query returned errSecItemNotFound (%d): the search list holds no item of this class", name, errSecItemNotFound) return nil default: - log.Infof("keychain %s query returned OSStatus %d", name, status) + log.Debugf("keychain %s query returned OSStatus %d", name, status) return fmt.Errorf("SecItemCopyMatching: %d", status) } - defer cfRelease(items) + defer release(items) n := cfArrayGetCount(items) - log.Infof("keychain %s query returned %d items", name, n) + log.Debugf("keychain %s query returned %d items", name, n) for i := 0; i < n; i++ { if stop, err := fn(cfArrayGetValueAtIndex(items, i)); stop || err != nil { return err @@ -228,23 +327,40 @@ func eachMatching(class uintptr, name string, fn func(item uintptr) (bool, error return nil } +// certificateDER returns the DER form of cert, or nil when SecCertificateCopyData +// returns NULL, which it does for an object that is not a valid certificate. func certificateDER(cert uintptr) []byte { data := secCertificateCopyData(cert) - defer cfRelease(data) + if data == 0 { + return nil + } + defer release(data) return dataBytes(data) } func dataBytes(data uintptr) []byte { - return bytes.Clone(unsafe.Slice((*byte)(cfDataGetBytePtr(data)), cfDataGetLength(data))) + n := cfDataGetLength(data) + if n <= 0 { + return nil + } + return bytes.Clone(unsafe.Slice((*byte)(cfDataGetBytePtr(data)), n)) +} + +// release drops a CoreFoundation reference. CFRelease crashes the process on NULL, and +// several Security calls return NULL on failure, so every release goes through here. +func release(ref uintptr) { + if ref != 0 { + cfRelease(ref) + } } func loadKeychain() error { keychainOnce.Do(func() { if keychainErr = resolveKeychain(); keychainErr != nil { - log.Infof("macOS keychain unavailable for certificate posture: %v", keychainErr) + log.Debugf("macOS keychain unavailable for certificate posture: %v", keychainErr) return } - log.Infof("macOS Security framework loaded for certificate posture, running as uid=%d euid=%d", os.Getuid(), os.Geteuid()) + log.Debugf("macOS Security framework loaded for certificate posture, running as uid=%d euid=%d", os.Getuid(), os.Geteuid()) logSearchList() }) return keychainErr @@ -254,21 +370,21 @@ func loadKeychain() error { // System keychain and System Roots, never a user's login keychain. func logSearchList() { if secKeychainCopySearchList == nil || secKeychainGetPath == nil { - log.Info("keychain search list diagnostics unavailable on this macOS version") + log.Debug("keychain search list diagnostics unavailable on this macOS version") return } var list uintptr if status := secKeychainCopySearchList(&list); status != 0 { - log.Infof("SecKeychainCopySearchList returned OSStatus %d", status) + log.Debugf("SecKeychainCopySearchList returned OSStatus %d", status) return } - defer cfRelease(list) + defer release(list) n := cfArrayGetCount(list) - log.Infof("keychain search list contains %d keychains", n) + log.Debugf("keychain search list contains %d keychains", n) for i := 0; i < n; i++ { - log.Infof("keychain search list[%d]: %s", i, keychainPath(cfArrayGetValueAtIndex(list, i))) + log.Debugf("keychain search list[%d]: %s", i, keychainPath(cfArrayGetValueAtIndex(list, i))) } } @@ -301,6 +417,8 @@ func resolveKeychain() error { {&secIdentityCopyPrivateKey, security, "SecIdentityCopyPrivateKey"}, {&secCertificateCopyData, security, "SecCertificateCopyData"}, {&secKeyCreateSignature, security, "SecKeyCreateSignature"}, + {&secKeychainOpen, security, "SecKeychainOpen"}, + {&cfArrayCreate, coreFoundation, "CFArrayCreate"}, {&cfDictionaryCreate, coreFoundation, "CFDictionaryCreate"}, {&cfArrayGetCount, coreFoundation, "CFArrayGetCount"}, {&cfArrayGetValueAtIndex, coreFoundation, "CFArrayGetValueAtIndex"}, @@ -329,12 +447,14 @@ func resolveKeychain() error { {&kSecMatchLimit, security, "kSecMatchLimit", true}, {&kSecMatchLimitAll, security, "kSecMatchLimitAll", true}, {&kSecReturnRef, security, "kSecReturnRef", true}, + {&kSecMatchSearchList, security, "kSecMatchSearchList", true}, {&kSecKeyAlgorithmECDSASHA256, security, "kSecKeyAlgorithmECDSASignatureDigestX962SHA256", true}, {&kSecKeyAlgorithmECDSASHA384, security, "kSecKeyAlgorithmECDSASignatureDigestX962SHA384", true}, {&kSecKeyAlgorithmRSAPSSSHA256, security, "kSecKeyAlgorithmRSASignatureDigestPSSSHA256", true}, {&kCFBooleanTrue, coreFoundation, "kCFBooleanTrue", true}, {&kCFTypeDictionaryKeyCallBacks, coreFoundation, "kCFTypeDictionaryKeyCallBacks", false}, {&kCFTypeDictionaryValueCallBacks, coreFoundation, "kCFTypeDictionaryValueCallBacks", false}, + {&kCFTypeArrayCallBacks, coreFoundation, "kCFTypeArrayCallBacks", false}, } { addr, err := purego.Dlsym(global.lib, global.name) if err != nil { @@ -356,7 +476,7 @@ func resolveKeychain() error { func resolveOptional(lib uintptr, name string, ptr any) { symbol, err := purego.Dlsym(lib, name) if err != nil { - log.Infof("keychain diagnostics: %s unavailable: %v", name, err) + log.Debugf("keychain diagnostics: %s unavailable: %v", name, err) return } purego.RegisterFunc(ptr, symbol) diff --git a/client/internal/certproof/pinlatch.go b/client/internal/certproof/pinlatch.go new file mode 100644 index 000000000..79e396b11 --- /dev/null +++ b/client/internal/certproof/pinlatch.go @@ -0,0 +1,54 @@ +package certproof + +import ( + "crypto/sha256" + "encoding/binary" + "errors" + "sync" +) + +// errPINRejectedBefore is returned instead of logging in with a PIN the token already +// refused: every failed login counts towards the token's lockout, which for a TPM is +// shared with everything else on the machine, and proofs are collected on every sync. +var errPINRejectedBefore = errors.New("PKCS#11 token rejected this PIN before, not trying it again") + +// errPINNeedsToken refuses a PIN that names no token to log in to. +var errPINNeedsToken = errors.New("a PKCS#11 PIN needs the token named in NB_CERT_PKCS11_URI, as token=