diff --git a/internal/broker/broker.go b/internal/broker/broker.go index 84025ef..51e5c87 100644 --- a/internal/broker/broker.go +++ b/internal/broker/broker.go @@ -16,6 +16,7 @@ import ( "encoding/pem" "errors" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "os" "strings" ) @@ -40,7 +41,10 @@ var ErrNotConfigured = errors.New("this control plane has not been told about it // FromEnvironment reads the two settings, if they are there. func FromEnvironment() (Broker, error) { - address := strings.TrimSpace(os.Getenv(AddressVar)) + address, err := envfile.Value(AddressVar) + if err != nil { + return Broker{}, err + } path := strings.TrimSpace(os.Getenv(CertificateVar)) if address == "" && path == "" { diff --git a/internal/broker/management.go b/internal/broker/management.go index 684d0a1..cea2a44 100644 --- a/internal/broker/management.go +++ b/internal/broker/management.go @@ -5,10 +5,10 @@ import ( "context" "encoding/json" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "io" "net/http" "net/url" - "os" "regexp" "strings" "time" @@ -42,7 +42,10 @@ type Management struct { // ManagementFromEnvironment reads where the management API is, if it is configured. func ManagementFromEnvironment() (*Management, error) { - raw := strings.TrimSpace(os.Getenv(ManagementVar)) + raw, err := envfile.Value(ManagementVar) + if err != nil { + return nil, err + } if raw == "" { return nil, ErrNotConfigured } diff --git a/internal/catalogue/declaration.go b/internal/catalogue/declaration.go index c6cf0f3..616908c 100644 --- a/internal/catalogue/declaration.go +++ b/internal/catalogue/declaration.go @@ -442,6 +442,10 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { // And the machine underneath, which no binding of its own can tell it. thisMachine := machineFacts(r) + // Which of this module's files carry a secret, for the rule that a container may not read + // one of them as its environment without saying so (ADR 0086, issue 041). + secretFiles := secretFilesOf(resources) + for _, unsettled := range resources { resource, err := ApplySettings(unsettled, with.Settings[m.Module]) if err != nil { @@ -451,6 +455,12 @@ func (r Resolution) Declaration(with Rendering) ([]map[string]any, error) { for k, v := range resource { copied[k] = v } + if err := refuseSecretsInEnvironment(copied, secretFiles, m.Module); err != nil { + return nil, err + } + // Said in the catalogue, not on the machine: the host parses strictly and knows no + // such field, and the reason is for a reader of the manifest. + delete(copied, SecretsInEnvironment) // **After settings, and that is the whole reason it is here.** A module's file // content is where a setting lands, so a placeholder may only exist once the setting // has been put in — filling secrets first would look at content that is not yet what diff --git a/internal/catalogue/secrets_in_environment_test.go b/internal/catalogue/secrets_in_environment_test.go new file mode 100644 index 0000000..9bbde92 --- /dev/null +++ b/internal/catalogue/secrets_in_environment_test.go @@ -0,0 +1,72 @@ +package catalogue + +import ( + "strings" + "testing" +) + +func aModuleWithAnEnvFileSecret(exception string) Manifest { + container := map[string]any{ + "id": "server", "type": "container", "name": "app", "image": "app@sha256:" + strings.Repeat("a", 64), + "env-file": []any{"/var/lib/app/server.env"}, + } + if exception != "" { + container[SecretsInEnvironment] = exception + } + return Manifest{ + Module: "app", Version: "1", + OwnSecrets: map[string]string{"token": "/var/lib/app/token.secret"}, + Resources: []map[string]any{ + {"id": "env", "type": "file", "path": "/var/lib/app/server.env", "mode": "0600", + "content": "APP_TOKEN=${secret:token}\n"}, + container, + }, + } +} + +func declare(t *testing.T, m Manifest) ([]map[string]any, error) { + t.Helper() + got, err := Resolve(shelf(m), []string{m.Module}, + Node{Name: "anchor", At: "10.0.0.1", Capabilities: map[string]bool{}}, World{}) + if err != nil { + t.Fatal(err) + } + return got.Declaration(Rendering{Needed: map[string]map[string]string{"app": {"token": "SEALED"}}}) +} + +// A container that reads a file carrying a secret as its environment is refused, unless it says +// why — and then the reason stays in the catalogue and never reaches the machine. +func TestASecretInAnEnvFileIsRefusedUnlessDeclared(t *testing.T) { + _, err := declare(t, aModuleWithAnEnvFileSecret("")) + if err == nil || !strings.Contains(err.Error(), "04-ISSUES/041") { + t.Fatalf("an env-file carrying a secret was declared without a word: %v", err) + } + + out, err := declare(t, aModuleWithAnEnvFileSecret("the image reads its configuration from the environment only")) + if err != nil { + t.Fatal(err) + } + for _, r := range out { + if _, leaked := r[SecretsInEnvironment]; leaked { + t.Fatalf("the catalogue's reason was sent to the machine: %v", r) + } + } +} + +// A secret placeholder inside a container's env is refused outright: nothing fills it there. +func TestASecretInEnvIsAlwaysRefused(t *testing.T) { + m := aModuleWithAnEnvFileSecret("said") + m.Resources[1]["env"] = map[string]any{"APP_TOKEN": "${secret:token}"} + if _, err := declare(t, m); err == nil || !strings.Contains(err.Error(), "never filled into an environment variable") { + t.Fatalf("a secret in env was accepted: %v", err) + } +} + +// A file with no secret in it is an ordinary env-file and needs no word. +func TestAnEnvFileWithoutASecretNeedsNothing(t *testing.T) { + m := aModuleWithAnEnvFileSecret("") + m.Resources[0]["content"] = "APP_MODE=production\n" + if _, err := declare(t, m); err != nil { + t.Fatal(err) + } +} diff --git a/internal/catalogue/secrets_into_files.go b/internal/catalogue/secrets_into_files.go index 07ae2c0..f47656c 100644 --- a/internal/catalogue/secrets_into_files.go +++ b/internal/catalogue/secrets_into_files.go @@ -4,6 +4,7 @@ import ( "fmt" "regexp" "sort" + "strings" ) // A credential and a configuration file meeting. @@ -83,6 +84,70 @@ func sealedFor(m Manifest, needs []Needed, with Rendering) (map[string]string, e return sealed, nil } +// SecretsInEnvironment is the key a container carries to say, out loud, that a secret reaches +// its process through the environment — and why. +// +// **A secret reaches a process as a file** (novox/hq ADR 0086). The mesh seals a value to the +// machine and the host writes it at 0600; a container that then reads it from an env-file hands +// it to the runtime, which prints it in `docker inspect` and keeps it in the process's /proc entry +// for anything on the machine that can talk to the runtime (novox/hq 04-ISSUES/041). Some software +// reads its configuration from the environment and nothing else, and for that this key exists: a +// reason, on the container, so a reader can tell from the manifest which secrets are exposed that +// way and which are not. Without it, a container whose env-file carries a secret is refused. +// +// Catalogue-level: the host never sees this key. +const SecretsInEnvironment = "secrets-in-environment" + +// refuseSecretsInEnvironment is the check. `secretFiles` is every file of this module whose +// content names a secret. +func refuseSecretsInEnvironment(resource map[string]any, secretFiles map[string]bool, module string) error { + if fmt.Sprint(resource["type"]) != "container" { + return nil + } + name := fmt.Sprint(resource["name"]) + if env, ok := resource["env"].(map[string]any); ok { + for key, value := range env { + if strings.Contains(fmt.Sprint(value), "${secret:") { + return fmt.Errorf( + "%s's container %s puts ${secret:…} in its env (%s). A secret is never filled into an "+ + "environment variable: put it in a file the module declares and mount that, or name the "+ + "file in env-file and say %q why", module, name, key, SecretsInEnvironment) + } + } + } + reason, _ := resource[SecretsInEnvironment].(string) + if strings.TrimSpace(reason) != "" { + return nil + } + if files, ok := resource["env-file"].([]any); ok { + for _, f := range files { + if secretFiles[fmt.Sprint(f)] { + return fmt.Errorf( + "%s's container %s reads %s as an env-file, and that file carries a secret, so the secret "+ + "reaches the process environment — readable in `docker inspect` and /proc (novox/hq "+ + "04-ISSUES/041). Mount the secret's file and point the program at it, or, if the program "+ + "reads only its environment, say so on the container: %q: \"\"", + module, name, f, SecretsInEnvironment) + } + } + } + return nil +} + +// secretFilesOf is the paths of a module's file resources whose content names a secret. +func secretFilesOf(resources []map[string]any) map[string]bool { + out := map[string]bool{} + for _, r := range resources { + if fmt.Sprint(r["type"]) != "file" { + continue + } + if content, ok := r["content"].(string); ok && len(secretsUsed(content)) > 0 { + out[fmt.Sprint(r["path"])] = true + } + } + return out +} + // intoFile gives a file the sealed values its content asks for. // // A name the module never declared is refused here rather than on the machine. The host would diff --git a/internal/envfile/envfile.go b/internal/envfile/envfile.go new file mode 100644 index 0000000..d408b54 --- /dev/null +++ b/internal/envfile/envfile.go @@ -0,0 +1,39 @@ +// Package envfile reads a setting that may be a secret from the environment or, preferably, from +// a file the environment names. +// +// **A secret reaches a process as a file** (novox/hq ADR 0086). An environment variable is +// readable in `docker inspect`, in the process's /proc entry and in whatever composed it; a file +// the mesh sealed to the machine and the host wrote at 0600 is readable where it is used and +// nowhere else. So every variable of the control plane's that carries a credential has a `_FILE` +// twin naming such a file, and the plain form remains only for a control plane a person starts by +// hand and for the bundle that raises the first one. The store's connections had this shape +// already (internal/store); this is the same rule for the rest. +package envfile + +import ( + "fmt" + "os" + "strings" +) + +// Value is the setting named by `name`: the content of the file `name_FILE` points at when that +// is set, else the variable itself. Both set is refused — two sources that could disagree is how +// a setting silently stops meaning what it says. Neither set is "", nil. +func Value(name string) (string, error) { + plain, hasPlain := os.LookupEnv(name) + path, hasFile := os.LookupEnv(name + "_FILE") + switch { + case hasFile && hasPlain && strings.TrimSpace(plain) != "" && strings.TrimSpace(path) != "": + return "", fmt.Errorf("both %s and %s_FILE are set; one of them, not both", name, name) + case hasFile && strings.TrimSpace(path) != "": + raw, err := os.ReadFile(strings.TrimSpace(path)) + if err != nil { + return "", fmt.Errorf("%s_FILE names %s, which cannot be read: %w", name, path, err) + } + // A file has a line ending and a value does not — trimmed, and only the ending, because a + // value may begin or end with a space and still be the value. + return strings.TrimRight(string(raw), "\r\n"), nil + default: + return strings.TrimSpace(plain), nil + } +} diff --git a/internal/envfile/envfile_test.go b/internal/envfile/envfile_test.go new file mode 100644 index 0000000..f8c04e4 --- /dev/null +++ b/internal/envfile/envfile_test.go @@ -0,0 +1,44 @@ +package envfile + +import ( + "os" + "path/filepath" + "testing" +) + +func TestAFileTwinIsPreferredAndOnlyItsLineEndingGoes(t *testing.T) { + path := filepath.Join(t.TempDir(), "value") + if err := os.WriteFile(path, []byte(" amqp://u:p@h/ \n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("MESH_X", "") + t.Setenv("MESH_X_FILE", path) + got, err := Value("MESH_X") + if err != nil || got != " amqp://u:p@h/ " { + t.Fatalf("got %q, %v", got, err) + } +} + +func TestThePlainVariableStillWorks(t *testing.T) { + t.Setenv("MESH_Y", " plain ") + if got, err := Value("MESH_Y"); err != nil || got != "plain" { + t.Fatalf("got %q, %v", got, err) + } +} + +func TestBothSetIsRefused(t *testing.T) { + path := filepath.Join(t.TempDir(), "value") + _ = os.WriteFile(path, []byte("a"), 0o600) + t.Setenv("MESH_Z", "b") + t.Setenv("MESH_Z_FILE", path) + if _, err := Value("MESH_Z"); err == nil { + t.Fatal("two sources were accepted") + } +} + +func TestAMissingFileIsSaid(t *testing.T) { + t.Setenv("MESH_W_FILE", filepath.Join(t.TempDir(), "absent")) + if _, err := Value("MESH_W"); err == nil { + t.Fatal("a missing file produced a value") + } +} diff --git a/internal/link/serve.go b/internal/link/serve.go index 2d402e7..f0a8ec3 100644 --- a/internal/link/serve.go +++ b/internal/link/serve.go @@ -5,9 +5,9 @@ import ( "encoding/json" "errors" "fmt" + "github.com/novox/mesh-controller/internal/envfile" "log" "os" - "strings" "time" amqp "github.com/rabbitmq/amqp091-go" @@ -107,7 +107,10 @@ func (s *Server) Answers(r Replayer) error { // Connect opens the control plane's own connection to the broker. func Connect(enroller Enroller, listener Listener) (*Server, error) { - url := strings.TrimSpace(os.Getenv(AMQPVar)) + url, err := envfile.Value(AMQPVar) + if err != nil { + return nil, err + } if url == "" { return nil, fmt.Errorf( "this control plane has no %s, so it cannot reach its broker. Nodes talk to it over "+