diff --git a/config.go b/config.go index 458d07d..bd7b9fb 100644 --- a/config.go +++ b/config.go @@ -50,6 +50,7 @@ type options struct { NoConfigFile bool BinDirectory string Directory string + Mode int Host string Port int Username string @@ -129,6 +130,7 @@ func defaultOptions() options { return options{ NoConfigFile: false, Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -161,6 +163,17 @@ func (*parseCliResult) Error() string { return "please exit now" } +func validateMode(s string) (int, error) { + if (strings.HasPrefix(s, "0") && len(s) <= 5) || (strings.HasPrefix(s, "-")) { + mode, err := strconv.ParseInt(s, 0, 32) + if err != nil { + return 0, fmt.Errorf("Invalid permission %q", s) + } + return int(mode), nil + } + return 0, fmt.Errorf("Invalid permission %q, must be octal (start by 0 and max 5 digits) number or negative", s) +} + func validateDumpFormat(s string) error { for _, format := range []string{"plain", "custom", "tar", "directory"} { // PostgreSQL tools allow the full name of the format and the @@ -251,7 +264,7 @@ func validateDirectory(s string) error { } func parseCli(args []string) (options, []string, error) { - var format, purgeKeep, purgeInterval string + var format, mode, purgeKeep, purgeInterval string opts := defaultOptions() pce := &parseCliResult{} @@ -268,6 +281,7 @@ func parseCli(args []string) (options, []string, error) { pflag.BoolVar(&opts.NoConfigFile, "no-config-file", false, "skip reading config file\n") pflag.StringVarP(&opts.BinDirectory, "bin-directory", "B", "", "PostgreSQL binaries directory. Empty to search $PATH") pflag.StringVarP(&opts.Directory, "backup-directory", "b", "/var/backups/postgresql", "store dump files there") + pflag.StringVarP(&mode, "backup-file-mode", "m", "0600", "mode to apply to dump files") pflag.StringVarP(&opts.CfgFile, "config", "c", defaultCfgFile, "alternate config file") pflag.StringSliceVarP(&opts.ExcludeDbs, "exclude-dbs", "D", []string{}, "list of databases to exclude") pflag.BoolVarP(&opts.WithTemplates, "with-templates", "t", false, "include templates") @@ -414,6 +428,12 @@ func parseCli(args []string) (options, []string, error) { changed = append(changed, "include-dbs") } + parsed_mode, err := validateMode(mode) + if err != nil { + return opts, changed, fmt.Errorf("invalid value for --backup-file-mode: %s", err) + } + opts.Mode = parsed_mode + // Validate purge keep and time limit keep, err := validatePurgeKeepValue(purgeKeep) if err != nil { @@ -520,7 +540,7 @@ func validateConfigurationFile(cfg *ini.File) error { s, _ := cfg.GetSection(ini.DefaultSection) known_globals := []string{ - "bin_directory", "backup_directory", "timestamp_format", "host", "port", "user", + "bin_directory", "backup_directory", "backup_file_mode", "timestamp_format", "host", "port", "user", "dbname", "exclude_dbs", "include_dbs", "with_templates", "format", "parallel_backup_jobs", "compress_level", "jobs", "pause_timeout", "purge_older_than", "purge_min_keep", "checksum_algorithm", "pre_backup_hook", @@ -574,7 +594,7 @@ gkLoop: } func loadConfigurationFile(path string) (options, error) { - var format, purgeKeep, purgeInterval string + var format, mode, purgeKeep, purgeInterval string opts := defaultOptions() @@ -600,6 +620,7 @@ func loadConfigurationFile(path string) (options, error) { // flags opts.BinDirectory = s.Key("bin_directory").MustString("") opts.Directory = s.Key("backup_directory").MustString("/var/backups/postgresql") + mode = s.Key("backup_file_mode").MustString("0600") timeFormat := s.Key("timestamp_format").MustString("rfc3339") opts.Host = s.Key("host").MustString("") opts.Port = s.Key("port").MustInt(0) @@ -662,6 +683,13 @@ func loadConfigurationFile(path string) (options, error) { opts.AzureKey = s.Key("azure_key").MustString("") opts.AzureEndpoint = s.Key("azure_endpoint").MustString("blob.core.windows.net") + // Validate mode and convert to int + m, err := validateMode(mode) + if err != nil { + return opts, err + } + opts.Mode = m + // Validate purge keep and time limit keep, err := validatePurgeKeepValue(purgeKeep) if err != nil { @@ -811,6 +839,8 @@ func mergeCliAndConfigOptions(cliOpts options, configOpts options, onCli []strin opts.BinDirectory = cliOpts.BinDirectory case "backup-directory": opts.Directory = cliOpts.Directory + case "backup-file-mode": + opts.Mode = cliOpts.Mode case "exclude-dbs": opts.ExcludeDbs = cliOpts.ExcludeDbs case "include-dbs": diff --git a/config_test.go b/config_test.go index 0072f22..72bbb12 100644 --- a/config_test.go +++ b/config_test.go @@ -62,6 +62,35 @@ func TestValidateDumpFormat(t *testing.T) { } +func TestValidateMode(t *testing.T) { + var tests = []struct { + give string + want int + wantError bool + }{ + {"0700", 448, false}, + {"070000", 0, true}, // invalid mode (too long) + {"18446744000", 0, true}, // still invalid, positive integer + {"08170", 0, true}, // non valid mode (8 on it) + {"-8170", -8170, false}, // valid and mean do nothing (useful when using umask) + } + + l.logger.SetOutput(ioutil.Discard) + for i, st := range tests { + t.Run(fmt.Sprintf("%v", i), func(t *testing.T) { + got, err := validateMode(st.give) + if err == nil && st.wantError { + t.Errorf("excepted an error got nil") + } else if err != nil && !st.wantError { + t.Errorf("did not want an error, got %s", err) + } + if got != st.want { + t.Errorf("got %v, want %v", got, st.want) + } + }) + } +} + func TestValidatePurgeKeepValue(t *testing.T) { var tests = []struct { give string @@ -183,6 +212,7 @@ func TestDefaultOptions(t *testing.T) { var want = options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -228,6 +258,7 @@ func TestParseCli(t *testing.T) { []string{"-b", "test", "-Z", "2", "a", "b"}, options{ Directory: "test", + Mode: 0o600, Dbnames: []string{"a", "b"}, Format: 'c', DirJobs: 1, @@ -255,6 +286,7 @@ func TestParseCli(t *testing.T) { []string{"-t", "--without-templates"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, WithTemplates: false, Format: 'c', DirJobs: 1, @@ -306,6 +338,7 @@ func TestParseCli(t *testing.T) { []string{"--upload", "wrong"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -334,6 +367,7 @@ func TestParseCli(t *testing.T) { []string{"--download", "wrong"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -370,6 +404,7 @@ func TestParseCli(t *testing.T) { []string{"--cipher-pass", "mypass"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -398,6 +433,7 @@ func TestParseCli(t *testing.T) { []string{"--cipher-private-key", "mykey"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -426,6 +462,7 @@ func TestParseCli(t *testing.T) { []string{"--cipher-public-key", "fakepubkey"}, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -538,10 +575,11 @@ func TestLoadConfigurationFile(t *testing.T) { want options }{ { - []string{"backup_directory = test", "port = 5433"}, + []string{"backup_directory = test", "port = 5433", "backup_file_mode = 0700"}, false, options{ Directory: "test", + Mode: 0o700, Port: 5433, Format: 'c', DirJobs: 1, @@ -562,10 +600,11 @@ func TestLoadConfigurationFile(t *testing.T) { }, }, { // ensure comma separated lists work - []string{"backup_directory = test", "include_dbs = a, b, postgres", "compress_level = 9"}, + []string{"backup_directory = test", "include_dbs = a, b, postgres", "compress_level = 9", "backup_file_mode = 0400"}, false, options{ Directory: "test", + Mode: 0o400, Dbnames: []string{"a", "b", "postgres"}, Format: 'c', DirJobs: 1, @@ -590,6 +629,7 @@ func TestLoadConfigurationFile(t *testing.T) { false, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -613,6 +653,7 @@ func TestLoadConfigurationFile(t *testing.T) { false, options{ Directory: "/var/backups/postgresql", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -654,6 +695,7 @@ func TestLoadConfigurationFile(t *testing.T) { false, options{ Directory: "test", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: -1, @@ -698,6 +740,7 @@ func TestLoadConfigurationFile(t *testing.T) { false, options{ Directory: "test", + Mode: 0o600, Format: 'c', DirJobs: 1, CompressLevel: 3, @@ -774,6 +817,7 @@ func TestMergeCliAndConfigoptions(t *testing.T) { want := options{ BinDirectory: "/bin", Directory: "test", + Mode: 0o600, Host: "localhost", Port: 5433, Username: "test", diff --git a/crypto.go b/crypto.go index 97ef0c4..541ae43 100644 --- a/crypto.go +++ b/crypto.go @@ -130,7 +130,7 @@ func ageDecryptInternal(src io.Reader, dst io.Writer, identity age.Identity) err return nil } -func encryptFile(path string, params encryptParams, keep bool) ([]string, error) { +func encryptFile(path string, mode int, params encryptParams, keep bool) ([]string, error) { encrypted := make([]string, 0) i, err := os.Stat(path) @@ -210,7 +210,11 @@ func encryptFile(path string, params encryptParams, keep bool) ([]string, error) } encrypted = append(encrypted, dstFile) - + if mode > 0 { + if err := os.Chmod(dstFile, os.FileMode(mode)); err != nil { + return encrypted, fmt.Errorf("could not chmod to more secure permission for encrypted file: %w", err) + } + } if !keep { l.Verboseln("removing source file:", path) src.Close() diff --git a/hash.go b/hash.go index be5832b..f6f5ed4 100644 --- a/hash.go +++ b/hash.go @@ -51,7 +51,7 @@ func computeChecksum(path string, h hash.Hash) (string, error) { return string(h.Sum(nil)), nil } -func checksumFile(path string, algo string) (string, error) { +func checksumFile(path string, mode int, algo string) (string, error) { var h hash.Hash switch algo { @@ -114,10 +114,16 @@ func checksumFile(path string, algo string) (string, error) { r, _ := computeChecksum(path, h) fmt.Fprintf(o, "%x %s\n", r, path) } + l.Verboseln("computing checksum with MODE", mode, path) + if mode > 0 { + if err := os.Chmod(o.Name(), os.FileMode(mode)); err != nil { + return "", fmt.Errorf("could not chmod checksum file %s: %s", path, err) + } + } return sumFile, nil } -func checksumFileList(paths []string, algo string, sumFilePrefix string) (string, error) { +func checksumFileList(paths []string, mode int, algo string, sumFilePrefix string) (string, error) { var h hash.Hash switch algo { @@ -157,6 +163,12 @@ func checksumFileList(paths []string, algo string, sumFilePrefix string) (string } fmt.Fprintf(o, "%x *%s\n", r, path) + + if mode > 0 { + if err := os.Chmod(o.Name(), os.FileMode(mode)); err != nil { + return "", fmt.Errorf("could not chmod checksum file %s: %s", path, err) + } + } } if failed { diff --git a/hash_test.go b/hash_test.go index 1c35e71..be86a25 100644 --- a/hash_test.go +++ b/hash_test.go @@ -77,18 +77,18 @@ func TestChecksumFile(t *testing.T) { } // bad algo - if _, err := checksumFile("", "none"); err != nil { + if _, err := checksumFile("", 0o700, "none"); err != nil { t.Errorf("expected , got %q\n", err) } - if _, err := checksumFile("", "other"); err == nil { + if _, err := checksumFile("", 0o700, "other"); err == nil { t.Errorf("expected err, got \n") } // test each algo with the file for i, st := range tests { t.Run(fmt.Sprintf("f%v", i), func(t *testing.T) { - if _, err := checksumFile("test", st.algo); err != nil { + if _, err := checksumFile("test", 0o700, st.algo); err != nil { t.Errorf("checksumFile returned: %v", err) } @@ -111,12 +111,12 @@ func TestChecksumFile(t *testing.T) { // bad files var e *os.PathError l.logger.SetOutput(ioutil.Discard) - if _, err := checksumFile("", "sha1"); !errors.As(err, &e) { + if _, err := checksumFile("", 0o700, "sha1"); !errors.As(err, &e) { t.Errorf("expected an *os.PathError, got %q\n", err) } os.Chmod("test.sha1", 0444) - if _, err := checksumFile("test", "sha1"); !errors.As(err, &e) { + if _, err := checksumFile("test", 0o700, "sha1"); !errors.As(err, &e) { t.Errorf("expected an *os.PathError, got %q\n", err) } os.Chmod("test.sha1", 0644) @@ -138,7 +138,7 @@ func TestChecksumFile(t *testing.T) { // test each algo with the directory for i, st := range tests { t.Run(fmt.Sprintf("d%v", i), func(t *testing.T) { - if _, err := checksumFile("test.d", st.algo); err != nil { + if _, err := checksumFile("test.d", 0o700, st.algo); err != nil { t.Errorf("checksumFile returned: %v", err) } diff --git a/main.go b/main.go index 32c3d9a..ac768a9 100644 --- a/main.go +++ b/main.go @@ -56,6 +56,9 @@ type dump struct { // Directory is the target directory where to create the dump Directory string + // Mode is the permission for the resulting backup + Mode int + // Time format for the filename TimeFormat string @@ -313,7 +316,7 @@ func run() (retVal error) { } else { l.Infoln("dumping globals without role passwords") } - if err := dumpGlobals(opts.Directory, opts.TimeFormat, dumpRolePasswords, conninfo, producedFiles); err != nil { + if err := dumpGlobals(opts.Directory, opts.Mode, opts.TimeFormat, dumpRolePasswords, conninfo, producedFiles); err != nil { return fmt.Errorf("pg_dumpall of globals failed: %w", err) } @@ -323,7 +326,7 @@ func run() (retVal error) { perr *pgPrivError ) - if err := dumpSettings(opts.Directory, opts.TimeFormat, db, producedFiles); err != nil { + if err := dumpSettings(opts.Directory, opts.Mode, opts.TimeFormat, db, producedFiles); err != nil { if errors.As(err, &verr) || errors.As(err, &perr) { l.Warnln(err) } else { @@ -331,7 +334,7 @@ func run() (retVal error) { } } - if err := dumpConfigFiles(opts.Directory, opts.TimeFormat, db, producedFiles); err != nil { + if err := dumpConfigFiles(opts.Directory, opts.Mode, opts.TimeFormat, db, producedFiles); err != nil { return fmt.Errorf("could not dump configuration files: %w", err) } } @@ -377,6 +380,7 @@ func run() (retVal error) { Database: dbname, Options: o, Directory: opts.Directory, + Mode: opts.Mode, TimeFormat: opts.TimeFormat, ConnString: conninfo, CipherPassphrase: passphrase, @@ -473,9 +477,8 @@ func run() (retVal error) { fmt.Fprintf(f, "%s", c) f.Close() - - if err := os.Chmod(aclpath, 0600); err != nil { - return fmt.Errorf("could not chmod to more secure permission for ACL %s: %s", dbname, err) + if err := os.Chmod(aclpath, os.FileMode(d.Mode)); err != nil { + return fmt.Errorf("could not chmod to more secure permission for ACL %s: %w", dbname, err) } // Have its checksum computed @@ -738,20 +741,46 @@ func (d *dump) dump(fc chan<- sumFileJob) error { d.Path = file d.ExitCode = 0 - var mode os.FileMode = 0600 - if d.Options.Format == 'd' { - // The hardening of permissions only apply to the top level - // directory, this won't make the contents executable - mode = 0700 - } - - if err := os.Chmod(file, mode); err != nil { - return fmt.Errorf("could not chmod to more secure permission for %s: %s", dbname, err) + if d.Mode > 0 { + var mode os.FileMode = os.FileMode(d.Mode) + isDirFormat := d.Options.Format == 'd' + if isDirFormat { + // calculate appropriate permission for parent directory, we need +x + // permission to walk through the directory + if (mode&0o400 > 0) || (mode&0o200 > 0) { + mode = mode | 0o100 + } + } + if err := os.Chmod(file, mode); err != nil { + return fmt.Errorf("could not chmod to more secure permission for %s: %w", dbname, err) + } + if isDirFormat { + // adapt mode on files on directory based on initial configured mode + if err := recursiveChmod(file, os.FileMode(d.Mode)); err != nil { + return err + } + } } return nil } +func recursiveChmod(file string, newMode os.FileMode) error { + err := filepath.Walk(file, func(path string, info os.FileInfo, err error) error { + if err != nil { + return fmt.Errorf("error when accesing file %s: %w", path, err) + } + if !info.IsDir() { + if err := os.Chmod(path, newMode); err != nil { + return fmt.Errorf("could not chmod %s to more secure permission: %w", file, err) + } + } + return nil + + }) + return err +} + func dumper(jobs <-chan *dump, results chan<- *dump, fc chan<- sumFileJob) { for j := range jobs { @@ -896,7 +925,7 @@ func pgToolVersion(tool string) int { return numver } -func dumpGlobals(dir string, timeFormat string, withRolePasswords bool, conninfo *ConnInfo, fc chan<- sumFileJob) error { +func dumpGlobals(dir string, mode int, timeFormat string, withRolePasswords bool, conninfo *ConnInfo, fc chan<- sumFileJob) error { command := execPath("pg_dumpall") args := []string{"-g", "-w"} @@ -954,9 +983,10 @@ func dumpGlobals(dir string, timeFormat string, withRolePasswords bool, conninfo } } } - - if err := os.Chmod(file, 0600); err != nil { - return fmt.Errorf("could not chmod to more secure permission for pg_globals: %s", err) + if mode > 0 { + if err := os.Chmod(file, os.FileMode(mode)); err != nil { + return fmt.Errorf("could not chmod to more secure permission for pg_globals: %w", err) + } } if fc != nil { @@ -968,11 +998,11 @@ func dumpGlobals(dir string, timeFormat string, withRolePasswords bool, conninfo return nil } -func dumpSettings(dir string, timeFormat string, db *pg, fc chan<- sumFileJob) error { +func dumpSettings(dir string, mode int, timeFormat string, db *pg, fc chan<- sumFileJob) error { file := formatDumpPath(dir, timeFormat, "out", "pg_settings", time.Now(), 0) - if err := os.MkdirAll(filepath.Dir(file), 0700); err != nil { + if err := os.MkdirAll(filepath.Dir(file), 0o700); err != nil { return err } @@ -981,12 +1011,18 @@ func dumpSettings(dir string, timeFormat string, db *pg, fc chan<- sumFileJob) e return err } - // Use a Buffer to avoid creating an empty file if len(s) > 0 { l.Verboseln("writing settings to:", file) - if err := os.WriteFile(file, []byte(s), 0600); err != nil { + f, err := os.Create(file) + if err != nil { return err } + fmt.Fprintf(f, "%s", s) + if mode > 0 { // otherwhise let umask do the job + if err := os.Chmod(f.Name(), os.FileMode(mode)); err != nil { + return fmt.Errorf("could not chmod to more secure permission for settings file %s: %w", f.Name(), err) + } + } if fc != nil { fc <- sumFileJob{ @@ -998,7 +1034,7 @@ func dumpSettings(dir string, timeFormat string, db *pg, fc chan<- sumFileJob) e return nil } -func dumpConfigFiles(dir string, timeFormat string, db *pg, fc chan<- sumFileJob) error { +func dumpConfigFiles(dir string, mode int, timeFormat string, db *pg, fc chan<- sumFileJob) error { for _, param := range []string{"hba_file", "ident_file"} { file := formatDumpPath(dir, timeFormat, "out", param, time.Now(), 0) @@ -1014,9 +1050,16 @@ func dumpConfigFiles(dir string, timeFormat string, db *pg, fc chan<- sumFileJob // Use a Buffer to avoid creating an empty file if len(s) > 0 { l.Verbosef("writing contents of '%s' to: %s", param, file) - if err := os.WriteFile(file, []byte(s), 0600); err != nil { + f, err := os.Create(file) + if err != nil { return err } + fmt.Fprintf(f, "%s", s) + if mode > 0 { // otherwhise let umask do the job + if err := os.Chmod(f.Name(), os.FileMode(mode)); err != nil { + return fmt.Errorf("could not chmod to more secure permission for settings file %s: %w", f.Name(), err) + } + } // We have produced a file send it to the channel for // further processing @@ -1346,7 +1389,7 @@ func postProcessFiles(inFiles chan sumFileJob, wg *sync.WaitGroup, opts options) if j.SumAlgo != "none" { l.Infoln("computing checksum of", j.Path) - p, err := checksumFile(j.Path, j.SumAlgo) + p, err := checksumFile(j.Path, opts.Mode, j.SumAlgo) if err != nil { l.Errorln("checksum failed:", err) if !failed { @@ -1441,7 +1484,7 @@ func postProcessFiles(inFiles chan sumFileJob, wg *sync.WaitGroup, opts options) if opts.Encrypt { l.Infoln("encrypting", j.Path) - encFiles, err := encryptFile(j.Path, j.Params, j.KeepSrc) + encFiles, err := encryptFile(j.Path, opts.Mode, j.Params, j.KeepSrc) if err != nil { l.Errorln("encryption failed:", err) if !failed { @@ -1491,7 +1534,7 @@ func postProcessFiles(inFiles chan sumFileJob, wg *sync.WaitGroup, opts options) if j.SumAlgo != "none" { l.Infoln("computing checksum of", j.SumFile) - p, err := checksumFileList(j.Paths, j.SumAlgo, j.SumFile) + p, err := checksumFileList(j.Paths, opts.Mode, j.SumAlgo, j.SumFile) if err != nil { l.Errorln("checksum of encrypted files failed:", err) if !failed { diff --git a/pg_back.conf b/pg_back.conf index a3f18fe..9987602 100644 --- a/pg_back.conf +++ b/pg_back.conf @@ -8,6 +8,16 @@ bin_directory = # being dumped. backup_directory = /var/backups/postgresql +# Mode permissions to apply to database dumps after the dump is completed. +# This parameter uses Unix file permission chmod / mode with an octal +# representation (Example: 0700 or 0600). A negative value can be used to +# disable modifying permission and let the system handle that (example when +# umask is defined). When the format is set to directory, pg_back ensures +# the top-level directory is traversable by adding execute (+x) permission +# if read (r) or write (w) permission is set. This does not affect the +# permissions of files inside the directory. +backup_file_mode = 0600 + # Timestamp format to use in filenames of output files. Two values are # possible: legacy and rfc3339. For example legacy is 2006-01-02_15-04-05, and # rfc3339 is 2006-01-02T15:04:05-07:00. rfc3339 is the default, except on