diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 85ed5cd3b..a1336cdab 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -14,5 +14,15 @@ reviews: - "!**/*.ts" - "!**/*.js" - "!**/*.svg" + pre_merge_checks: + custom_checks: + - name: "No attribution trailers" + mode: error + instructions: >- + Fail when the PR description or any commit message carries an + attribution trailer or footer: Co-Authored-By, Claude-Session, + Generated-By, or a "Generated with"/"Generated by" tool line. + Contributors own their contributions (AGENTS.md); ask for the + lines to be removed. chat: auto_reply: true diff --git a/.githooks/commit-msg b/.githooks/commit-msg new file mode 100755 index 000000000..e5699bd9c --- /dev/null +++ b/.githooks/commit-msg @@ -0,0 +1,26 @@ +#!/bin/bash +# Refuses commit messages that carry attribution trailers. Contributors own +# their contributions (AGENTS.md, "No Co-Authored-By or tool-attribution +# trailers"); a trailer spreads that ownership onto a tool or a bystander. + +msg_file="$1" + +# Trailer keys in any casing, with any bullet or emoji in front. +trailers='^[^[:alnum:]]*(co-authored-by|claude-session|generated-by):' +# "Generated with/by" footers, including "Generated with by". +footer='^[^[:alnum:]]*generated (with|by)( [^[:alnum:]]*by)? ' +# A footer names a product, so a capitalized word must follow the phrase +# itself. Prose such as "generated by the protobuf compiler" stays legal. +tool='[Gg][Ee][Nn][Ee][Rr][Aa][Tt][Ee][Dd] ([Ww][Ii][Tt][Hh]|[Bb][Yy])( [^[:alnum:]]*[Bb][Yy])? [^[:alnum:]]*[A-Z]' + +offending=$( { + grep -Ein "$trailers" "$msg_file" + grep -Ein "$footer" "$msg_file" | grep -E "$tool" +} | sort -un ) + +if [ -n "$offending" ]; then + echo "commit-msg: attribution trailers are not accepted in this repository:" >&2 + printf '%s\n' "$offending" | sed 's/^/ /' >&2 + echo "Remove them and commit again (see AGENTS.md)." >&2 + exit 1 +fi diff --git a/.github/workflows/golang-test-darwin.yml b/.github/workflows/golang-test-darwin.yml index c17d8e775..dec57dc80 100644 --- a/.github/workflows/golang-test-darwin.yml +++ b/.github/workflows/golang-test-darwin.yml @@ -46,15 +46,17 @@ jobs: run: git --no-pager diff --exit-code - name: Test - # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # Exclude the client/ui package itself: its main.go uses //go:embed + # all:frontend/dist, which fails to compile until the frontend has been + # built, and its release pipeline runs `pnpm build` before goreleaser. + # The pattern is anchored so the subpackages (services, preferences, + # i18n, authsession) still run: they hold Go-side unit tests and need no + # frontend bundle. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the grep then drops the broken package by path. Without -e, # go list aborts with empty stdout and `go test` falls back to the repo # root, which has no Go files. - run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -coverprofile=coverage.txt -tags 'devcert privileged' -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 $(go list -e ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e /client/ui -e /client/testutil/privileged) + run: NETBIRD_STORE_ENGINE=${{ matrix.store }} CI=true go test -coverprofile=coverage.txt -tags 'devcert privileged' -exec 'sudo --preserve-env=CI,NETBIRD_STORE_ENGINE' -timeout 5m -p 1 $(go list -e ./... | grep -v -e /management -e /signal -e /relay -e /proxy -e /combined -e '/client/ui$' -e /client/testutil/privileged) - name: Upload coverage reports to Codecov uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f #v7.0.0 diff --git a/.github/workflows/golang-test-linux.yml b/.github/workflows/golang-test-linux.yml index f24dfbe9d..449eb14fa 100644 --- a/.github/workflows/golang-test-linux.yml +++ b/.github/workflows/golang-test-linux.yml @@ -160,9 +160,10 @@ jobs: - name: Test # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # which fails to compile until the frontend has been built, and its + # release pipeline runs `pnpm build` before goreleaser. The subpackages + # go with it because this runner's gtk4 is older than the wails runtime + # needs; the Client UI / Unit job below covers them instead. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the grep then drops the broken package by path. Without -e, # go list aborts with empty stdout and `go test` falls back to the repo @@ -177,6 +178,35 @@ jobs: slug: netbirdio/netbird flags: unit,client + test_client_ui: + name: "Client UI / Unit" + # Pinned to 24.04 rather than the 22.04 the other client jobs use: the wails + # runtime's linux cgo layer needs GtkFileDialog, which arrived in gtk4 4.10, + # and jammy ships 4.6. Not ubuntu-latest, so a runner image rollover cannot + # move this out from under us. + runs-on: ubuntu-24.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 dependencies + run: sudo apt update && sudo apt install -y -q libgtk-4-dev libwebkitgtk-6.0-dev libsoup-3.0-dev + + - name: Test + # client/ui itself stays out: its main.go embeds all:frontend/dist, + # which only exists after `pnpm build`. The subpackages carry the + # Go-side unit tests, including the window manager re-entrancy + # 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_on_docker: name: "Client (Docker) / Unit" needs: [build-cache] @@ -211,6 +241,9 @@ jobs: ${{ runner.os }}-gotest-cache- - name: Run tests in container + # Unlike the native job above, this one drops all of client/ui including + # the subpackages: the alpine container has no gtk4/webkitgtk, so the + # Wails application package they import would fail to link. env: HOST_GOCACHE: ${{ steps.go-env.outputs.cache_dir }} HOST_GOMODCACHE: ${{ steps.go-env.outputs.modcache_dir }} @@ -481,14 +514,32 @@ jobs: if: matrix.store == 'mysql' run: docker pull mlsmaycon/warmed-mysql:8 + # The -json stream goes through tools/gotestsummary so the log shows one + # line per test, the output of failed tests, the head of a timeout panic + # with the still-running tests, and the slowest tests per package. - name: Test + shell: bash run: | + set -o pipefail CGO_ENABLED=1 GOARCH=${{ matrix.arch }} \ NETBIRD_STORE_ENGINE=${{ matrix.store }} \ CI=true \ - go test -tags=devcert -coverprofile=coverage.txt \ + go test -json -tags=devcert -coverprofile=coverage.txt \ -exec "sudo --preserve-env=CI,NETBIRD_STORE_ENGINE" \ - -timeout 20m ./management/... ./shared/management/... + -timeout 20m ./management/... ./shared/management/... \ + | tee management-test-events.jsonl \ + | go run ./tools/gotestsummary + + # The summary trims long outputs; the raw stream keeps every line for + # the failures that need it. A green run has no use for it. + - name: Upload raw test events + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a #v7.0.1 + with: + name: management-unit-test-events-${{ matrix.store }} + path: management-test-events.jsonl + if-no-files-found: ignore + retention-days: 14 - name: Upload coverage reports to Codecov if: matrix.arch == 'amd64' @@ -738,12 +789,27 @@ jobs: - name: check git status run: git --no-pager diff --exit-code + # Same summary as the unit job: a timeout here names the tests still + # running instead of ending in a goroutine dump. - name: Test + shell: bash run: | + set -o pipefail CGO_ENABLED=1 GOARCH=${{ matrix.arch }} \ NETBIRD_STORE_ENGINE=${{ matrix.store }} \ CI=true \ - mage integrationtest:all -gotestflags="-coverprofile=coverage.txt" + mage integrationtest:all -gotestflags="-json -coverprofile=coverage.txt" \ + | tee management-integration-test-events.jsonl \ + | go run ./tools/gotestsummary + + - name: Upload raw test events + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a #v7.0.1 + with: + name: management-integration-test-events-${{ matrix.store }} + path: management-integration-test-events.jsonl + if-no-files-found: ignore + retention-days: 14 - name: Upload coverage reports to Codecov if: matrix.arch == 'amd64' diff --git a/.github/workflows/golang-test-windows.yml b/.github/workflows/golang-test-windows.yml index fb7b745d2..ca300f7df 100644 --- a/.github/workflows/golang-test-windows.yml +++ b/.github/workflows/golang-test-windows.yml @@ -66,15 +66,17 @@ jobs: - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe env -w GOCACHE=${{ env.modcache }} - run: PsExec64 -s -w ${{ github.workspace }} C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe mod tidy - name: Generate test script - # Exclude client/ui: its main.go uses //go:embed all:frontend/dist, - # which fails to compile until the frontend has been built. The Wails UI - # has no Go-side unit tests, and its release pipeline runs `pnpm build` - # before goreleaser. + # Exclude the client/ui package itself: its main.go uses //go:embed + # all:frontend/dist, which fails to compile until the frontend has been + # built, and its release pipeline runs `pnpm build` before goreleaser. + # The pattern is anchored so the subpackages (services, preferences, + # i18n, authsession) still run: they hold Go-side unit tests and need no + # frontend bundle. # `go list -e` lets the listing succeed even though the embed fails to # resolve; the Where-Object pipeline then drops the broken package by # path. Without -e, go list aborts with empty stdout. run: | - $packages = go list -e ./... | Where-Object { $_ -notmatch '/management' } | Where-Object { $_ -notmatch '/relay' } | Where-Object { $_ -notmatch '/signal' } | Where-Object { $_ -notmatch '/proxy' } | Where-Object { $_ -notmatch '/combined' } | Where-Object { $_ -notmatch '/client/ui' } + $packages = go list -e ./... | Where-Object { $_ -notmatch '/management' } | Where-Object { $_ -notmatch '/relay' } | Where-Object { $_ -notmatch '/signal' } | Where-Object { $_ -notmatch '/proxy' } | Where-Object { $_ -notmatch '/combined' } | Where-Object { $_ -notmatch '/client/ui$' } $goExe = "C:\hostedtoolcache\windows\go\${{ steps.go.outputs.go-version }}\x64\bin\go.exe" $cmd = "$goExe test -tags `"devcert privileged`" -timeout 10m -p 1 $($packages -join ' ') > test-out.txt 2>&1" Set-Content -Path "${{ github.workspace }}\run-tests.cmd" -Value $cmd diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c1bbe9c44..51426a7ce 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -186,6 +186,11 @@ jobs: run: bash shared/management/http/api/generate.sh - name: check git status run: git --no-pager diff --exit-code + - name: Generate RPM changelog from git tags + # nfpm embeds changelog.yml into the RPM; Red Hat software certification + # requires a changelog. Generated, not committed (see .gitignore). + # chglog is a go.mod tool directive, so go.sum pins it and its deps. + run: bash release_files/rpm-changelog.sh - name: Set up QEMU uses: docker/setup-qemu-action@06116385d9baf250c9f4dcb4858b16962ea869c3 #v4.1.0 - name: Set up Docker Buildx @@ -287,10 +292,15 @@ jobs: image_refs=() tag_and_push() { - local src="$1" img_name tag dst + local src="$1" img_name tag dst variant="" img_name="${src%%:*}" + # Client variants share a repository, so keep their tag suffixes. + case "$src" in + *-rootless-ubi-amd64) variant="-rootless-ubi" ;; + *-rootless-amd64) variant="-rootless" ;; + esac for tag in $(resolve_tags); do - dst="${img_name}:${tag}" + dst="${img_name}:${tag}${variant}" echo "Tagging ${src} -> ${dst}" docker tag "$src" "$dst" docker push "$dst" diff --git a/.gitignore b/.gitignore index 305f3cb50..dd7eea76f 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,7 @@ vendor/ /netbird client/netbird-electron/ management/server/types/testdata/ + +# generated by chglog in the release workflow, embedded into the RPM +changelog.yml +.chglog.yml diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 294f1ea58..acdb3d3d5 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -234,10 +234,33 @@ nfpms: - netbird formats: - rpm + # The client verifies TLS to management and signal against the system trust + # store. Red Hat software certification (RPM Dependency Tracking) also + # rejects packages that declare no dependencies at all. + dependencies: + - ca-certificates + # Generated in CI by chglog from git tags; Red Hat certification requires an + # RPM changelog (RPM Version Handling subtest). + changelog: changelog.yml + # License, documentation and a config file so the RPM Provenance subtest sees + # %license, %doc and %config entries instead of a bare binary. + contents: + - src: LICENSE + dst: /usr/share/licenses/netbird/LICENSE + type: license + - src: README.md + dst: /usr/share/doc/netbird/README.md + type: doc + - src: release_files/netbird.sysconfig + dst: /etc/sysconfig/netbird + type: config|noreplace scripts: postinstall: "release_files/post_install.sh" preremove: "release_files/pre_remove.sh" rpm: + summary: NetBird client + group: Applications/Internet + packager: NetBird signature: key_file: '{{ if index .Env "GPG_RPM_KEY_FILE" }}{{ .Env.GPG_RPM_KEY_FILE }}{{ end }}' dockers_v2: @@ -289,6 +312,43 @@ dockers_v2: "org.opencontainers.image.revision": "{{.FullCommit}}" "org.opencontainers.image.source": "{{.GitURL}}" "maintainer": "dev@netbird.io" + - id: netbird-rootless-ubi + disable: "{{ .Env.SKIP_DOCKER_PUSH }}" + ids: + - netbird + images: + - netbirdio/netbird + - ghcr.io/netbirdio/netbird + tags: + - "{{ .Version }}-rootless-ubi" + - "{{ if eq .Env.SKIP_PUBLISH \"false\" }}rootless-ubi-latest{{ end }}" + dockerfile: client/Dockerfile-rootless.ubi + extra_files: + - client/netbird-entrypoint.sh + platforms: + - linux/amd64 + - linux/arm64 + build_args: + VERSION: "{{ .Version }}" + RELEASE: "{{ .Timestamp }}" + hooks: + pre: + - cmd: 'sh client/collect-licenses.sh "{{ .ContextDir }}/licenses" amd64 arm64' + env: + - GOOS=linux + - CGO_ENABLED=0 + labels: + "org.opencontainers.image.created": "{{.Date}}" + "org.opencontainers.image.version": "{{.Version}}" + "org.opencontainers.image.revision": "{{.FullCommit}}" + "org.opencontainers.image.source": "{{.GitURL}}" + annotations: + "org.opencontainers.image.created": "{{.Date}}" + "org.opencontainers.image.title": "{{.ProjectName}}" + "org.opencontainers.image.version": "{{.Version}}" + "org.opencontainers.image.revision": "{{.FullCommit}}" + "org.opencontainers.image.source": "{{.GitURL}}" + "maintainer": "dev@netbird.io" - id: relay disable: "{{ .Env.SKIP_DOCKER_PUSH }}" ids: diff --git a/AGENTS.md b/AGENTS.md index 5497acb15..3838a7913 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -77,7 +77,7 @@ make lint # golangci-lint on files changed vs origin/main (also the p make lint-all # full-repository lint, matches CI make test-unit # host-safe unit tests, -tags devcert, no sudo make test-privileged # privileged-tagged suite in a Docker container with NET_ADMIN -make setup-hooks # wire make lint into .githooks/pre-push +make setup-hooks # wire .githooks: pre-push runs make lint, commit-msg refuses attribution trailers # Narrow runs go test ./client/internal/dns/... diff --git a/CLAUDE.md b/CLAUDE.md index 764f406be..72681748e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1 +1,4 @@ -See [AGENTS.md](AGENTS.md) for the agent guidelines in this repository. +The agent guidelines live in [AGENTS.md](AGENTS.md). It is imported here so +every session loads it in full rather than following a pointer. + +@AGENTS.md diff --git a/Makefile b/Makefile index 0a4fad2f2..26c5b932e 100644 --- a/Makefile +++ b/Makefile @@ -23,8 +23,8 @@ lint-install: $(GOLANGCI_LINT) # Setup git hooks for all developers setup-hooks: @git config core.hooksPath .githooks - @chmod +x .githooks/pre-push - @echo "✅ Git hooks configured! Pre-push will now run 'make lint'" + @chmod +x .githooks/pre-push .githooks/commit-msg + @echo "✅ Git hooks configured! Pre-push runs 'make lint'; commit-msg refuses attribution trailers" # Host-safe unit tests: excludes the privileged-tagged tests (root / system-mutating). # Runs as a normal user with no sudo and leaves host networking untouched. diff --git a/client/Dockerfile-rootless.ubi b/client/Dockerfile-rootless.ubi new file mode 100644 index 000000000..4701728c1 --- /dev/null +++ b/client/Dockerfile-rootless.ubi @@ -0,0 +1,45 @@ +FROM registry.access.redhat.com/ubi9/ubi-minimal@sha256:7fbeae18dc9476399f565e68255f602a3374ea8614ba3d14843565131a13ff93 + +ARG TARGETPLATFORM +ARG NETBIRD_BINARY=$TARGETPLATFORM/netbird +ARG VERSION=dev +ARG RELEASE=1 + +LABEL name="netbird-rootless" \ + maintainer="NetBird " \ + vendor="NetBird GmbH" \ + version="${VERSION}" \ + release="${RELEASE}" \ + summary="NetBird Rootless Client" \ + description="NetBird connects devices through an encrypted overlay using userspace networking without a TUN device or network administration capabilities." + +RUN microdnf install -y bash ca-certificates && microdnf clean all + +COPY --chmod=0555 client/netbird-entrypoint.sh /usr/local/bin/netbird-entrypoint.sh +COPY --chmod=0555 ${NETBIRD_BINARY} /usr/local/bin/netbird +COPY licenses/ /licenses/ +# Only application storage is group-writable for arbitrary non-root UIDs. +# Runtime-created credentials keep the client's restrictive file modes. +RUN mkdir -p /var/lib/netbird && \ + chown 1000:0 /var/lib/netbird && \ + chmod 0770 /var/lib/netbird && \ + chmod -R a+rX /licenses + +WORKDIR /var/lib/netbird +USER 1000:0 + +ENV \ + HOME="/var/lib/netbird" \ + NETBIRD_BIN="/usr/local/bin/netbird" \ + NB_USE_NETSTACK_MODE="true" \ + NB_ENABLE_NETSTACK_LOCAL_FORWARDING="true" \ + NB_CONFIG="/var/lib/netbird/config.json" \ + NB_STATE_DIR="/var/lib/netbird" \ + NB_DAEMON_ADDR="unix:///var/lib/netbird/netbird.sock" \ + NB_LOG_FILE="console,/var/lib/netbird/client.log" \ + NB_DISABLE_DNS="true" \ + NB_ENABLE_CAPTURE="false" \ + NB_ENTRYPOINT_SERVICE_TIMEOUT="30" + +STOPSIGNAL SIGTERM +ENTRYPOINT ["/usr/local/bin/netbird-entrypoint.sh"] diff --git a/client/collect-licenses.sh b/client/collect-licenses.sh new file mode 100644 index 000000000..7dfabada9 --- /dev/null +++ b/client/collect-licenses.sh @@ -0,0 +1,77 @@ +#!/bin/sh +set -eu + +if [ "$#" -lt 2 ]; then + printf '%s\n' "usage: $0 OUTPUT_DIRECTORY GOARCH..." >&2 + exit 2 +fi + +repo_root=$(CDPATH= cd -- "$(dirname "$0")/.." && pwd) +output_name=$(basename "$1") +if [ -z "$output_name" ] || [ "$output_name" = "." ] || + [ "$output_name" = ".." ] || [ "$output_name" = "/" ]; then + printf '%s\n' "OUTPUT_DIRECTORY must name a directory" >&2 + exit 2 +fi +output_parent=$(CDPATH= cd -- "$(dirname "$1")" && pwd) +output="$output_parent/$output_name" +shift +modules=$(mktemp "${TMPDIR:-/tmp}/netbird-client-licenses.modules.XXXXXX") +sorted_modules=$(mktemp "${TMPDIR:-/tmp}/netbird-client-licenses.sorted.XXXXXX") +trap 'rm -f "$modules" "$sorted_modules"' EXIT HUP INT TERM + +if [ -e "$output" ] || [ -L "$output" ]; then + printf 'output directory already exists: %s\n' "$output" >&2 + exit 1 +fi +mkdir "$output" +mkdir "$output/third_party" + +cp "$repo_root/LICENSE" "$output/BSD-3-Clause.txt" + +cd "$repo_root" +for arch in "$@"; do + GOOS=${GOOS:-linux} GOARCH="$arch" CGO_ENABLED=${CGO_ENABLED:-0} \ + go list -deps -f '{{with .Module}}{{if .Replace}}{{.Replace.Path}}{{"\t"}}{{.Replace.Version}}{{"\t"}}{{.Replace.Dir}}{{else}}{{.Path}}{{"\t"}}{{.Version}}{{"\t"}}{{.Dir}}{{end}}{{end}}' -tags load_wgnt_from_rsrc ./client >>"$modules" +done +LC_ALL=C sort -u "$modules" >"$sorted_modules" + +goroot=$(go env GOROOT) +for term in LICENSE PATENTS; do + if [ ! -f "$goroot/$term" ]; then + printf 'missing Go standard-library term: %s\n' "$goroot/$term" >&2 + exit 1 + fi + cp "$goroot/$term" "$output/Go-$term" +done + +while IFS=' ' read -r module version module_dir; do + [ -n "$module" ] || continue + [ "$module" = "github.com/netbirdio/netbird" ] && continue + + if [ -z "$version" ] || [ ! -d "$module_dir" ]; then + printf 'cannot collect terms for module %s at version %s\n' "$module" "$version" >&2 + exit 1 + fi + + destination="$output/third_party/$module/$version" + mkdir -p "$destination" + printf 'module: %s\nversion: %s\n' "$module" "$version" >"$destination/MODULE" + + found=false + for term in \ + "$module_dir"/LICENSE* "$module_dir"/License* "$module_dir"/license* \ + "$module_dir"/LICENCE* "$module_dir"/Licence* "$module_dir"/licence* \ + "$module_dir"/COPYING* "$module_dir"/Copying* "$module_dir"/copying* \ + "$module_dir"/NOTICE* "$module_dir"/Notice* "$module_dir"/notice* \ + "$module_dir"/PATENTS* "$module_dir"/Patents* "$module_dir"/patents*; do + [ -f "$term" ] || continue + cp "$term" "$destination/" + found=true + done + + if [ "$found" = false ]; then + printf 'no root license terms found for module %s at %s\n' "$module" "$module_dir" >&2 + exit 1 + fi +done <"$sorted_modules" diff --git a/client/internal/dns/service_listener.go b/client/internal/dns/service_listener.go index 3dc29c4dc..d65a727b1 100644 --- a/client/internal/dns/service_listener.go +++ b/client/internal/dns/service_listener.go @@ -6,6 +6,7 @@ import ( "net" "net/netip" "runtime" + "slices" "strconv" "sync" "time" @@ -17,17 +18,20 @@ import ( nberrors "github.com/netbirdio/netbird/client/errors" firewall "github.com/netbirdio/netbird/client/firewall/manager" - "github.com/netbirdio/netbird/client/internal/ebpf" - ebpfMgr "github.com/netbirdio/netbird/client/internal/ebpf/manager" ) const ( customPort = 5053 + // randomPortAttempts bounds the search for a port free on both protocols. + randomPortAttempts = 5 ) var ( defaultIP = netip.MustParseAddr("127.0.0.1") customIP = netip.MustParseAddr("127.0.0.153") + + // dnatProtocols are the protocols the port 53 redirect covers. + dnatProtocols = []firewall.Protocol{firewall.ProtocolUDP, firewall.ProtocolTCP} ) type serviceViaListener struct { @@ -40,9 +44,20 @@ type serviceViaListener struct { listenPort uint16 listenerIsRunning bool listenerFlagLock sync.Mutex - ebpfService ebpfMgr.Manager firewall Firewall - tcpDNATConfigured bool + // dnatRules holds the port 53 redirects that are installed and not yet + // removed, so a removal that fails can be retried. + dnatRules []dnatRule +} + +// dnatRule is a port 53 redirect as it was installed. The target is kept with +// the rule because the listener can come back on a different address or port, +// and a retried removal has to name the address and port the rule was added +// with, not the ones in use now. +type dnatRule struct { + protocol firewall.Protocol + ip netip.Addr + port uint16 } func newServiceViaListener(wgIface WGIface, customAddr *netip.AddrPort, fw Firewall) *serviceViaListener { @@ -112,34 +127,93 @@ func (s *serviceViaListener) Listen() error { } }() - // When eBPF redirects UDP port 53 to our listen port, TCP still needs - // a DNAT rule because eBPF only handles UDP. - if s.ebpfService != nil && s.firewall != nil && s.listenPort != DefaultPort { - if err := s.firewall.AddOutputDNAT(s.listenIP, firewall.ProtocolTCP, DefaultPort, s.listenPort); err != nil { - log.Warnf("failed to add DNS TCP DNAT rule, TCP DNS on port 53 will not work: %v", err) - } else { - s.tcpDNATConfigured = true - log.Infof("added DNS TCP DNAT rule: %s:%d -> %s:%d", s.listenIP, DefaultPort, s.listenIP, s.listenPort) - } + if s.listenPort != DefaultPort { + s.setupDNAT() } return nil } +// setupDNAT redirects port 53 to the port the DNS server actually listens on. +// Both protocols must be redirected or none: RuntimePort reports port 53 only +// while the full redirect is in place, so a half-configured redirect would +// advertise a resolver that answers over one protocol. +func (s *serviceViaListener) setupDNAT() { + if s.firewall == nil { + log.Errorf("no firewall manager available to redirect DNS port %d to %d, "+ + "clients pointed at %s will not reach the resolver", DefaultPort, s.listenPort, s.listenIP) + return + } + + // Clear whatever an earlier removal left behind first. Those rules can point + // at an address or port this listener no longer uses, and they are matched + // before anything added now, so adding a redirect on top of one would keep + // sending port 53 traffic to the previous listener while reporting the + // redirect as complete. The rules stay recorded for a later attempt. + if err := s.removeDNAT(); err != nil { + log.Errorf("failed to remove stale DNS DNAT rules, leaving port %d redirected to the previous listener: %v", + DefaultPort, err) + return + } + + for _, proto := range dnatProtocols { + if err := s.firewall.AddOutputDNAT(s.listenIP, proto, DefaultPort, s.listenPort); err != nil { + log.Errorf("failed to add DNS %s DNAT rule, DNS on port %d will not work: %v", + proto, DefaultPort, err) + if err := s.removeDNAT(); err != nil { + log.Warnf("failed to roll back DNS DNAT rules, retrying on stop: %v", err) + } + return + } + s.dnatRules = append(s.dnatRules, dnatRule{protocol: proto, ip: s.listenIP, port: s.listenPort}) + } + + log.Infof("added DNS DNAT rules: %s:%d -> %s:%d (UDP + TCP)", s.listenIP, DefaultPort, s.listenIP, s.listenPort) +} + +// removeDNAT removes every installed port 53 redirect. A rule whose removal +// fails stays recorded so a later setup or Stop retries it, rather than leaving +// port 53 pointing at a resolver that is no longer listening. +func (s *serviceViaListener) removeDNAT() error { + if s.firewall == nil { + return nil + } + + var merr *multierror.Error + var remaining []dnatRule + for _, rule := range s.dnatRules { + if err := s.firewall.RemoveOutputDNAT(rule.ip, rule.protocol, DefaultPort, rule.port); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove DNS %s DNAT rule for %s:%d: %w", + rule.protocol, rule.ip, rule.port, err)) + remaining = append(remaining, rule) + } + } + s.dnatRules = remaining + + return nberrors.FormatErrorOrNil(merr) +} + func (s *serviceViaListener) Stop() error { s.listenerFlagLock.Lock() defer s.listenerFlagLock.Unlock() + var merr *multierror.Error + + // Redirects are removed even when the listener is already stopped, so that + // a removal which failed earlier is retried instead of leaving port 53 + // pointing at a resolver that no longer listens. + if err := s.removeDNAT(); err != nil { + merr = multierror.Append(merr, err) + } + if !s.listenerIsRunning { - return nil + return nberrors.FormatErrorOrNil(merr) } s.listenerIsRunning = false ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - var merr *multierror.Error - if err := s.server.ShutdownContext(ctx); err != nil { merr = multierror.Append(merr, fmt.Errorf("stop DNS UDP server: %w", err)) } @@ -148,19 +222,6 @@ func (s *serviceViaListener) Stop() error { merr = multierror.Append(merr, fmt.Errorf("stop DNS TCP server: %w", err)) } - if s.tcpDNATConfigured && s.firewall != nil { - if err := s.firewall.RemoveOutputDNAT(s.listenIP, firewall.ProtocolTCP, DefaultPort, s.listenPort); err != nil { - merr = multierror.Append(merr, fmt.Errorf("remove DNS TCP DNAT rule: %w", err)) - } - s.tcpDNATConfigured = false - } - - if s.ebpfService != nil { - if err := s.ebpfService.FreeDNSFwd(); err != nil { - merr = multierror.Append(merr, fmt.Errorf("stop traffic forwarder: %w", err)) - } - } - return nberrors.FormatErrorOrNil(merr) } @@ -177,11 +238,23 @@ func (s *serviceViaListener) RuntimePort() int { s.listenerFlagLock.Lock() defer s.listenerFlagLock.Unlock() - if s.ebpfService != nil { + if s.redirectInstalled() { return DefaultPort - } else { - return int(s.listenPort) } + return int(s.listenPort) +} + +// redirectInstalled reports whether every protocol is redirected from port 53 +// to the address and port the listener currently serves. Rules left over from +// an earlier listener do not count. +func (s *serviceViaListener) redirectInstalled() bool { + for _, proto := range dnatProtocols { + current := dnatRule{protocol: proto, ip: s.listenIP, port: s.listenPort} + if !slices.Contains(s.dnatRules, current) { + return false + } + } + return true } func (s *serviceViaListener) RuntimeIP() netip.Addr { @@ -190,30 +263,29 @@ func (s *serviceViaListener) RuntimeIP() netip.Addr { // evalListenAddress figures out the listen address for the DNS server. // IPv4-only: all peers have a v4 overlay address, and DNS config points to v4. -// First checks port 53 on WG interface or lo, then tries eBPF on a random port, -// then falls back to port 5053. +// Prefers port 53 on the overlay interface or lo, so no redirect is needed at +// all; when it is taken it falls back to port 5053 and then to a random free +// port, both of which need the port 53 redirect set up by setupDNAT. func (s *serviceViaListener) evalListenAddress() (netip.Addr, uint16, error) { if s.customAddr != nil { return s.customAddr.Addr(), s.customAddr.Port(), nil } - ip, ok := s.testFreePort(DefaultPort) - if ok { + if ip, ok := s.testFreePort(DefaultPort); ok { return ip, DefaultPort, nil } - ebpfSrv, port, ok := s.tryToUseeBPF() - if ok { - s.ebpfService = ebpfSrv - return s.wgInterface.Address().IP, port, nil - } - - ip, ok = s.testFreePort(customPort) - if ok { + if ip, ok := s.testFreePort(customPort); ok { return ip, customPort, nil } - return netip.Addr{}, 0, fmt.Errorf("failed to find a free port for DNS server") + ip := s.wgInterface.Address().IP + port, err := s.randomFreePort(ip) + if err != nil { + return netip.Addr{}, 0, fmt.Errorf("find a free port for DNS server: %w", err) + } + + return ip, port, nil } func (s *serviceViaListener) testFreePort(port int) (netip.Addr, bool) { @@ -260,48 +332,25 @@ func (s *serviceViaListener) tryToBind(ip netip.Addr, port int) bool { return true } -// tryToUseeBPF decides whether to apply eBPF program to capture DNS traffic on port 53. -// This is needed because on some operating systems if we start a DNS server not on a default port 53, -// the domain name resolution won't work. So, in case we are running on Linux and picked a free -// port we should fall back to the eBPF solution that will capture traffic on port 53 and forward -// it to a local DNS server running on the chosen port. -func (s *serviceViaListener) tryToUseeBPF() (ebpfMgr.Manager, uint16, bool) { - if runtime.GOOS != "linux" { - return nil, 0, false +// randomFreePort returns a port that is free on ip for both UDP and TCP, since +// the DNS server binds both. The probe listeners are closed again, so the port +// is only likely, not guaranteed, to still be free when the server binds it. +func (s *serviceViaListener) randomFreePort(ip netip.Addr) (uint16, error) { + for range randomPortAttempts { + probeListener, err := net.ListenUDP("udp4", &net.UDPAddr{}) + if err != nil { + return 0, fmt.Errorf("bind random port: %w", err) + } + + port := uint16(probeListener.LocalAddr().(*net.UDPAddr).Port) + if err := probeListener.Close(); err != nil { + return 0, fmt.Errorf("free up probed port: %w", err) + } + + if s.tryToBind(ip, int(port)) { + return port, nil + } } - port, err := s.generateFreePort() //nolint:staticcheck,unused - if err != nil { - log.Warnf("failed to generate a free port for eBPF DNS forwarder server: %s", err) - return nil, 0, false - } - - ebpfSrv := ebpf.GetEbpfManagerInstance() - err = ebpfSrv.LoadDNSFwd(s.wgInterface.Address().IP, int(port)) - if err != nil { - log.Warnf("failed to load DNS forwarder eBPF program, error: %s", err) - return nil, 0, false - } - - return ebpfSrv, port, true -} - -func (s *serviceViaListener) generateFreePort() (uint16, error) { - ok := s.tryToBind(s.wgInterface.Address().IP, customPort) - if ok { - return customPort, nil - } - - probeListener, err := net.ListenUDP("udp4", &net.UDPAddr{}) - if err != nil { - log.Debugf("failed to bind random port for DNS: %s", err) - return 0, err - } - - port := uint16(probeListener.LocalAddr().(*net.UDPAddr).Port) - if err = probeListener.Close(); err != nil { - log.Debugf("failed to free up DNS port: %s", err) - return 0, err - } - return port, nil + return 0, fmt.Errorf("no port free for UDP and TCP on %s after %d attempts", ip, randomPortAttempts) } diff --git a/client/internal/dns/service_listener_test.go b/client/internal/dns/service_listener_test.go index 90ef71d19..b158a79fd 100644 --- a/client/internal/dns/service_listener_test.go +++ b/client/internal/dns/service_listener_test.go @@ -1,6 +1,7 @@ package dns import ( + "errors" "fmt" "net" "net/netip" @@ -10,6 +11,8 @@ import ( "github.com/miekg/dns" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + firewall "github.com/netbirdio/netbird/client/firewall/manager" ) func TestServiceViaListener_TCPAndUDP(t *testing.T) { @@ -84,3 +87,133 @@ func TestServiceViaListener_TCPAndUDP(t *testing.T) { require.NotEmpty(t, tcpResp.Answer) assert.Contains(t, tcpResp.Answer[0].String(), "192.0.2.1", "TCP response should contain expected IP") } + +type dnatCall struct { + rule dnatRule + added bool +} + +// fakeFirewall records DNAT calls and fails the ones named in addErrs/removeErrs. +type fakeFirewall struct { + calls []dnatCall + addErrs map[firewall.Protocol]error + removeErrs map[firewall.Protocol]error +} + +func (f *fakeFirewall) AddOutputDNAT(ip netip.Addr, protocol firewall.Protocol, _, translatedPort uint16) error { + if err := f.addErrs[protocol]; err != nil { + return err + } + f.calls = append(f.calls, dnatCall{rule: dnatRule{protocol: protocol, ip: ip, port: translatedPort}, added: true}) + return nil +} + +func (f *fakeFirewall) RemoveOutputDNAT(ip netip.Addr, protocol firewall.Protocol, _, translatedPort uint16) error { + if err := f.removeErrs[protocol]; err != nil { + return err + } + f.calls = append(f.calls, dnatCall{rule: dnatRule{protocol: protocol, ip: ip, port: translatedPort}}) + return nil +} + +func newDNATTestService(fw Firewall) *serviceViaListener { + return &serviceViaListener{ + listenIP: netip.MustParseAddr("100.64.0.1"), + listenPort: customPort, + firewall: fw, + } +} + +func TestSetupDNAT_BothProtocols(t *testing.T) { + svc := newDNATTestService(&fakeFirewall{}) + + svc.setupDNAT() + + assert.Len(t, svc.dnatRules, len(dnatProtocols)) + assert.Equal(t, DefaultPort, svc.RuntimePort(), "port 53 is advertised once both redirects are installed") +} + +func TestSetupDNAT_RollsBackPartialRedirect(t *testing.T) { + fw := &fakeFirewall{addErrs: map[firewall.Protocol]error{firewall.ProtocolTCP: errors.New("nftables busy")}} + svc := newDNATTestService(fw) + + svc.setupDNAT() + + assert.Empty(t, svc.dnatRules, "the UDP redirect installed before the failure must be rolled back") + assert.Equal(t, int(svc.listenPort), svc.RuntimePort(), "an incomplete redirect must not advertise port 53") + udp := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: svc.listenPort} + assert.Contains(t, fw.calls, dnatCall{rule: udp}, "UDP removal should have been attempted") +} + +// A rollback that fails must keep the rule recorded, so port 53 is not left +// redirected to a resolver that no longer listens. +func TestStop_RetriesFailedDNATRemoval(t *testing.T) { + fw := &fakeFirewall{ + addErrs: map[firewall.Protocol]error{firewall.ProtocolTCP: errors.New("nftables busy")}, + removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")}, + } + svc := newDNATTestService(fw) + + svc.setupDNAT() + udp := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: svc.listenPort} + require.Equal(t, []dnatRule{udp}, svc.dnatRules, "a failed rollback keeps the rule for a later retry") + + require.Error(t, svc.Stop(), "the failing removal should be reported") + require.Equal(t, []dnatRule{udp}, svc.dnatRules) + + delete(fw.removeErrs, firewall.ProtocolUDP) + require.NoError(t, svc.Stop(), "a later stop retries the removal") + assert.Empty(t, svc.dnatRules) +} + +// A stale rule that cannot be removed is matched before anything added now, so +// no new redirect may be installed on top of it and port 53 must not be +// advertised as reaching this listener. +func TestSetupDNAT_AbortsWhileStaleRuleRemains(t *testing.T) { + fw := &fakeFirewall{removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")}} + svc := newDNATTestService(fw) + stalePort := svc.listenPort + + svc.setupDNAT() + require.Error(t, svc.Stop()) + staleUDP := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: stalePort} + require.Equal(t, []dnatRule{staleUDP}, svc.dnatRules) + + svc.listenPort = stalePort + 1 + fw.calls = nil + + svc.setupDNAT() + + assert.Equal(t, []dnatRule{staleUDP}, svc.dnatRules, "the stale rule stays recorded for a later attempt") + for _, call := range fw.calls { + assert.False(t, call.added, "no redirect may be installed while a stale one is still in place") + } + assert.Equal(t, int(svc.listenPort), svc.RuntimePort(), "port 53 must not be advertised") +} + +// A rule left behind by a failed removal must be removed with the address and +// port it was installed with, even when the listener has since moved to another +// port, and it must not count towards the redirect the new listener advertises. +func TestSetupDNAT_ClearsStaleRuleAfterPortChange(t *testing.T) { + fw := &fakeFirewall{removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")}} + svc := newDNATTestService(fw) + stalePort := svc.listenPort + + svc.setupDNAT() + require.Error(t, svc.Stop()) + staleUDP := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: stalePort} + require.Equal(t, []dnatRule{staleUDP}, svc.dnatRules) + + delete(fw.removeErrs, firewall.ProtocolUDP) + svc.listenPort = stalePort + 1 + fw.calls = nil + + svc.setupDNAT() + + assert.Contains(t, fw.calls, dnatCall{rule: staleUDP}, "the stale rule must be removed with its original port") + assert.Len(t, svc.dnatRules, len(dnatProtocols)) + assert.Equal(t, DefaultPort, svc.RuntimePort(), "the new listener is fully redirected") + for _, rule := range svc.dnatRules { + assert.Equal(t, svc.listenPort, rule.port, "only rules for the current listener remain") + } +} diff --git a/client/internal/ebpf/ebpf/bpf_bpfeb.go b/client/internal/ebpf/ebpf/bpf_bpfeb.go index 04b19883b..4b6230217 100644 --- a/client/internal/ebpf/ebpf/bpf_bpfeb.go +++ b/client/internal/ebpf/ebpf/bpf_bpfeb.go @@ -1,5 +1,5 @@ // Code generated by bpf2go; DO NOT EDIT. -//go:build arm64be || armbe || mips || mips64 || mips64p32 || ppc64 || s390 || s390x || sparc || sparc64 +//go:build mips || mips64 || ppc64 || s390x package ebpf @@ -47,9 +47,10 @@ func loadBpfObjects(obj interface{}, opts *ebpf.CollectionOptions) error { type bpfSpecs struct { bpfProgramSpecs bpfMapSpecs + bpfVariableSpecs } -// bpfSpecs contains programs before they are loaded into the kernel. +// bpfProgramSpecs contains programs before they are loaded into the kernel. // // It can be passed ebpf.CollectionSpec.Assign. type bpfProgramSpecs struct { @@ -61,17 +62,28 @@ type bpfProgramSpecs struct { // It can be passed ebpf.CollectionSpec.Assign. type bpfMapSpecs struct { NbFeatures *ebpf.MapSpec `ebpf:"nb_features"` - NbMapDnsIp *ebpf.MapSpec `ebpf:"nb_map_dns_ip"` - NbMapDnsPort *ebpf.MapSpec `ebpf:"nb_map_dns_port"` NbWgProxySettingsMap *ebpf.MapSpec `ebpf:"nb_wg_proxy_settings_map"` } +// bpfVariableSpecs contains global variables before they are loaded into the kernel. +// +// It can be passed ebpf.CollectionSpec.Assign. +type bpfVariableSpecs struct { + FlagFeatureWgProxy *ebpf.VariableSpec `ebpf:"flag_feature_wg_proxy"` + MapKeyFeatures *ebpf.VariableSpec `ebpf:"map_key_features"` + MapKeyProxyPort *ebpf.VariableSpec `ebpf:"map_key_proxy_port"` + MapKeyWgPort *ebpf.VariableSpec `ebpf:"map_key_wg_port"` + ProxyPort *ebpf.VariableSpec `ebpf:"proxy_port"` + WgPort *ebpf.VariableSpec `ebpf:"wg_port"` +} + // bpfObjects contains all objects after they have been loaded into the kernel. // // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. type bpfObjects struct { bpfPrograms bpfMaps + bpfVariables } func (o *bpfObjects) Close() error { @@ -86,20 +98,28 @@ func (o *bpfObjects) Close() error { // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. type bpfMaps struct { NbFeatures *ebpf.Map `ebpf:"nb_features"` - NbMapDnsIp *ebpf.Map `ebpf:"nb_map_dns_ip"` - NbMapDnsPort *ebpf.Map `ebpf:"nb_map_dns_port"` NbWgProxySettingsMap *ebpf.Map `ebpf:"nb_wg_proxy_settings_map"` } func (m *bpfMaps) Close() error { return _BpfClose( m.NbFeatures, - m.NbMapDnsIp, - m.NbMapDnsPort, m.NbWgProxySettingsMap, ) } +// bpfVariables contains all global variables after they have been loaded into the kernel. +// +// It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. +type bpfVariables struct { + FlagFeatureWgProxy *ebpf.Variable `ebpf:"flag_feature_wg_proxy"` + MapKeyFeatures *ebpf.Variable `ebpf:"map_key_features"` + MapKeyProxyPort *ebpf.Variable `ebpf:"map_key_proxy_port"` + MapKeyWgPort *ebpf.Variable `ebpf:"map_key_wg_port"` + ProxyPort *ebpf.Variable `ebpf:"proxy_port"` + WgPort *ebpf.Variable `ebpf:"wg_port"` +} + // bpfPrograms contains all programs after they have been loaded into the kernel. // // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. diff --git a/client/internal/ebpf/ebpf/bpf_bpfeb.o b/client/internal/ebpf/ebpf/bpf_bpfeb.o index 7433ad740..b435d4964 100644 Binary files a/client/internal/ebpf/ebpf/bpf_bpfeb.o and b/client/internal/ebpf/ebpf/bpf_bpfeb.o differ diff --git a/client/internal/ebpf/ebpf/bpf_bpfel.go b/client/internal/ebpf/ebpf/bpf_bpfel.go index 03b494aa2..f56efc901 100644 --- a/client/internal/ebpf/ebpf/bpf_bpfel.go +++ b/client/internal/ebpf/ebpf/bpf_bpfel.go @@ -1,5 +1,5 @@ // Code generated by bpf2go; DO NOT EDIT. -//go:build 386 || amd64 || amd64p32 || arm || arm64 || loong64 || mips64le || mips64p32le || mipsle || ppc64le || riscv64 +//go:build 386 || amd64 || arm || arm64 || loong64 || mips64le || mipsle || ppc64le || riscv64 || wasm package ebpf @@ -47,9 +47,10 @@ func loadBpfObjects(obj interface{}, opts *ebpf.CollectionOptions) error { type bpfSpecs struct { bpfProgramSpecs bpfMapSpecs + bpfVariableSpecs } -// bpfSpecs contains programs before they are loaded into the kernel. +// bpfProgramSpecs contains programs before they are loaded into the kernel. // // It can be passed ebpf.CollectionSpec.Assign. type bpfProgramSpecs struct { @@ -61,17 +62,28 @@ type bpfProgramSpecs struct { // It can be passed ebpf.CollectionSpec.Assign. type bpfMapSpecs struct { NbFeatures *ebpf.MapSpec `ebpf:"nb_features"` - NbMapDnsIp *ebpf.MapSpec `ebpf:"nb_map_dns_ip"` - NbMapDnsPort *ebpf.MapSpec `ebpf:"nb_map_dns_port"` NbWgProxySettingsMap *ebpf.MapSpec `ebpf:"nb_wg_proxy_settings_map"` } +// bpfVariableSpecs contains global variables before they are loaded into the kernel. +// +// It can be passed ebpf.CollectionSpec.Assign. +type bpfVariableSpecs struct { + FlagFeatureWgProxy *ebpf.VariableSpec `ebpf:"flag_feature_wg_proxy"` + MapKeyFeatures *ebpf.VariableSpec `ebpf:"map_key_features"` + MapKeyProxyPort *ebpf.VariableSpec `ebpf:"map_key_proxy_port"` + MapKeyWgPort *ebpf.VariableSpec `ebpf:"map_key_wg_port"` + ProxyPort *ebpf.VariableSpec `ebpf:"proxy_port"` + WgPort *ebpf.VariableSpec `ebpf:"wg_port"` +} + // bpfObjects contains all objects after they have been loaded into the kernel. // // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. type bpfObjects struct { bpfPrograms bpfMaps + bpfVariables } func (o *bpfObjects) Close() error { @@ -86,20 +98,28 @@ func (o *bpfObjects) Close() error { // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. type bpfMaps struct { NbFeatures *ebpf.Map `ebpf:"nb_features"` - NbMapDnsIp *ebpf.Map `ebpf:"nb_map_dns_ip"` - NbMapDnsPort *ebpf.Map `ebpf:"nb_map_dns_port"` NbWgProxySettingsMap *ebpf.Map `ebpf:"nb_wg_proxy_settings_map"` } func (m *bpfMaps) Close() error { return _BpfClose( m.NbFeatures, - m.NbMapDnsIp, - m.NbMapDnsPort, m.NbWgProxySettingsMap, ) } +// bpfVariables contains all global variables after they have been loaded into the kernel. +// +// It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. +type bpfVariables struct { + FlagFeatureWgProxy *ebpf.Variable `ebpf:"flag_feature_wg_proxy"` + MapKeyFeatures *ebpf.Variable `ebpf:"map_key_features"` + MapKeyProxyPort *ebpf.Variable `ebpf:"map_key_proxy_port"` + MapKeyWgPort *ebpf.Variable `ebpf:"map_key_wg_port"` + ProxyPort *ebpf.Variable `ebpf:"proxy_port"` + WgPort *ebpf.Variable `ebpf:"wg_port"` +} + // bpfPrograms contains all programs after they have been loaded into the kernel. // // It can be passed to loadBpfObjects or ebpf.CollectionSpec.LoadAndAssign. diff --git a/client/internal/ebpf/ebpf/bpf_bpfel.o b/client/internal/ebpf/ebpf/bpf_bpfel.o index 779f43a00..a388b6d6d 100644 Binary files a/client/internal/ebpf/ebpf/bpf_bpfel.o and b/client/internal/ebpf/ebpf/bpf_bpfel.o differ diff --git a/client/internal/ebpf/ebpf/dns_fwd_linux.go b/client/internal/ebpf/ebpf/dns_fwd_linux.go deleted file mode 100644 index 1e7774573..000000000 --- a/client/internal/ebpf/ebpf/dns_fwd_linux.go +++ /dev/null @@ -1,52 +0,0 @@ -package ebpf - -import ( - "encoding/binary" - "fmt" - "net/netip" - - log "github.com/sirupsen/logrus" -) - -const ( - mapKeyDNSIP uint32 = 0 - mapKeyDNSPort uint32 = 1 -) - -func (tf *GeneralManager) LoadDNSFwd(ip netip.Addr, dnsPort int) error { - log.Debugf("load eBPF DNS forwarder, watching addr: %s:53, redirect to port: %d", ip, dnsPort) - tf.lock.Lock() - defer tf.lock.Unlock() - - err := tf.loadXdp() - if err != nil { - return err - } - - if !ip.Is4() { - return fmt.Errorf("eBPF DNS forwarder only supports IPv4, got %s", ip) - } - ip4 := ip.As4() - err = tf.bpfObjs.NbMapDnsIp.Put(mapKeyDNSIP, binary.BigEndian.Uint32(ip4[:])) - if err != nil { - return err - } - - err = tf.bpfObjs.NbMapDnsPort.Put(mapKeyDNSPort, uint16(dnsPort)) - if err != nil { - return err - } - - tf.setFeatureFlag(featureFlagDnsForwarder) - err = tf.bpfObjs.NbFeatures.Put(mapKeyFeatures, tf.featureFlags) - if err != nil { - return err - } - return nil -} - -func (tf *GeneralManager) FreeDNSFwd() error { - log.Debugf("free ebpf DNS forwarder") - return tf.unsetFeatureFlag(featureFlagDnsForwarder) -} - diff --git a/client/internal/ebpf/ebpf/manager_linux.go b/client/internal/ebpf/ebpf/manager_linux.go index 7520a6387..a13f5f19a 100644 --- a/client/internal/ebpf/ebpf/manager_linux.go +++ b/client/internal/ebpf/ebpf/manager_linux.go @@ -15,8 +15,7 @@ import ( const ( mapKeyFeatures uint32 = 0 - featureFlagWGProxy = 0b00000001 - featureFlagDnsForwarder = 0b00000010 + featureFlagWGProxy = 0b00000001 ) var ( @@ -28,9 +27,9 @@ var ( // GeneralManager is used to load multiple eBPF programs with a custom check (if then) done in prog.c // The manager simply adds a feature (byte) of each program to a map that is shared between the userspace and kernel. -// When packet arrives, the C code checks for each feature (if it is set) and executes each enabled program (e.g., dns_fwd.c and wg_proxy.c). +// When packet arrives, the C code checks for each feature (if it is set) and executes each enabled program (e.g., wg_proxy.c). // -//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang-14 bpf src/prog.c -- -I /usr/x86_64-linux-gnu/include +//go:generate go run github.com/cilium/ebpf/cmd/bpf2go -cc clang-14 bpf src/prog.c -- -I /usr/x86_64-linux-gnu/include -include src/bpf_map_def.h type GeneralManager struct { lock sync.Mutex link link.Link diff --git a/client/internal/ebpf/ebpf/manager_linux_test.go b/client/internal/ebpf/ebpf/manager_linux_test.go index 5664a4565..e09fcb977 100644 --- a/client/internal/ebpf/ebpf/manager_linux_test.go +++ b/client/internal/ebpf/ebpf/manager_linux_test.go @@ -7,33 +7,24 @@ import ( func TestManager_setFeatureFlag(t *testing.T) { mgr := GeneralManager{} mgr.setFeatureFlag(featureFlagWGProxy) - if mgr.featureFlags != 1 { + if mgr.featureFlags != featureFlagWGProxy { t.Errorf("invalid feature state") } - mgr.setFeatureFlag(featureFlagDnsForwarder) - if mgr.featureFlags != 3 { - t.Errorf("invalid feature state") + mgr.setFeatureFlag(featureFlagWGProxy) + if mgr.featureFlags != featureFlagWGProxy { + t.Errorf("setting a flag twice must be idempotent, got: %d", mgr.featureFlags) } } func TestManager_unsetFeatureFlag(t *testing.T) { mgr := GeneralManager{} mgr.setFeatureFlag(featureFlagWGProxy) - mgr.setFeatureFlag(featureFlagDnsForwarder) err := mgr.unsetFeatureFlag(featureFlagWGProxy) if err != nil { t.Errorf("unexpected error: %s", err) } - if mgr.featureFlags != 2 { - t.Errorf("invalid feature state, expected: %d, got: %d", 2, mgr.featureFlags) - } - - err = mgr.unsetFeatureFlag(featureFlagDnsForwarder) - if err != nil { - t.Errorf("unexpected error: %s", err) - } if mgr.featureFlags != 0 { t.Errorf("invalid feature state, expected: %d, got: %d", 0, mgr.featureFlags) } diff --git a/client/internal/ebpf/ebpf/src/bpf_map_def.h b/client/internal/ebpf/ebpf/src/bpf_map_def.h new file mode 100644 index 000000000..9528fb592 --- /dev/null +++ b/client/internal/ebpf/ebpf/src/bpf_map_def.h @@ -0,0 +1,16 @@ +// libbpf 1.0 removed struct bpf_map_def, but the programs here keep the legacy +// map definitions: they load on kernels built without BTF, which BTF-style +// (SEC(".maps")) definitions do not. Define the struct ourselves so the +// programs compile against current libbpf headers. +#ifndef NB_BPF_MAP_DEF_H +#define NB_BPF_MAP_DEF_H + +struct bpf_map_def { + unsigned int type; + unsigned int key_size; + unsigned int value_size; + unsigned int max_entries; + unsigned int map_flags; +}; + +#endif diff --git a/client/internal/ebpf/ebpf/src/dns_fwd.c b/client/internal/ebpf/ebpf/src/dns_fwd.c deleted file mode 100644 index 9f8de2001..000000000 --- a/client/internal/ebpf/ebpf/src/dns_fwd.c +++ /dev/null @@ -1,67 +0,0 @@ -const __u32 map_key_dns_ip = 0; -const __u32 map_key_dns_port = 1; - -struct bpf_map_def SEC("maps") nb_map_dns_ip = { - .type = BPF_MAP_TYPE_ARRAY, - .key_size = sizeof(__u32), - .value_size = sizeof(__u32), - .max_entries = 10, -}; - -struct bpf_map_def SEC("maps") nb_map_dns_port = { - .type = BPF_MAP_TYPE_ARRAY, - .key_size = sizeof(__u32), - .value_size = sizeof(__u16), - .max_entries = 10, -}; - -__be32 dns_ip = 0; -__be16 dns_port = 0; - -// 13568 is 53 in big endian -__be16 GENERAL_DNS_PORT = 13568; - -bool read_settings() { - __u16 *port_value; - __u32 *ip_value; - - // read dns ip - ip_value = bpf_map_lookup_elem(&nb_map_dns_ip, &map_key_dns_ip); - if(!ip_value) { - return false; - } - dns_ip = htonl(*ip_value); - - // read dns port - port_value = bpf_map_lookup_elem(&nb_map_dns_port, &map_key_dns_port); - if (!port_value) { - return false; - } - dns_port = htons(*port_value); - return true; -} - -int xdp_dns_fwd(struct iphdr *ip, struct udphdr *udp) { - if (dns_port == 0) { - if(!read_settings()){ - return XDP_PASS; - } - // bpf_printk("dns port: %d", ntohs(dns_port)); - // bpf_printk("dns ip: %d", ntohl(dns_ip)); - } - - if (udp->dest == GENERAL_DNS_PORT && ip->daddr == dns_ip) { - udp->dest = dns_port; - // Clear the now-stale checksum; zero means "not computed" for IPv4. - udp->check = 0; - return XDP_PASS; - } - - if (udp->source == dns_port && ip->saddr == dns_ip) { - udp->source = GENERAL_DNS_PORT; - udp->check = 0; - return XDP_PASS; - } - - return XDP_PASS; -} diff --git a/client/internal/ebpf/ebpf/src/prog.c b/client/internal/ebpf/ebpf/src/prog.c index f32103f28..44ee53458 100644 --- a/client/internal/ebpf/ebpf/src/prog.c +++ b/client/internal/ebpf/ebpf/src/prog.c @@ -5,11 +5,9 @@ #include #include #include -#include "dns_fwd.c" #include "wg_proxy.c" const __u16 flag_feature_wg_proxy = 0b01; -const __u16 flag_feature_dns_fwd = 0b10; const __u32 map_key_features = 0; struct bpf_map_def SEC("maps") nb_features = { @@ -48,10 +46,6 @@ int nb_xdp_prog(struct xdp_md *ctx) { return XDP_PASS; } - if (*features & flag_feature_dns_fwd) { - xdp_dns_fwd(ip, udp); - } - if (*features & flag_feature_wg_proxy) { xdp_wg_proxy(ip, udp); } diff --git a/client/internal/ebpf/ebpf/src/readme.md b/client/internal/ebpf/ebpf/src/readme.md index 0ab393dd4..aa47847da 100644 --- a/client/internal/ebpf/ebpf/src/readme.md +++ b/client/internal/ebpf/ebpf/src/readme.md @@ -1,8 +1,18 @@ -# DNS forwarder +# XDP programs -The agent attach the XDP program to the lo device. We can not use fake address in eBPF because the -traffic does not appear in the eBPF program. The program capture the traffic on wg_ip:53 and -overwrite in it the destination port to 5053. +`prog.c` is attached to the `lo` device and dispatches to the features enabled in the +`nb_features` map. The only feature is the WireGuard proxy (`wg_proxy.c`): it rewrites +loopback UDP sent from the WireGuard listen port so it reaches the userspace relay proxy +port instead, and swaps the peer endpoint port into the source so the proxy can tell +peers apart. + +Maps use the legacy `struct bpf_map_def` form, defined in `bpf_map_def.h` because libbpf +1.0 removed it. They load on kernels built without BTF, which BTF-style (`SEC(".maps")`) +definitions do not. + +Regenerate the objects with `go generate ./client/internal/ebpf/ebpf/`; it needs +`clang-14`. Loading a regenerated object needs root, attaching it needs `bpf_link` +(kernel >= 5.7), and only one XDP program can own `lo` at a time. # Debug diff --git a/client/internal/ebpf/manager/manager.go b/client/internal/ebpf/manager/manager.go index 25a767090..fdc5d8d82 100644 --- a/client/internal/ebpf/manager/manager.go +++ b/client/internal/ebpf/manager/manager.go @@ -1,11 +1,7 @@ package manager -import "net/netip" - -// Manager is used to load multiple eBPF programs. E.g., current DNS programs and WireGuard proxy +// Manager is used to load multiple eBPF programs. E.g., the WireGuard proxy type Manager interface { - LoadDNSFwd(ip netip.Addr, dnsPort int) error - FreeDNSFwd() error LoadWgProxy(proxyPort, wgPort int) error FreeWGProxy() error } diff --git a/client/internal/engine.go b/client/internal/engine.go index f8b65f7d8..d517d1d68 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -14,12 +14,14 @@ import ( "sort" "strings" "sync" + "sync/atomic" "time" "github.com/hashicorp/go-multierror" "github.com/pion/ice/v4" "github.com/pion/stun/v3" log "github.com/sirupsen/logrus" + wgdevice "golang.zx2c4.com/wireguard/device" "golang.zx2c4.com/wireguard/tun/netstack" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -236,6 +238,12 @@ type Engine struct { wgInterface WGIface + // wgDevice is a lock-free handle on the WireGuard device behind + // wgInterface. Reaching the device through wgInterface requires + // syncMsgMux, which handleSync holds while it adds and removes peers; + // SetPerformance must stay reachable exactly when that work is stuck. + wgDevice atomic.Pointer[wgdevice.Device] + udpMux *udpmux.UniversalUDPMuxDefault // networkSerial is the latest CurrentSerial (state ID) of the network sent by the Management service @@ -651,6 +659,7 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL) log.Errorf("failed to pull up wgInterface [%s]: %s", e.wgInterface.Name(), err.Error()) return fmt.Errorf("up wg interface: %w", err) } + e.wgDevice.Store(e.wgInterface.GetWGDevice()) // Set up notrack rules immediately after proxy is listening to prevent // conntrack entries from being created before the rules are in place @@ -2144,6 +2153,10 @@ func (e *Engine) close() { log.Debugf("removing Netbird interface %s", e.config.WgIfaceName) if e.wgInterface != nil { + // Drop the handle before the close starts: a retune that loads it + // afterwards would touch a device on its way out and report success + // for an engine that is already gone. + e.wgDevice.Store(nil) if err := e.wgInterface.Close(); err != nil { log.Errorf("failed closing Netbird interface %s %v", e.config.WgIfaceName, err) } @@ -2303,15 +2316,16 @@ type Performance struct { } // SetPerformance applies the given tuning to this engine's live Device. +// +// It deliberately does not take syncMsgMux. Raising the buffer pool cap is the +// recovery path for a device whose pool is exhausted, and an exhausted pool +// blocks peer removal inside handleSync, which holds syncMsgMux for as long as +// it stays blocked. Taking the lock here would make the retune unreachable in +// the one situation that needs it. func (e *Engine) SetPerformance(t Performance) error { - e.syncMsgMux.Lock() - defer e.syncMsgMux.Unlock() - if e.wgInterface == nil { - return fmt.Errorf("wg interface not initialized") - } - dev := e.wgInterface.GetWGDevice() + dev := e.wgDevice.Load() if dev == nil { - return fmt.Errorf("wg device not initialized") + return errors.New("wg device not initialized") } if t.PreallocatedBuffersPerPool != nil { dev.SetPreallocatedBuffersPerPool(*t.PreallocatedBuffersPerPool) diff --git a/client/internal/peer/handshaker.go b/client/internal/peer/handshaker.go index 6ecb2a947..654e32158 100644 --- a/client/internal/peer/handshaker.go +++ b/client/internal/peer/handshaker.go @@ -116,7 +116,7 @@ func (h *Handshaker) Listen(ctx context.Context) { for { select { case remoteOfferAnswer := <-h.remoteOffersCh: - h.log.Infof("received offer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials()) + h.log.Infof("received offer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t, relay server: %s, relay IP: %s", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials(), remoteOfferAnswer.RelaySrvAddress, remoteOfferAnswer.RelaySrvIP) // Record signaling received for reconnection attempts if h.metricsStages != nil { @@ -138,7 +138,7 @@ func (h *Handshaker) Listen(ctx context.Context) { continue } case remoteOfferAnswer := <-h.remoteAnswerCh: - h.log.Infof("received answer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials()) + h.log.Infof("received answer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t, relay server: %s, relay IP: %s", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials(), remoteOfferAnswer.RelaySrvAddress, remoteOfferAnswer.RelaySrvIP) // Record signaling received for reconnection attempts if h.metricsStages != nil { @@ -209,14 +209,14 @@ func (h *Handshaker) sendOffer() error { } offer := h.buildOfferAnswer() - h.log.Debugf("sending offer with serial: %s", offer.SessionIDString()) + h.log.Debugf("sending offer with serial: %s, relay server: %s, relay IP: %s", offer.SessionIDString(), offer.RelaySrvAddress, offer.RelaySrvIP) return h.signaler.SignalOffer(offer, h.config.Key) } func (h *Handshaker) sendAnswer() error { answer := h.buildOfferAnswer() - h.log.Debugf("sending answer with serial: %s", answer.SessionIDString()) + h.log.Debugf("sending answer with serial: %s, relay server: %s, relay IP: %s", answer.SessionIDString(), answer.RelaySrvAddress, answer.RelaySrvIP) return h.signaler.SignalAnswer(answer, h.config.Key) } diff --git a/client/internal/peer/status.go b/client/internal/peer/status.go index bf36b944b..d753ee43e 100644 --- a/client/internal/peer/status.go +++ b/client/internal/peer/status.go @@ -819,8 +819,8 @@ func (d *Status) SetSessionExpiresAt(deadline time.Time) { // "none" would blank the UI at the exact moment it should say the session // ended. func (d *Status) GetSessionExpiresAt() time.Time { - d.mux.Lock() - defer d.mux.Unlock() + d.mux.RLock() + defer d.mux.RUnlock() return d.sessionExpiresAt } diff --git a/client/internal/profilemanager/invoking_user.go b/client/internal/profilemanager/invoking_user.go index c86a6ce43..7ba612ffb 100644 --- a/client/internal/profilemanager/invoking_user.go +++ b/client/internal/profilemanager/invoking_user.go @@ -6,6 +6,7 @@ import ( "os/user" "path/filepath" "runtime" + "strconv" log "github.com/sirupsen/logrus" ) @@ -13,17 +14,21 @@ import ( const envSudoUser = "SUDO_USER" var ( - geteuid = os.Geteuid - lookupUser = user.Lookup + currentUser = user.Current + getegid = os.Getegid + geteuid = os.Geteuid + lookupUser = user.Lookup ) // InvokingUser returns the user a CLI invocation acts for. Under sudo that is // the user who ran sudo, not root: privileged flags force commands through // sudo, and resolving profiles as root would silently switch the daemon to -// root's (default) profile instead of the invoking user's. Privilege decisions -// are not made here — those stay on the kernel credentials of the daemon -// connection, which SUDO_USER (a plain environment variable) can never -// influence; a forged value only selects a profile root could select anyway. +// root's (default) profile instead of the invoking user's. An unmapped positive +// process UID uses its numeric kernel identity; root, sudo lookup failures, and +// unavailable platform identities still fail closed. Privilege decisions stay +// on the kernel credentials of the daemon connection, which SUDO_USER (a plain +// environment variable) can never influence; a forged value only selects a +// profile root could select anyway. func InvokingUser() (*user.User, error) { if u, ok := sudoInvokingUser(); ok { return u, nil @@ -35,7 +40,23 @@ func InvokingUser() (*user.User, error) { if sudoActive() { return nil, fmt.Errorf("resolve sudo invoking user %q: refusing to fall back to root", os.Getenv(envSudoUser)) } - return user.Current() + u, err := currentUser() + if err == nil { + return u, nil + } + + uid := geteuid() + if uid <= 0 { + return nil, err + } + + log.Debugf("current user lookup for UID %d: %v; using numeric UID", uid, err) + uidString := strconv.Itoa(uid) + return &user.User{ + Username: uidString, + Uid: uidString, + Gid: strconv.Itoa(getegid()), + }, nil } // IsPlainRoot reports that the process runs as root with no usable sudo diff --git a/client/internal/profilemanager/invoking_user_test.go b/client/internal/profilemanager/invoking_user_test.go index 54c8ad8fd..159d2616b 100644 --- a/client/internal/profilemanager/invoking_user_test.go +++ b/client/internal/profilemanager/invoking_user_test.go @@ -2,6 +2,7 @@ package profilemanager import ( "errors" + "fmt" "io/fs" "os" "os/user" @@ -21,7 +22,51 @@ func TestInvokingUserFallsBackToProcessUser(t *testing.T) { current, err := user.Current() require.NoError(t, err) - assert.Equal(t, current.Username, got.Username) + assert.Equal(t, current.Username, got.Username, "invoking user should match the process user without sudo") +} + +func TestInvokingUserFailsClosedWithoutPositiveUID(t *testing.T) { + for _, uid := range []int{0, -1} { + t.Run(fmt.Sprintf("UID%d", uid), func(t *testing.T) { + t.Setenv(envSudoUser, "") + lookupErr := errors.New("current user unavailable") + fakeUnmappedUser(t, uid, 0, lookupErr) + + got, err := InvokingUser() + require.ErrorIs(t, err, lookupErr) + assert.Nil(t, got, "root or unavailable UID must not become a synthetic identity") + }) + } +} + +func TestProfileFilePathUsesNumericIdentityForUnmappedNonRoot(t *testing.T) { + t.Setenv(envSudoUser, "") + fakeUnmappedUser(t, 1001230000, 0, errors.New("user: unknown userid 1001230000")) + + profilesRoot := t.TempDir() + origDir := DefaultConfigPathDir + origOverride := ConfigDirOverride + DefaultConfigPathDir = profilesRoot + ConfigDirOverride = "" + t.Cleanup(func() { + DefaultConfigPathDir = origDir + ConfigDirOverride = origOverride + }) + + profileID := ID("0123456789abcdef0123456789abcdef") + got, err := (&Profile{ID: profileID}).FilePath() + require.NoError(t, err) + assert.Equal(t, + filepath.Join(profilesRoot, "1001230000", profileID.String()+".json"), + got, + "profile path should use the numeric UID namespace", + ) + + entries, err := os.ReadDir(profilesRoot) + require.NoError(t, err) + require.Len(t, entries, 1, "only the numeric UID directory should be created") + assert.Equal(t, "1001230000", entries[0].Name(), "profile namespace should be numeric") + assert.True(t, entries[0].IsDir(), "profile namespace should be a directory") } func TestSudoInvokingUserInactiveWithoutSudoContext(t *testing.T) { @@ -60,6 +105,13 @@ func TestInvokingUserFailsClosedWhenSudoLookupFails(t *testing.T) { fakeSudo(t, filepath.Join("/home", "misha")) lookupUser = func(string) (*user.User, error) { return nil, errors.New("nss unavailable") } + origCurrentUser := currentUser + currentUser = func() (*user.User, error) { + t.Fatal("currentUser must not be called after a sudo lookup failure") + return nil, errors.New("currentUser called unexpectedly") + } + t.Cleanup(func() { currentUser = origCurrentUser }) + got, err := InvokingUser() require.Error(t, err) assert.Nil(t, got, "must not resolve to the root process user") @@ -215,6 +267,22 @@ func fakeSudo(t *testing.T, home string) { }) } +func fakeUnmappedUser(t *testing.T, uid, gid int, lookupErr error) { + t.Helper() + + origCurrentUser := currentUser + origEuid := geteuid + origEgid := getegid + currentUser = func() (*user.User, error) { return nil, lookupErr } + geteuid = func() int { return uid } + getegid = func() int { return gid } + t.Cleanup(func() { + currentUser = origCurrentUser + geteuid = origEuid + getegid = origEgid + }) +} + func assertNoEntries(t *testing.T, root string) { t.Helper() err := filepath.WalkDir(root, func(path string, _ fs.DirEntry, err error) error { diff --git a/client/mdm/conflicts.go b/client/mdm/conflicts.go index a04cfb05c..160212afb 100644 --- a/client/mdm/conflicts.go +++ b/client/mdm/conflicts.go @@ -1,6 +1,10 @@ package mdm -import "net/url" +import ( + "net/url" + + "github.com/netbirdio/netbird/util" +) // PreSharedKeyRedactedSentinel is the redaction mask returned in place of a // real pre-shared key; an incoming value equal to it is a round-trip echo, @@ -44,8 +48,9 @@ func ConflictStringPtr(key string, p *string) ConflictCheck { } } -// ConflictURL builds a ConflictCheck for a URL-typed MDM key; both sides are -// normalized via CanonicalURL before comparison. +// ConflictURL builds a ConflictCheck for a URL-typed MDM key. The two sides are +// compared as the endpoints they address, not as strings: see +// util.SameServiceURL. func ConflictURL(key, got string) ConflictCheck { return ConflictCheck{ Key: key, @@ -54,7 +59,7 @@ func ConflictURL(key, got string) ConflictCheck { return true } want, ok := pol.GetString(key) - return ok && CanonicalURL(want) == CanonicalURL(got) + return ok && util.SameServiceURLStrings(want, got) }, } } diff --git a/client/mdm/conflicts_test.go b/client/mdm/conflicts_test.go new file mode 100644 index 000000000..d145ec103 --- /dev/null +++ b/client/mdm/conflicts_test.go @@ -0,0 +1,40 @@ +package mdm + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The same spellings, through the conflict check that decides whether a request +// is refused. An enforced URL restated in another spelling addresses the very +// server the policy names, so it must not be reported as a conflict. +func TestConflictURLComparesEndpoints(t *testing.T) { + policy := NewPolicy(map[string]any{KeyManagementURL: "https://mgmt.example.com"}) + require.True(t, policy.HasKey(KeyManagementURL)) + + for _, restated := range []string{ + "https://mgmt.example.com", + "https://mgmt.example.com:443", + "https://mgmt.example.com/", + "https://MGMT.example.com", + "https://mgmt.example.com:0443", + } { + conflicts := ResolveConflicts(policy, []ConflictCheck{ConflictURL(KeyManagementURL, restated)}) + assert.Empty(t, conflicts, "%q is the enforced endpoint written differently", restated) + } + + for _, diverging := range []string{ + "https://other.example.com", + "http://mgmt.example.com", + "https://mgmt.example.com:8443", + "https://mgmt.example.com/other", + } { + conflicts := ResolveConflicts(policy, []ConflictCheck{ConflictURL(KeyManagementURL, diverging)}) + assert.Equal(t, []string{KeyManagementURL}, conflicts, "%q addresses another endpoint", diverging) + } + + // An unset field is not a request to change anything. + assert.Empty(t, ResolveConflicts(policy, []ConflictCheck{ConflictURL(KeyManagementURL, "")})) +} diff --git a/client/mdm/policy.go b/client/mdm/policy.go index c57c5303e..638fa0d80 100644 --- a/client/mdm/policy.go +++ b/client/mdm/policy.go @@ -235,6 +235,8 @@ func (p *Policy) GetBool(key string) (bool, bool) { return t != 0, true case int64: return t != 0, true + case float64: + return t != 0, true } return false, false } diff --git a/client/mdm/policy_test.go b/client/mdm/policy_test.go index 177fcd550..ea467f861 100644 --- a/client/mdm/policy_test.go +++ b/client/mdm/policy_test.go @@ -96,7 +96,8 @@ func TestPolicy_GetBool(t *testing.T) { {"int64 nonzero", int64(2), true, true}, {"int64 zero", int64(0), false, true}, {"string garbage", "maybe", false, false}, - {"float unsupported", 1.0, false, false}, + {"float nonzero", 1.0, true, true}, + {"float zero", 0.0, false, true}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -156,6 +157,20 @@ func TestPolicy_GetStringSlice(t *testing.T) { }) } +// encoding/json decodes every JSON number into float64, so the mobile +// loaders never see int. +func TestJSONLoader_BoolFromNumber(t *testing.T) { + p := NewJSONLoader(func() string { return `{"blockInbound":1,"disableProfiles":0}` }).Load() + + got, ok := p.GetBool(KeyBlockInbound) + assert.True(t, ok) + assert.True(t, got) + + got, ok = p.GetBool(KeyDisableProfiles) + assert.True(t, ok) + assert.False(t, got) +} + func TestLoader_NilFetcherReturnsEmpty(t *testing.T) { // Loader.Load with no fetcher (desktop construction) must degrade // gracefully and never return nil; on linux loadPlatform is a stub diff --git a/client/mdm/restrictions.go b/client/mdm/restrictions.go index c8e443395..200756b78 100644 --- a/client/mdm/restrictions.go +++ b/client/mdm/restrictions.go @@ -20,6 +20,7 @@ type Fields struct { DisableMetricsCollection bool `json:"disableMetricsCollection"` SplitTunnelMode bool `json:"splitTunnelMode"` SplitTunnelApps bool `json:"splitTunnelApps"` + RemoteJobsAllowed bool `json:"allowRemoteJobs"` DisableAdvancedView *bool `json:"disableAdvancedView"` } @@ -60,6 +61,7 @@ func BuildRestrictions(policy *Policy) Restrictions { r.MDM.DisableMetricsCollection = policy.HasKey(KeyDisableMetricsCollection) r.MDM.SplitTunnelMode = policy.HasKey(KeySplitTunnelMode) r.MDM.SplitTunnelApps = policy.HasKey(KeySplitTunnelApps) + r.MDM.RemoteJobsAllowed = policy.HasKey(KeyRemoteJobsAllowed) if v, ok := policy.GetBool(KeyAllowServerSSH); ok { r.MDM.AllowServerSSH = &v } diff --git a/client/ui/frontend/index.html b/client/ui/frontend/index.html index e62139956..f4c8b3d68 100644 --- a/client/ui/frontend/index.html +++ b/client/ui/frontend/index.html @@ -6,7 +6,25 @@ NetBird +
diff --git a/client/ui/frontend/src/app.tsx b/client/ui/frontend/src/app.tsx index 7f1359510..6accda36f 100644 --- a/client/ui/frontend/src/app.tsx +++ b/client/ui/frontend/src/app.tsx @@ -13,6 +13,7 @@ import { SkeletonTheme } from "react-loading-skeleton"; import "react-loading-skeleton/dist/skeleton.css"; import { welcome } from "@/lib/welcome"; import LoginWaitingForBrowserDialog from "@/modules/login/LoginWaitingForBrowserDialog.tsx"; +import { ThemeProvider } from "@/contexts/ThemeContext.tsx"; import { initI18n } from "@/lib/i18n"; import { initPlatform } from "@/lib/platform"; import { initLogForwarding } from "@/lib/logs"; @@ -35,30 +36,38 @@ Promise.all([ ]).finally(() => { ReactDOM.createRoot(document.getElementById("root")!).render( - - - - - } - /> - } /> - } - /> - } /> - } /> - - }> - } /> - } /> - } /> - - - - + + + + + + } + /> + } + /> + } + /> + } /> + } /> + + }> + } /> + } /> + } /> + + + + + , ); }); diff --git a/client/ui/frontend/src/assets/logos/netbird-full-light.svg b/client/ui/frontend/src/assets/logos/netbird-full-light.svg new file mode 100644 index 000000000..3457b50c6 --- /dev/null +++ b/client/ui/frontend/src/assets/logos/netbird-full-light.svg @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + diff --git a/client/ui/frontend/src/components/Badge.tsx b/client/ui/frontend/src/components/Badge.tsx index c5e2b5f22..a6f2e886a 100644 --- a/client/ui/frontend/src/components/Badge.tsx +++ b/client/ui/frontend/src/components/Badge.tsx @@ -11,12 +11,14 @@ type Props = HTMLAttributes & { }; const VARIANT_CLASSES: Record = { - info: "bg-sky-900 border border-sky-700 text-sky-200", + info: "bg-sky-100 border border-sky-300 text-sky-800 dark:bg-sky-900 dark:border-sky-700 dark:text-sky-200", neutral: "bg-nb-gray-900 border border-nb-gray-850 text-nb-gray-200", - brand: "bg-netbird/15 border border-netbird/30 text-netbird", - success: "bg-green-900 border border-green-700 text-green-200", - warning: "bg-yellow-900 border border-yellow-700 text-yellow-200", - danger: "bg-red-900 border border-red-700 text-red-200", + brand: "bg-netbird/15 border border-netbird/30 text-netbird-700 dark:text-netbird", + success: + "bg-green-100 border border-green-300 text-green-800 dark:bg-green-900 dark:border-green-700 dark:text-green-200", + warning: + "bg-yellow-100 border border-yellow-300 text-yellow-800 dark:bg-yellow-900 dark:border-yellow-700 dark:text-yellow-200", + danger: "bg-red-100 border border-red-300 text-red-800 dark:bg-red-900 dark:border-red-700 dark:text-red-200", }; export const Badge = forwardRef(function Badge( diff --git a/client/ui/frontend/src/components/CopyToClipboard.tsx b/client/ui/frontend/src/components/CopyToClipboard.tsx index 3cf681a1c..4af4ecc8f 100644 --- a/client/ui/frontend/src/components/CopyToClipboard.tsx +++ b/client/ui/frontend/src/components/CopyToClipboard.tsx @@ -81,7 +81,7 @@ export const CopyToClipboard = ({ aria-live={"polite"} className={cn( "group/copy wails-no-draggable pointer-events-auto inline-flex cursor-default items-center gap-2 rounded-sm text-left outline-none", - "focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", + "focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", className, )} > diff --git a/client/ui/frontend/src/components/DropdownMenu.tsx b/client/ui/frontend/src/components/DropdownMenu.tsx index d43c37e1b..8cedcea03 100644 --- a/client/ui/frontend/src/components/DropdownMenu.tsx +++ b/client/ui/frontend/src/components/DropdownMenu.tsx @@ -16,7 +16,7 @@ const menuItemVariants = cva("", { variant: { default: "text-nb-gray-200 hover:bg-nb-gray-900 hover:text-nb-gray-50 focus-visible:bg-nb-gray-900 focus-visible:text-nb-gray-50 data-[state=open]:bg-nb-gray-900 data-[state=open]:text-nb-gray-50", - danger: "text-red-500 hover:bg-red-900/20 hover:text-red-500 focus-visible:bg-red-900/20 focus-visible:text-red-500", + danger: "text-red-500 hover:bg-red-500/10 hover:text-red-500 focus-visible:bg-red-500/10 focus-visible:text-red-500 dark:hover:bg-red-900/20 dark:focus-visible:bg-red-900/20", }, }, defaultVariants: { variant: "default" }, diff --git a/client/ui/frontend/src/components/LanguagePicker.tsx b/client/ui/frontend/src/components/LanguagePicker.tsx index 7a30f8b33..35ef7d5b5 100644 --- a/client/ui/frontend/src/components/LanguagePicker.tsx +++ b/client/ui/frontend/src/components/LanguagePicker.tsx @@ -97,9 +97,9 @@ export function LanguagePicker() { "rounded-md border bg-white dark:bg-nb-gray-900", "border-neutral-200 dark:border-nb-gray-700", "cursor-default text-xs font-semibold text-nb-gray-100 outline-none", - "hover:border-nb-gray-600 data-[state=open]:border-nb-gray-600", + "hover:border-nb-gray-700 data-[state=open]:border-nb-gray-700 dark:hover:border-nb-gray-600 dark:data-[state=open]:border-nb-gray-600", isFocusVisible && - "focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", + "focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", "disabled:opacity-50", )} > @@ -157,7 +157,7 @@ export function LanguagePicker() { placeholder={t("settings.general.language.search")} aria-label={t("settings.general.language.search")} className={cn( - "w-full bg-transparent text-xs text-nb-gray-100 placeholder:text-nb-gray-300", + "w-full bg-transparent text-xs text-nb-gray-100 placeholder:text-nb-gray-600 dark:placeholder:text-nb-gray-300", "border-none outline-none", )} /> diff --git a/client/ui/frontend/src/components/SquareIcon.tsx b/client/ui/frontend/src/components/SquareIcon.tsx index e904d2de5..aaf3b1100 100644 --- a/client/ui/frontend/src/components/SquareIcon.tsx +++ b/client/ui/frontend/src/components/SquareIcon.tsx @@ -5,7 +5,7 @@ import { cn } from "@/lib/cn"; export type SquareIconVariant = "default" | "info" | "warning" | "danger"; const variantClass: Record = { - default: "text-white", + default: "text-nb-gray-50", info: "text-sky-400", warning: "text-netbird", danger: "text-red-500", @@ -27,7 +27,7 @@ export const SquareIcon = ({
o.value === theme) ?? OPTIONS[0]; + const CurrentIcon = current.icon; + + const select = async (value: string) => { + if (busy || value === theme) return; + setBusy(true); + try { + await setTheme(value as ThemePreference); + } catch (e) { + await errorDialog({ + Title: t("settings.error.saveTitle"), + Message: formatErrorMessage(e), + }); + } finally { + setBusy(false); + } + }; + + return ( +
+
+ + {t("settings.general.theme.help")} +
+
+ + + + + + void select(v)}> + {OPTIONS.map(({ value, icon: Icon, labelKey }) => ( + + + {t(labelKey)} + + ))} + + + +
+
+ ); +} diff --git a/client/ui/frontend/src/components/Tooltip.tsx b/client/ui/frontend/src/components/Tooltip.tsx index 2c77ba139..d7a85277a 100644 --- a/client/ui/frontend/src/components/Tooltip.tsx +++ b/client/ui/frontend/src/components/Tooltip.tsx @@ -81,12 +81,12 @@ export const Tooltip = ({ onPointerLeave={interactive ? scheduleClose : undefined} onPointerDownOutside={interactive ? undefined : (e) => e.preventDefault()} className={cn( - "z-50 select-none text-xs text-nb-gray-100 shadow-lg", + "z-50 select-none text-xs text-nb-gray-100 shadow-sm dark:shadow-lg", "data-[state=delayed-open]:animate-in data-[state=closed]:animate-out", "data-[state=closed]:fade-out-0 data-[state=delayed-open]:fade-in-0", !interactive && "pointer-events-none", contentClassName ?? - "rounded-md border border-nb-gray-850 bg-nb-gray-900 px-2 py-1", + "rounded-md border border-nb-gray-800 bg-white px-2 py-1 dark:border-nb-gray-850 dark:bg-nb-gray-900", )} > {content} diff --git a/client/ui/frontend/src/components/VerticalTabs.tsx b/client/ui/frontend/src/components/VerticalTabs.tsx index 1aedf82a6..306850ee2 100644 --- a/client/ui/frontend/src/components/VerticalTabs.tsx +++ b/client/ui/frontend/src/components/VerticalTabs.tsx @@ -46,12 +46,12 @@ const Trigger = forwardRef(function VerticalTab (function VerticalTab aria-hidden={"true"} className={cn( "ml-2 shrink-0 transition-colors duration-150", - "text-nb-gray-400 group-data-[state=active]:text-nb-gray-100", + "text-nb-gray-350 dark:text-nb-gray-400", + "group-data-[state=active]:text-nb-gray-100", )} /> {title} diff --git a/client/ui/frontend/src/components/buttons/Button.tsx b/client/ui/frontend/src/components/buttons/Button.tsx index 6b151c17b..931988ead 100644 --- a/client/ui/frontend/src/components/buttons/Button.tsx +++ b/client/ui/frontend/src/components/buttons/Button.tsx @@ -24,71 +24,74 @@ const buttonVariants = cva( variants: { variant: { default: [ - "border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", - "dark:border-gray-700/30 dark:bg-nb-gray dark:text-gray-400 dark:hover:bg-zinc-800/50 dark:hover:text-white dark:focus:ring-zinc-800/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", + "dark:border-gray-700/30 dark:bg-nb-gray dark:text-gray-400 dark:hover:bg-zinc-800/50 dark:hover:text-nb-gray-50 dark:focus:ring-zinc-800/50", ], primary: [ - "dark:text-gray-100 dark:ring-offset-neutral-950/50 dark:focus:ring-netbird-600/50 enabled:dark:bg-netbird enabled:dark:hover:bg-netbird-500/80 enabled:dark:hover:text-white disabled:dark:bg-nb-gray-900", - "enabled:bg-netbird enabled:text-white enabled:hover:bg-netbird-500 enabled:focus:ring-netbird-400/50", + "dark:text-gray-100 dark:ring-offset-neutral-950/50 dark:focus:ring-netbird-600/50 enabled:dark:bg-netbird enabled:dark:hover:bg-netbird-500/80 enabled:dark:hover:text-nb-gray-50 disabled:dark:bg-nb-gray-900", + "enabled:bg-netbird enabled:text-white enabled:hover:bg-netbird-500 enabled:focus:ring-netbird-400/50 disabled:bg-nb-gray-700", ], secondary: [ - "border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", - "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20", - "dark:border-gray-700/40 dark:bg-nb-gray-920 dark:text-gray-400 dark:hover:bg-nb-gray-910 dark:hover:text-white", + "border-neutral-200 bg-white text-neutral-900 hover:border-nb-gray-700 hover:bg-nb-gray-950 hover:text-black focus:ring-nb-gray-500/50 focus:ring-offset-0", + "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20 dark:focus:ring-offset-1", + "dark:border-gray-700/40 dark:bg-nb-gray-920 dark:text-gray-400 dark:hover:border-gray-700/40 dark:hover:bg-nb-gray-910 dark:hover:text-nb-gray-50", ], secondaryLighter: [ - "border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20", - "dark:border-gray-700/70 dark:bg-nb-gray-900/70 dark:text-gray-400 dark:hover:bg-nb-gray-800/60 dark:hover:text-white", + "dark:border-gray-700/70 dark:bg-nb-gray-900/70 dark:text-gray-400 dark:hover:bg-nb-gray-800/60 dark:hover:text-nb-gray-50", ], subtle: [ - "border-nb-gray-200 bg-nb-gray-50 text-nb-gray-900 hover:bg-nb-gray-100 focus:ring-nb-gray-200/60", + "border-neutral-200 bg-neutral-50 text-neutral-900 hover:bg-neutral-100 focus:ring-neutral-200/60", "dark:ring-offset-neutral-950/50 dark:focus:ring-nb-gray-200/40", "dark:border-nb-gray-200 dark:bg-nb-gray-50 dark:text-nb-gray-900 dark:hover:bg-nb-gray-100 dark:hover:text-nb-gray-950", ], input: [ - "border-neutral-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20", "dark:border-nb-gray-700 dark:bg-nb-gray-900 dark:text-gray-400 dark:hover:bg-nb-gray-900/80", ], dropdown: [ - "border-neutral-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20", "dark:border-nb-gray-900 dark:bg-nb-gray-900/40 dark:text-gray-400 dark:hover:bg-nb-gray-900/50", ], dotted: [ - "border-dashed border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-dashed border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:ring-offset-neutral-950/50 dark:focus:ring-neutral-500/20", - "dark:border-gray-500/40 dark:bg-nb-gray-900/30 dark:text-gray-400 dark:hover:bg-nb-gray-900/50 dark:hover:text-white", + "dark:border-gray-500/40 dark:bg-nb-gray-900/30 dark:text-gray-400 dark:hover:bg-nb-gray-900/50 dark:hover:text-nb-gray-50", ], tertiary: [ - "border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:border-gray-700/40 dark:bg-white dark:text-gray-800 dark:hover:bg-neutral-200 dark:focus:ring-zinc-800/50 disabled:dark:bg-nb-gray-920 disabled:dark:text-nb-gray-300", ], white: [ - "border-white bg-white text-gray-800 outline-none hover:bg-neutral-200 focus:ring-white/50 disabled:dark:bg-nb-gray-920 disabled:dark:text-nb-gray-300", + "border-white bg-white text-neutral-800 outline-none hover:bg-neutral-200 focus:ring-white/50 dark:text-gray-800 disabled:dark:bg-nb-gray-920 disabled:dark:text-nb-gray-300", "disabled:dark:border-nb-gray-900 disabled:dark:bg-nb-gray-900 disabled:dark:text-nb-gray-300", ], outline: [ - "border-gray-200 bg-white text-gray-900 hover:bg-gray-100 hover:text-black focus:ring-zinc-200/50", + "border-neutral-200 bg-white text-neutral-900 hover:bg-neutral-100 hover:text-black focus:ring-neutral-200/50", "dark:border-netbird dark:bg-transparent dark:text-netbird dark:hover:bg-nb-gray-900/30 dark:focus:ring-zinc-800/50", ], "danger-outline": [ + "bg-transparent text-red-600 enabled:hover:bg-red-50 enabled:focus:ring-red-200/50", "dark:bg-transparent dark:text-red-500 enabled:dark:hover:border-red-800/50 enabled:hover:dark:bg-red-950/50 enabled:dark:focus:bg-red-950/40 enabled:dark:focus:ring-red-800/20", ], "danger-text": [ - "rounded-sm !px-0 !py-0 !shadow-none focus:ring-red-500/30 dark:border-transparent dark:bg-transparent dark:text-red-500 dark:ring-offset-neutral-950/50 dark:hover:text-red-600", + "rounded-sm border-transparent bg-transparent !px-0 !py-0 text-red-600 !shadow-none hover:text-red-700 focus:ring-red-500/30", + "dark:border-transparent dark:bg-transparent dark:text-red-500 dark:ring-offset-neutral-950/50 dark:hover:text-red-600", ], "default-outline": [ - "dark:ring-offset-nb-gray-950/50 dark:focus:ring-nb-gray-500/20", - "dark:border-transparent dark:bg-transparent dark:text-nb-gray-400 dark:hover:border-nb-gray-800/50 dark:hover:bg-nb-gray-900/30 dark:hover:text-white", - "data-[state=open]:dark:border-nb-gray-800/50 data-[state=open]:dark:bg-nb-gray-900/30 data-[state=open]:dark:text-white", + "ring-offset-nb-gray-950/50 focus:ring-nb-gray-500/20", + "border-transparent bg-transparent text-nb-gray-400 hover:border-nb-gray-800/50 hover:bg-nb-gray-900/30 hover:text-nb-gray-50", + "data-[state=open]:border-nb-gray-800/50 data-[state=open]:bg-nb-gray-900/30 data-[state=open]:text-nb-gray-50", ], ghost: [ - "dark:ring-offset-nb-gray-950/50 dark:focus:ring-nb-gray-500/20", - "dark:border-transparent dark:bg-transparent dark:text-nb-gray-400 dark:hover:bg-nb-gray-900/30 dark:hover:text-white", + "ring-offset-nb-gray-950/50 focus:ring-nb-gray-500/20", + "border-transparent bg-transparent text-nb-gray-400 hover:bg-nb-gray-900/30 hover:text-nb-gray-50", ], danger: [ + "bg-red-600 text-red-50 hover:bg-red-700 focus:bg-red-700 focus:ring-red-700/20", "dark:bg-red-600 dark:text-red-100 dark:hover:border-red-800/50 hover:dark:bg-red-700 dark:focus:bg-red-700 dark:focus:ring-red-700/20", ], }, diff --git a/client/ui/frontend/src/components/buttons/IconButton.tsx b/client/ui/frontend/src/components/buttons/IconButton.tsx index 3d36bc111..8d688cb0b 100644 --- a/client/ui/frontend/src/components/buttons/IconButton.tsx +++ b/client/ui/frontend/src/components/buttons/IconButton.tsx @@ -24,7 +24,7 @@ export const IconButton = forwardRef(function IconButt "flex h-10 w-10 cursor-default items-center justify-center rounded-lg outline-none", "text-nb-gray-400 hover:bg-nb-gray-900 hover:text-nb-gray-300", isFocusVisible && - "focus-visible:ring-2 focus-visible:ring-white/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", + "focus-visible:ring-2 focus-visible:ring-nb-gray-50/60 focus-visible:ring-offset-2 focus-visible:ring-offset-nb-gray-940", "wails-no-draggable transition-colors duration-150", className, )} diff --git a/client/ui/frontend/src/components/dialog/Dialog.tsx b/client/ui/frontend/src/components/dialog/Dialog.tsx index fa8007d9f..c43c04b0b 100644 --- a/client/ui/frontend/src/components/dialog/Dialog.tsx +++ b/client/ui/frontend/src/components/dialog/Dialog.tsx @@ -23,7 +23,7 @@ const Overlay = forwardRef, OverlayPr ref={ref} className={cn( "fixed inset-0 z-50 grid items-center justify-items-center overflow-y-auto px-10 py-16", - "bg-black/60", + "bg-black/25 dark:bg-black/60", "data-[state=open]:animate-in data-[state=open]:fade-in-0", exitAnimation && "data-[state=closed]:animate-out data-[state=closed]:fade-out-0", @@ -67,7 +67,7 @@ export const Content = forwardRef, Co className={cn( "relative z-[52] mx-auto w-full outline-none ring-0", "focus:outline-none focus:ring-0 focus-visible:outline-none focus-visible:ring-0", - "rounded-lg border border-nb-gray-900 bg-nb-gray py-7 shadow-2xl", + "rounded-lg border border-nb-gray-800 bg-nb-gray-940 py-7 shadow-2xl dark:border-nb-gray-900 dark:bg-nb-gray", "data-[state=open]:animate-in data-[state=open]:fade-in-0", "data-[state=open]:zoom-in-95 data-[state=open]:slide-in-from-left-1", exitAnimation && diff --git a/client/ui/frontend/src/components/inputs/Input.tsx b/client/ui/frontend/src/components/inputs/Input.tsx index 2dad80d7a..eada79a1f 100644 --- a/client/ui/frontend/src/components/inputs/Input.tsx +++ b/client/ui/frontend/src/components/inputs/Input.tsx @@ -32,19 +32,19 @@ const inputVariants = cva("", { variants: { variant: { default: [ - "border-neutral-200 placeholder:text-neutral-500 dark:border-nb-gray-700 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", + "border-neutral-200 placeholder:text-nb-gray-600 dark:border-nb-gray-700 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", "ring-offset-neutral-200/20 focus-visible:ring-neutral-300/10 dark:ring-offset-neutral-950/50 dark:focus-visible:ring-neutral-500/20", ], darker: [ - "border-neutral-300 placeholder:text-neutral-500 dark:border-nb-gray-800 dark:bg-nb-gray-920 dark:placeholder:text-neutral-400/70", + "border-neutral-300 placeholder:text-nb-gray-600 dark:border-nb-gray-800 dark:bg-nb-gray-920 dark:placeholder:text-neutral-400/70", "ring-offset-neutral-200/20 focus-visible:ring-neutral-300/10 dark:ring-offset-neutral-950/50 dark:focus-visible:ring-neutral-500/20", ], error: [ - "border-neutral-200 text-red-500 placeholder:text-neutral-500 dark:border-red-500 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", + "border-neutral-200 text-red-500 placeholder:text-nb-gray-600 dark:border-red-500 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", "ring-offset-red-500/10 focus-visible:ring-red-500/10 dark:ring-offset-red-500/10 dark:focus-visible:ring-red-500/10", ], warning: [ - "border-neutral-200 text-orange-400 placeholder:text-neutral-500 dark:border-orange-400 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", + "border-neutral-200 text-orange-400 placeholder:text-nb-gray-600 dark:border-orange-400 dark:bg-nb-gray-900 dark:placeholder:text-neutral-400/70", "ring-offset-orange-400/10 focus-visible:ring-orange-400/10 dark:ring-offset-orange-400/10 dark:focus-visible:ring-orange-400/10", ], }, @@ -158,7 +158,7 @@ function NumberStepper({ className={cn( "flex h-[40px] shrink-0 flex-col overflow-hidden", "rounded-r-md border border-l-0", - "border-neutral-200 dark:border-nb-gray-700 dark:bg-nb-gray-900", + "border-neutral-200 bg-white dark:border-nb-gray-700 dark:bg-nb-gray-900", error && "dark:border-red-500", disabled && "pointer-events-none opacity-40", )} @@ -274,7 +274,9 @@ export const Input = forwardRef(function Input(