Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,19 @@ jobs:
- name: Staging disk-monitor unit tests (issue #1684)
run: bash scripts/staging/test-disk-monitor.sh

- name: Non-root container helper unit tests + ShellCheck (issue #339)
run: |
bash scripts/test-nonroot-helper.sh
shellcheck docker/nonroot.sh scripts/test-nonroot-helper.sh scripts/test-docker-nonroot.sh scripts/test-nonroot-entrypoint.sh
# SC1091: the entrypoint sources the data volume's .env at run time.
shellcheck -e SC1091 docker/entrypoint-go.sh

# The real entrypoint and helper as root in the runtime image's pinned
# Alpine base, supervisord stubbed: off-mode start, ownership changes,
# the check before root uses files in the data dir, rollback records.
- name: Non-root container start path, supervisord stubbed (issue #339)
run: bash scripts/test-nonroot-entrypoint.sh

- name: QA blacklist script unit tests (issues #1977, #83)
run: bash qa/scripts/test-blacklist-sql.sh

Expand Down
5 changes: 3 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ COPY docker/Caddyfile /etc/caddy/Caddyfile
RUN mkdir -p /app/data /var/lib/mosquitto /data/caddy && \
chown -R mosquitto:mosquitto /var/lib/mosquitto

# Entrypoint
# Entrypoint (+ opt-in non-root helper, #339; unused unless enabled)
COPY docker/entrypoint-go.sh /entrypoint.sh
RUN chmod +x /entrypoint.sh
COPY docker/nonroot.sh /usr/local/lib/corescope/nonroot.sh
RUN chmod +x /entrypoint.sh /usr/local/lib/corescope/nonroot.sh

EXPOSE 80 443 1883

Expand Down
5 changes: 3 additions & 2 deletions Dockerfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ COPY docker/Caddyfile /etc/caddy/Caddyfile
RUN mkdir -p /app/data /var/lib/mosquitto /data/caddy && \
chown -R mosquitto:mosquitto /var/lib/mosquitto

# Entrypoint
# Entrypoint (+ opt-in non-root helper, #339; unused unless enabled)
COPY docker/entrypoint-go.sh /entrypoint.sh
RUN chmod +x /entrypoint.sh
COPY docker/nonroot.sh /usr/local/lib/corescope/nonroot.sh
RUN chmod +x /entrypoint.sh /usr/local/lib/corescope/nonroot.sh

EXPOSE 80 443 1883

Expand Down
28 changes: 28 additions & 0 deletions cmd/ingestor/admin_delete_client_rx.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,35 @@ func runAdminDeleteClientRxDryRun(dbPath, key string) int {
return 0
}

// clientRxGeteuid is os.Geteuid; a package var so tests can model a root CLI.
var clientRxGeteuid = os.Geteuid

// clientRxCheckRequester refuses a root CLI next to a database that another
// user owns (#339): the ingestor then runs as that user and cannot read the
// root-owned 0600 request. It would sit in the queue while the CLI reports
// "ingestor not running?", and run unannounced once its owner changed.
func clientRxCheckRequester(dbPath string) error {
if clientRxGeteuid() != 0 {
return nil
}
fi, err := os.Stat(dbPath)
if err != nil {
return nil
}
uid, ok := fileOwnerUID(fi)
if !ok || uid == 0 {
return nil
}
return fmt.Errorf("refusing to run as root: %s belongs to UID %d, so the ingestor runs as that user "+
"and could not read a request written by root; run this command as that user "+
"(e.g. docker exec -u CORESCOPE_UID:CORESCOPE_GID …)", dbPath, uid)
}

func runAdminDeleteClientRxOnline(dbPath, key string, wait time.Duration) int {
if err := clientRxCheckRequester(dbPath); err != nil {
fmt.Fprintf(os.Stderr, "delete-client-rx: %v\n", err)
return 1
}
id := clientRxNewID()
if err := clientRxWriteRequest(dbPath, clientRxDeleteRequest{
ID: id,
Expand Down
82 changes: 82 additions & 0 deletions cmd/ingestor/admin_delete_client_rx_owner_339_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package main

import (
"bytes"
"io"
"os"
"path/filepath"
"strings"
"testing"
)

// #339 review, finding 3: in the container's non-root mode the ingestor runs
// as CORESCOPE_UID and owns the database. A delete requested by a root
// `docker exec` used to leave a root-owned 0600 request that the ingestor
// could not read; the CLI reported "queued — ingestor not running?" and the
// delete then ran unannounced after a later restart. Run as root against a
// database owned by another user, the CLI must refuse and write nothing.

func captureStderr339(t *testing.T, fn func()) string {
t.Helper()
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
orig := os.Stderr
os.Stderr = w
defer func() { os.Stderr = orig }()
fn()
w.Close()
var buf bytes.Buffer
_, _ = io.Copy(&buf, r)
return buf.String()
}

func TestDeleteClientRxAsRootRefusesForeignOwnedDB339(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("needs a database owned by a non-root user")
}
dbPath := filepath.Join(t.TempDir(), "meshcore.db")
if err := os.WriteFile(dbPath, nil, 0o644); err != nil {
t.Fatal(err)
}
orig := clientRxGeteuid
t.Cleanup(func() { clientRxGeteuid = orig })
clientRxGeteuid = func() int { return 0 }

var code int
stderr := captureStderr339(t, func() {
code = runAdminDeleteClientRxOnline(dbPath, strings.Repeat("ab", 32), 0)
})
if code != 1 {
t.Errorf("exit = %d, want 1", code)
}
for _, want := range []string{"refusing to run as root", "-u "} {
if !strings.Contains(stderr, want) {
t.Errorf("stderr %q lacks %q", stderr, want)
}
}
if _, err := os.Stat(clientRxQueueDir(dbPath)); !os.IsNotExist(err) {
t.Errorf("queue directory exists after the refusal (err=%v)", err)
}
}

// The same user as the database owner (the ingestor's own user, and root in
// the default container mode where root owns the database) still queues.
func TestDeleteClientRxAsDBOwnerStillQueues339(t *testing.T) {
dbPath := filepath.Join(t.TempDir(), "meshcore.db")
if err := os.WriteFile(dbPath, nil, 0o644); err != nil {
t.Fatal(err)
}
var code int
stderr := captureStderr339(t, func() {
code = runAdminDeleteClientRxOnline(dbPath, strings.Repeat("ab", 32), 0)
})
if code != 2 || !strings.Contains(stderr, "queued") {
t.Fatalf("exit = %d (%q), want 2 queued", code, stderr)
}
pending, err := clientRxListPending(dbPath)
if err != nil || len(pending) != 1 {
t.Fatalf("pending = %v, %v; want one request", pending, err)
}
}
39 changes: 37 additions & 2 deletions cmd/server/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@ package main

import (
"encoding/json"
"errors"
"fmt"
"io"
"io/fs"
"log"
"math"
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"syscall"
"time"

"github.com/meshcore-analyzer/channelregistry"
Expand Down Expand Up @@ -1253,6 +1256,35 @@ var (
configStatFile = func(f *os.File) (os.FileInfo, error) { return f.Stat() }
)

// ErrConfigReadOnly marks a SaveGeoFilter failure caused by the deployment
// arrangement rather than by the data: config.json or its directory cannot be
// written by this process — a read-only mount, or the server running as a
// non-root user without write access (#339). Callers report it as such
// instead of as a generic save failure.
var ErrConfigReadOnly = errors.New("config.json is read-only for the server")

// configWritesDisabled reports whether the deployment forbids the server to
// write config.json at all: the container's opt-in non-root mode (#339),
// which the entrypoint passes on to the services. There config.json belongs
// to the host or root, and the refusal must not depend on which directory a
// save would happen to resolve to (#339 review, finding 2; #422).
func configWritesDisabled() bool {
switch os.Getenv("CORESCOPE_RUN_AS_NONROOT") {
case "1", "true":
return true
}
return false
}

// configReadOnlyErr wraps a create/rename failure in ErrConfigReadOnly when
// its cause is a permission or read-only-filesystem error.
func configReadOnlyErr(op string, err error) error {
if errors.Is(err, fs.ErrPermission) || errors.Is(err, syscall.EROFS) {
return fmt.Errorf("%w: %s: %w", ErrConfigReadOnly, op, err)
}
return fmt.Errorf("%s: %w", op, err)
}

// SaveGeoFilter writes the geo_filter section back to config.json on disk.
// Pass gf=nil to remove the filter. The rest of config.json is preserved as-is.
//
Expand All @@ -1275,6 +1307,9 @@ var (
// argument is ignored for a file that already exists, and it is masked by
// the umask for one that does not.
func SaveGeoFilter(configDir string, gf *GeoFilterConfig) error {
if configWritesDisabled() {
return fmt.Errorf("%w: CORESCOPE_RUN_AS_NONROOT is on", ErrConfigReadOnly)
}
var configPath string
for _, p := range []string{
filepath.Join(configDir, "config.json"),
Expand Down Expand Up @@ -1342,7 +1377,7 @@ func SaveGeoFilter(configDir string, gf *GeoFilterConfig) error {
// Atomic write: unique temp file in the destination directory + rename.
tmpFile, err := configCreateTemp(filepath.Dir(configPath), filepath.Base(configPath)+".*.tmp")
if err != nil {
return fmt.Errorf("create temp config: %w", err)
return configReadOnlyErr("create temp config", err)
}
tmpName := tmpFile.Name()
renamed := false
Expand Down Expand Up @@ -1370,7 +1405,7 @@ func SaveGeoFilter(configDir string, gf *GeoFilterConfig) error {
return fmt.Errorf("close temp config: %w", err)
}
if err := configRename(tmpName, configPath); err != nil {
return fmt.Errorf("rename config: %w", err)
return configReadOnlyErr("rename config", err)
}
renamed = true
return nil
Expand Down
Loading
Loading