security: hardening по результатам аудита безопасности
This commit is contained in:
@@ -55,7 +55,6 @@ func RegisterFlags(cmd *cobra.Command) {
|
||||
flags.String("pg-host", "", "PostgreSQL host")
|
||||
flags.Int("pg-port", 0, "PostgreSQL port")
|
||||
flags.String("pg-user", "", "PostgreSQL user")
|
||||
flags.String("pg-password", "", "PostgreSQL password")
|
||||
flags.String("pg-database", "", "PostgreSQL database name")
|
||||
flags.String("pg-sslmode", "", "PostgreSQL SSL mode")
|
||||
flags.StringSlice("pg-exclude-tables", nil, "PostgreSQL tables to exclude from backup")
|
||||
@@ -94,7 +93,6 @@ func Load(cmd *cobra.Command) (*Config, error) {
|
||||
_ = v.BindPFlag("pg.host", cmd.Flags().Lookup("pg-host"))
|
||||
_ = v.BindPFlag("pg.port", cmd.Flags().Lookup("pg-port"))
|
||||
_ = v.BindPFlag("pg.user", cmd.Flags().Lookup("pg-user"))
|
||||
_ = v.BindPFlag("pg.password", cmd.Flags().Lookup("pg-password"))
|
||||
_ = v.BindPFlag("pg.database", cmd.Flags().Lookup("pg-database"))
|
||||
_ = v.BindPFlag("pg.sslmode", cmd.Flags().Lookup("pg-sslmode"))
|
||||
_ = v.BindPFlag("pg.exclude_tables", cmd.Flags().Lookup("pg-exclude-tables"))
|
||||
@@ -151,6 +149,10 @@ func Load(cmd *cobra.Command) (*Config, error) {
|
||||
v.SetEnvPrefix("APP")
|
||||
v.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
|
||||
v.AutomaticEnv()
|
||||
// pg.password is intentionally not exposed as a CLI flag (CWE-214), but
|
||||
// must still be loadable from APP_PG_PASSWORD. Viper needs an explicit
|
||||
// BindEnv for a nested key that has no bound flag.
|
||||
_ = v.BindEnv("pg.password")
|
||||
|
||||
var cfg Config
|
||||
if err := v.Unmarshal(&cfg); err != nil {
|
||||
|
||||
@@ -74,6 +74,11 @@ var (
|
||||
// ErrUnexpectedEOF indicates the chunk stream ended before any chunk
|
||||
// with flags&0x01==1 (logical end-of-stream marker) was observed.
|
||||
ErrUnexpectedEOF = errors.New("composite: unexpected end of stream")
|
||||
|
||||
// ErrPlaintextTooLarge indicates the decrypted payload would exceed the
|
||||
// configured maximum plaintext size; decryption is aborted before the
|
||||
// limit is crossed to prevent unbounded disk consumption.
|
||||
ErrPlaintextTooLarge = errors.New("composite: plaintext size limit exceeded")
|
||||
)
|
||||
|
||||
// Format constants.
|
||||
@@ -89,6 +94,7 @@ const (
|
||||
wrappedCekLen int = 48 // 32-byte CEK + 16-byte GCM tag
|
||||
firstPayloadNonceLen int = 12
|
||||
kekLen int = 32
|
||||
maxPlaintextSize int64 = 1 << 40 // 1 TiB cap on decrypted output
|
||||
infoPq string = "git.tswf.io/infra/go-synapse-backupper/v2/kek/pq"
|
||||
infoComposite string = "git.tswf.io/infra/go-synapse-backupper/v2/kek/composite"
|
||||
)
|
||||
@@ -505,7 +511,24 @@ func (d *decryptor) Decrypt(
|
||||
return err
|
||||
}
|
||||
|
||||
return decryptChunks(src, plaintext, payloadGcm, firstPayloadNonce)
|
||||
plaintextLimiter := &limitedWriter{writer: plaintext, remaining: maxPlaintextSize}
|
||||
return decryptChunks(src, plaintextLimiter, payloadGcm, firstPayloadNonce)
|
||||
}
|
||||
|
||||
// limitedWriter wraps an io.Writer and rejects writes that would exceed a
|
||||
// maximum byte budget.
|
||||
type limitedWriter struct {
|
||||
writer io.Writer
|
||||
remaining int64
|
||||
}
|
||||
|
||||
func (lw *limitedWriter) Write(p []byte) (int, error) {
|
||||
if int64(len(p)) > lw.remaining {
|
||||
return 0, ErrPlaintextTooLarge
|
||||
}
|
||||
n, err := lw.writer.Write(p)
|
||||
lw.remaining -= int64(n)
|
||||
return n, err
|
||||
}
|
||||
|
||||
// decryptChunks reads and decrypts chunk records until a final chunk
|
||||
|
||||
@@ -947,3 +947,41 @@ func loadGoldenPrivs(
|
||||
}
|
||||
return newFakePriv(fakePqSchemeID, pqRaw), newFakePriv(fakeClassicalSchemeID, classicalRaw)
|
||||
}
|
||||
|
||||
func TestLimitedWriter_AllowsWritesWithinBudget(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
lw := &limitedWriter{writer: &buf, remaining: 10}
|
||||
|
||||
n, err := lw.Write([]byte("hello"))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if n != 5 {
|
||||
t.Errorf("wrote %d bytes, want 5", n)
|
||||
}
|
||||
|
||||
n, err = lw.Write([]byte("world"))
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if n != 5 {
|
||||
t.Errorf("wrote %d bytes, want 5", n)
|
||||
}
|
||||
|
||||
if buf.String() != "helloworld" {
|
||||
t.Errorf("buffer = %q, want %q", buf.String(), "helloworld")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLimitedWriter_RejectsOverBudget(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
lw := &limitedWriter{writer: &buf, remaining: 3}
|
||||
|
||||
_, err := lw.Write([]byte("hello"))
|
||||
if !errors.Is(err, ErrPlaintextTooLarge) {
|
||||
t.Errorf("error = %v, want ErrPlaintextTooLarge", err)
|
||||
}
|
||||
if buf.Len() != 0 {
|
||||
t.Errorf("buffer len = %d, want 0", buf.Len())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -26,7 +26,8 @@ func New(port int) (*Server, error) {
|
||||
|
||||
s := &Server{listener: listener}
|
||||
s.server = &http.Server{
|
||||
Handler: http.HandlerFunc(s.handleHealthz),
|
||||
Handler: http.HandlerFunc(s.handleHealthz),
|
||||
ReadHeaderTimeout: 5 * time.Second,
|
||||
}
|
||||
return s, nil
|
||||
}
|
||||
|
||||
@@ -6,11 +6,21 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"git.tswf.io/infra/go-synapse-backupper/pkg/domain/pgdump"
|
||||
)
|
||||
|
||||
var (
|
||||
passwordPattern = regexp.MustCompile(`(?i)\bpassword=[^\s]*`)
|
||||
hostPattern = regexp.MustCompile(`(?i)\bhost=[^\s]*`)
|
||||
// excludeTablePattern accepts unquoted PostgreSQL identifiers or the
|
||||
// schema.table form. It rejects shell-special characters and injection
|
||||
// payloads while still allowing the default Synapse table name.
|
||||
excludeTablePattern = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_$]*(\.[a-zA-Z_][a-zA-Z0-9_$]*)?$`)
|
||||
)
|
||||
|
||||
// adapter provides pg_dump functionality using the system's pg_dump binary.
|
||||
type adapter struct {
|
||||
commandContext func(ctx context.Context, name string, arg ...string) *exec.Cmd
|
||||
@@ -24,6 +34,10 @@ func New() pgdump.Dumper {
|
||||
}
|
||||
|
||||
// Dump executes pg_dump and writes the output to sink.
|
||||
//
|
||||
// The password is never passed through the PGPASSWORD environment variable;
|
||||
// instead a temporary .pgpass file with 0o600 permissions is created and
|
||||
// pointed to via PGPASSFILE.
|
||||
func (a *adapter) Dump(
|
||||
ctx context.Context,
|
||||
opts pgdump.Options,
|
||||
@@ -39,14 +53,24 @@ func (a *adapter) Dump(
|
||||
}
|
||||
}()
|
||||
|
||||
args := buildArgs(opts)
|
||||
args, err := buildArgs(opts)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
cmd := a.commandContext(ctx, "pg_dump", args...)
|
||||
|
||||
env := os.Environ()
|
||||
if opts.Password != "" {
|
||||
env = append(env, fmt.Sprintf("PGPASSWORD=%s", opts.Password))
|
||||
|
||||
pgpassPath, cleanupPgpass, err := writePgPassFile(opts)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer cleanupPgpass()
|
||||
if pgpassPath != "" {
|
||||
env = append(env, fmt.Sprintf("PGPASSFILE=%s", pgpassPath))
|
||||
}
|
||||
|
||||
if opts.Host != "" {
|
||||
env = append(env, fmt.Sprintf("PGHOST=%s", opts.Host))
|
||||
}
|
||||
@@ -66,7 +90,7 @@ func (a *adapter) Dump(
|
||||
cmd.Stdout = sink
|
||||
|
||||
runErr := cmd.Run()
|
||||
stderr := strings.TrimSpace(stderrBuilder.String())
|
||||
stderr := sanitizeStderr(strings.TrimSpace(stderrBuilder.String()))
|
||||
|
||||
if runErr != nil {
|
||||
if exitErr, ok := runErr.(*exec.ExitError); ok {
|
||||
@@ -82,7 +106,56 @@ func (a *adapter) Dump(
|
||||
return nil
|
||||
}
|
||||
|
||||
func buildArgs(opts pgdump.Options) []string {
|
||||
// writePgPassFile creates a temporary .pgpass file when a password is provided.
|
||||
// The returned cleanup function removes the file; callers should defer it.
|
||||
func writePgPassFile(opts pgdump.Options) (string, func(), error) {
|
||||
if opts.Password == "" {
|
||||
return "", func() {}, nil
|
||||
}
|
||||
|
||||
passFile, err := os.CreateTemp("", "pgpass-*.conf")
|
||||
if err != nil {
|
||||
return "", nil, fmt.Errorf("create temporary pgpass file: %w", err)
|
||||
}
|
||||
path := passFile.Name()
|
||||
cleanup := func() { _ = os.Remove(path) }
|
||||
|
||||
line := fmt.Sprintf(
|
||||
"%s:%d:%s:%s:%s\n",
|
||||
opts.Host,
|
||||
opts.Port,
|
||||
opts.Database,
|
||||
opts.User,
|
||||
opts.Password,
|
||||
)
|
||||
if _, err := passFile.WriteString(line); err != nil {
|
||||
_ = passFile.Close()
|
||||
cleanup()
|
||||
return "", nil, fmt.Errorf("write temporary pgpass file: %w", err)
|
||||
}
|
||||
if err := passFile.Close(); err != nil {
|
||||
cleanup()
|
||||
return "", nil, fmt.Errorf("close temporary pgpass file: %w", err)
|
||||
}
|
||||
if err := os.Chmod(path, 0o600); err != nil {
|
||||
cleanup()
|
||||
return "", nil, fmt.Errorf("chmod temporary pgpass file: %w", err)
|
||||
}
|
||||
return path, cleanup, nil
|
||||
}
|
||||
|
||||
// sanitizeStderr removes sensitive connection-string fragments from pg_dump
|
||||
// diagnostics before they are logged.
|
||||
func sanitizeStderr(input string) string {
|
||||
if input == "" {
|
||||
return ""
|
||||
}
|
||||
out := passwordPattern.ReplaceAllString(input, "password=***")
|
||||
out = hostPattern.ReplaceAllString(out, "host=***")
|
||||
return out
|
||||
}
|
||||
|
||||
func buildArgs(opts pgdump.Options) ([]string, error) {
|
||||
excludeTables := opts.ExcludeTables
|
||||
if len(excludeTables) == 0 {
|
||||
excludeTables = make([]string, 1)
|
||||
@@ -93,8 +166,11 @@ func buildArgs(opts pgdump.Options) []string {
|
||||
args = append(args, "--format=custom")
|
||||
|
||||
for _, table := range excludeTables {
|
||||
if !excludeTablePattern.MatchString(table) {
|
||||
return nil, fmt.Errorf("invalid exclude-table identifier %q", table)
|
||||
}
|
||||
args = append(args, "--exclude-table="+table)
|
||||
}
|
||||
|
||||
return args
|
||||
return args, nil
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"os/exec"
|
||||
"reflect"
|
||||
"strings"
|
||||
@@ -282,11 +283,15 @@ func TestDump_EnvVars(t *testing.T) {
|
||||
}
|
||||
|
||||
envStr := strings.Join(capturedCmd.Env, "\n")
|
||||
|
||||
if strings.Contains(envStr, "PGPASSWORD=") {
|
||||
t.Errorf("PGPASSWORD must not be passed to pg_dump subprocess")
|
||||
}
|
||||
|
||||
wantEnvVars := []string{
|
||||
"PGHOST=myhost",
|
||||
"PGPORT=5433",
|
||||
"PGUSER=myuser",
|
||||
"PGPASSWORD=mypass",
|
||||
"PGDATABASE=mydb",
|
||||
}
|
||||
for _, wantEnv := range wantEnvVars {
|
||||
@@ -294,6 +299,102 @@ func TestDump_EnvVars(t *testing.T) {
|
||||
t.Errorf("env missing %q", wantEnv)
|
||||
}
|
||||
}
|
||||
|
||||
if !strings.Contains(envStr, "PGPASSFILE=") {
|
||||
t.Errorf("PGPASSFILE must be set when a password is provided")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDump_PgpassFileRemovedAfterRun(t *testing.T) {
|
||||
var capturedCmd *exec.Cmd
|
||||
|
||||
adapter := &adapter{
|
||||
commandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd {
|
||||
capturedCmd = exec.CommandContext(ctx, "sh", "-c", "exit 0")
|
||||
return capturedCmd
|
||||
},
|
||||
}
|
||||
|
||||
err := adapter.Dump(
|
||||
context.Background(),
|
||||
pgdump.Options{
|
||||
Password: "secret",
|
||||
Database: "mydb",
|
||||
},
|
||||
io.Discard,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
envStr := strings.Join(capturedCmd.Env, "\n")
|
||||
passFilePrefix := "PGPASSFILE="
|
||||
idx := strings.Index(envStr, passFilePrefix)
|
||||
if idx == -1 {
|
||||
t.Fatalf("PGPASSFILE not found in env")
|
||||
}
|
||||
pgpassPath := envStr[idx+len(passFilePrefix):]
|
||||
if newlineIdx := strings.Index(pgpassPath, "\n"); newlineIdx != -1 {
|
||||
pgpassPath = pgpassPath[:newlineIdx]
|
||||
}
|
||||
|
||||
if _, statErr := os.Stat(pgpassPath); !os.IsNotExist(statErr) {
|
||||
t.Errorf("temporary pgpass file %s was not removed after Dump returned", pgpassPath)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWritePgPassFile_Permissions(t *testing.T) {
|
||||
path, cleanup, err := writePgPassFile(pgdump.Options{
|
||||
Host: "myhost",
|
||||
Port: 5432,
|
||||
Database: "mydb",
|
||||
User: "myuser",
|
||||
Password: "secret",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
info, err := os.Stat(path)
|
||||
if err != nil {
|
||||
t.Fatalf("stat pgpass file: %v", err)
|
||||
}
|
||||
if info.Mode().Perm() != 0o600 {
|
||||
t.Errorf("pgpass file permissions = %o, want %o", info.Mode().Perm(), 0o600)
|
||||
}
|
||||
|
||||
content, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
t.Fatalf("read pgpass file: %v", err)
|
||||
}
|
||||
want := "myhost:5432:mydb:myuser:secret\n"
|
||||
if string(content) != want {
|
||||
t.Errorf("pgpass content = %q, want %q", string(content), want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDump_InvalidExcludeTable(t *testing.T) {
|
||||
adapter := &adapter{
|
||||
commandContext: func(ctx context.Context, name string, arg ...string) *exec.Cmd {
|
||||
return exec.CommandContext(ctx, "sh", "-c", "exit 0")
|
||||
},
|
||||
}
|
||||
|
||||
err := adapter.Dump(
|
||||
context.Background(),
|
||||
pgdump.Options{
|
||||
Database: "testdb",
|
||||
ExcludeTables: []string{"table; DROP TABLE users;--"},
|
||||
},
|
||||
io.Discard,
|
||||
)
|
||||
if err == nil {
|
||||
t.Fatal("expected error for invalid exclude-table identifier")
|
||||
}
|
||||
if !strings.Contains(err.Error(), "invalid exclude-table identifier") {
|
||||
t.Errorf("error = %v, want invalid exclude-table identifier", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDump_PipeWriterClosed(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user