mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 17:51:29 +00:00
037dab7b6f
Sprint 1 unified-master-audit closure. Pre-fix the agent built its
on-disk key path via:
keyPath := filepath.Join(a.config.KeyDir, job.CertificateID+".key")
migrations/000001_initial_schema.up.sql declares managed_certificates.id
as TEXT PRIMARY KEY with no shape constraint, so a compromised control
plane (or a poisoned database row) could deliver a job whose
certificate_id is '../../etc/passwd', '/absolute/path', a NUL-byte
payload, or a Windows-separator-laden string — driving arbitrary
file write or read on the agent host.
Fix (two ends; both load-bearing):
Server side:
- New internal/validation/certificate_id.go: ValidateCertificateID
pins the canonical TEXT-PK shape (^[A-Za-z0-9._-]{1,128}$, plus
explicit '.'/'..' rejection).
- CertificateService.Create now invokes ValidateCertificateID after
the existing required-fields check; malformed IDs are refused
before persistence or downstream job creation.
Agent side:
- cmd/agent/keymem.go: validateAgentCertID mirrors the server-side
shape regex. safeAgentKeyPath additionally asserts the joined
path is contained within KeyDir via filepath.Rel — even if a
future refactor bypasses the shape check, a path that escapes
KeyDir fails closed.
- poll.go + deploy.go: both filepath.Join call sites routed
through safeAgentKeyPath; rejection surfaces via reportJobStatus
so the control plane sees the failure.
Regression coverage:
- internal/validation/certificate_id_test.go: production shapes
accepted; explicit rejection table for empty, overlong, posix
traversal, absolute, Windows traversal, Windows separator, NUL
byte, newline/tab injection, drive prefix, space, unicode dots.
- cmd/agent/keymem_test.go: validateAgentCertID acceptance +
rejection tables; safeAgentKeyPath happy path + the 8 audit
vectors plus empty-keyDir refusal.
Closes SEC-002.
160 lines
6.1 KiB
Go
160 lines
6.1 KiB
Go
// Copyright 2026 certctl LLC. All rights reserved.
|
|
// SPDX-License-Identifier: BUSL-1.1
|
|
|
|
package main
|
|
|
|
import (
|
|
"crypto/ecdsa"
|
|
"crypto/x509"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strings"
|
|
)
|
|
|
|
// Bundle-9 / Audit L-002 + L-003 (agent edition).
|
|
//
|
|
// The agent generates an ECDSA P-256 key locally and writes it to disk with
|
|
// mode 0600 in a directory it expects to be 0700. The duplication of the
|
|
// local-issuer helpers (instead of importing from internal/...) is deliberate:
|
|
//
|
|
// - cmd/agent is a separate binary with its own threat model (runs on every
|
|
// deployment target, not just the control plane). Coupling it to
|
|
// internal/connector/issuer/local would pull deployment-target footprint
|
|
// into a connector that's only relevant on the server.
|
|
// - The behavior is small and self-contained; copy-paste is cheaper than
|
|
// a refactor that introduces an internal/keystore package.
|
|
//
|
|
// If a third call site emerges, lift these into internal/keystore.
|
|
|
|
// marshalAgentKeyAndZeroize marshals an ECDSA private key to DER and invokes
|
|
// onDER with the bytes; the buffer is zeroized via builtin clear() after
|
|
// onDER returns. Caller must NOT retain the slice.
|
|
func marshalAgentKeyAndZeroize(priv *ecdsa.PrivateKey, onDER func([]byte) error) error {
|
|
if priv == nil {
|
|
return fmt.Errorf("marshalAgentKeyAndZeroize: nil private key")
|
|
}
|
|
der, err := x509.MarshalECPrivateKey(priv)
|
|
if err != nil {
|
|
return fmt.Errorf("marshal EC private key: %w", err)
|
|
}
|
|
defer clear(der)
|
|
return onDER(der)
|
|
}
|
|
|
|
// SEC-002 closure (Sprint 1, 2026-05-16). The agent derives an on-disk
|
|
// key path from job.CertificateID via filepath.Join. Pre-fix, a
|
|
// crafted certificate_id ("../../etc/passwd", "/absolute/path",
|
|
// "abc\x00d", "..\\Windows\\path") would drive arbitrary file
|
|
// write/read on the agent host. The shape regex below mirrors the
|
|
// server-side internal/validation.ValidateCertificateID gate — both
|
|
// ends MUST hold for the load-bearing defense (the server can't be
|
|
// trusted in isolation; a compromised control plane could deliver a
|
|
// crafted job).
|
|
//
|
|
// agentCertIDPattern accepts ASCII letters, digits, ".", "_", "-",
|
|
// bounded to 128 chars. Existing prefixed IDs (mc-..., cert-..., etc.)
|
|
// satisfy this trivially. Deliberately rejects path separators (POSIX
|
|
// and Windows), NUL byte, whitespace, control characters, and the
|
|
// bare relative-path tokens "." and "..".
|
|
var agentCertIDPattern = regexp.MustCompile(`^[A-Za-z0-9._-]{1,128}$`)
|
|
|
|
// validateAgentCertID returns an error if id is not a well-formed
|
|
// certificate identifier. Mirrors internal/validation.ValidateCertificateID
|
|
// — the duplication is deliberate per the package-level comment
|
|
// ("cmd/agent is a separate binary; copy-paste cheaper than lifting
|
|
// a shared internal/keystore for a single shape check").
|
|
func validateAgentCertID(id string) error {
|
|
if id == "" {
|
|
return fmt.Errorf("certificate_id is required")
|
|
}
|
|
if len(id) > 128 {
|
|
return fmt.Errorf("certificate_id length %d exceeds 128", len(id))
|
|
}
|
|
if !agentCertIDPattern.MatchString(id) {
|
|
return fmt.Errorf("certificate_id %q contains disallowed characters", id)
|
|
}
|
|
if id == "." || id == ".." {
|
|
return fmt.Errorf("certificate_id %q is a relative-path token", id)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
// safeAgentKeyPath returns the on-disk key path for the given
|
|
// certificateID, after validating the ID shape AND asserting the
|
|
// joined path is contained within keyDir. Containment is the
|
|
// authoritative guard — even if validateAgentCertID is bypassed (e.g.
|
|
// a future refactor removes it), the post-Clean rel-path check below
|
|
// rejects any path that escapes keyDir.
|
|
//
|
|
// The two-leg defense:
|
|
//
|
|
// leg 1: shape check (validateAgentCertID) → cheap up-front fail
|
|
// leg 2: containment check (filepath.Rel) → load-bearing guard
|
|
//
|
|
// Returns the joined path on success, or a non-nil error describing
|
|
// the rejected vector.
|
|
func safeAgentKeyPath(keyDir, certificateID string) (string, error) {
|
|
if err := validateAgentCertID(certificateID); err != nil {
|
|
return "", err
|
|
}
|
|
if keyDir == "" {
|
|
return "", fmt.Errorf("safeAgentKeyPath: empty keyDir")
|
|
}
|
|
cleanDir, err := filepath.Abs(filepath.Clean(keyDir))
|
|
if err != nil {
|
|
return "", fmt.Errorf("safeAgentKeyPath: resolve keyDir: %w", err)
|
|
}
|
|
joined := filepath.Join(cleanDir, certificateID+".key")
|
|
cleanJoined := filepath.Clean(joined)
|
|
rel, err := filepath.Rel(cleanDir, cleanJoined)
|
|
if err != nil {
|
|
return "", fmt.Errorf("safeAgentKeyPath: rel(%q,%q): %w", cleanDir, cleanJoined, err)
|
|
}
|
|
// Reject any path that escapes the directory: a leading ".." in the
|
|
// relative form means the joined path resolved outside keyDir.
|
|
if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
|
|
return "", fmt.Errorf("safeAgentKeyPath: %q escapes keyDir %q (rel=%q)", certificateID, cleanDir, rel)
|
|
}
|
|
// Belt-and-suspenders: the rel form must also not contain a NUL.
|
|
if strings.ContainsRune(rel, 0) {
|
|
return "", fmt.Errorf("safeAgentKeyPath: NUL byte in computed path")
|
|
}
|
|
return cleanJoined, nil
|
|
}
|
|
|
|
// ensureAgentKeyDirSecure creates dir (and ancestors) with mode 0700 or
|
|
// asserts an existing dir is owner-only. If a pre-existing dir is more
|
|
// permissive than 0700 we tighten it to 0700 (logging-free; this is a
|
|
// startup-style invariant, not a per-request check).
|
|
func ensureAgentKeyDirSecure(dir string) error {
|
|
if dir == "" || dir == "." || dir == "/" {
|
|
return fmt.Errorf("ensureAgentKeyDirSecure: refuse empty/root dir %q", dir)
|
|
}
|
|
clean := filepath.Clean(dir)
|
|
info, err := os.Stat(clean)
|
|
switch {
|
|
case os.IsNotExist(err):
|
|
if mkErr := os.MkdirAll(clean, 0o700); mkErr != nil {
|
|
return fmt.Errorf("create agent key dir %q: %w", clean, mkErr)
|
|
}
|
|
info, err = os.Stat(clean)
|
|
if err != nil {
|
|
return fmt.Errorf("stat newly-created agent key dir %q: %w", clean, err)
|
|
}
|
|
fallthrough
|
|
case err == nil:
|
|
mode := info.Mode().Perm()
|
|
if mode == 0o700 || mode&0o077 == 0 {
|
|
return nil
|
|
}
|
|
if chmodErr := os.Chmod(clean, 0o700); chmodErr != nil {
|
|
return fmt.Errorf("tighten agent key dir %q from %#o to 0700: %w", clean, mode, chmodErr)
|
|
}
|
|
return nil
|
|
default:
|
|
return fmt.Errorf("stat agent key dir %q: %w", clean, err)
|
|
}
|
|
}
|