Skip to content

Commit f3bcdc0

Browse files
l00ptrl00ptr
authored
Add an option to configure permission on dump (#138)
Add an option / configuration item to define the permissions for the dumps and hash files created by pg_back. The 0600 permissions mode is defined by default to ensure compatibility with the old (non configurable) approach. pg_back adds appropriate (+x) permission for parent directory when using the 'd' (AKA directory) dump format, files under the parent directory use the permission configured by the user (though backup_file_mode). Authored-by: Julian Vanden Broeck <[email protected]>
1 parent 1edbf9e commit f3bcdc0

7 files changed

Lines changed: 186 additions & 43 deletions

File tree

‎config.go‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ type options struct {
5050
NoConfigFile bool
5151
BinDirectory string
5252
Directory string
53+
Mode int
5354
Host string
5455
Port int
5556
Username string
@@ -130,6 +131,7 @@ func defaultOptions() options {
130131
return options{
131132
NoConfigFile: false,
132133
Directory: "/var/backups/postgresql",
134+
Mode: 0o600,
133135
Format: 'c',
134136
DirJobs: 1,
135137
CompressLevel: -1,
@@ -162,6 +164,17 @@ func (*parseCliResult) Error() string {
162164
return "please exit now"
163165
}
164166

167+
func validateMode(s string) (int, error) {
168+
if (strings.HasPrefix(s, "0") && len(s) <= 5) || (strings.HasPrefix(s, "-")) {
169+
mode, err := strconv.ParseInt(s, 0, 32)
170+
if err != nil {
171+
return 0, fmt.Errorf("Invalid permission %q", s)
172+
}
173+
return int(mode), nil
174+
}
175+
return 0, fmt.Errorf("Invalid permission %q, must be octal (start by 0 and max 5 digits) number or negative", s)
176+
}
177+
165178
func validateDumpFormat(s string) error {
166179
for _, format := range []string{"plain", "custom", "tar", "directory"} {
167180
// PostgreSQL tools allow the full name of the format and the
@@ -252,7 +265,7 @@ func validateDirectory(s string) error {
252265
}
253266

254267
func parseCli(args []string) (options, []string, error) {
255-
var format, purgeKeep, purgeInterval string
268+
var format, mode, purgeKeep, purgeInterval string
256269

257270
opts := defaultOptions()
258271
pce := &parseCliResult{}
@@ -269,6 +282,7 @@ func parseCli(args []string) (options, []string, error) {
269282
pflag.BoolVar(&opts.NoConfigFile, "no-config-file", false, "skip reading config file\n")
270283
pflag.StringVarP(&opts.BinDirectory, "bin-directory", "B", "", "PostgreSQL binaries directory. Empty to search $PATH")
271284
pflag.StringVarP(&opts.Directory, "backup-directory", "b", "/var/backups/postgresql", "store dump files there")
285+
pflag.StringVarP(&mode, "backup-file-mode", "m", "0600", "mode to apply to dump files")
272286
pflag.StringVarP(&opts.CfgFile, "config", "c", defaultCfgFile, "alternate config file")
273287
pflag.StringSliceVarP(&opts.ExcludeDbs, "exclude-dbs", "D", []string{}, "list of databases to exclude")
274288
pflag.BoolVarP(&opts.WithTemplates, "with-templates", "t", false, "include templates")
@@ -416,6 +430,12 @@ func parseCli(args []string) (options, []string, error) {
416430
changed = append(changed, "include-dbs")
417431
}
418432

433+
parsed_mode, err := validateMode(mode)
434+
if err != nil {
435+
return opts, changed, fmt.Errorf("invalid value for --backup-file-mode: %s", err)
436+
}
437+
opts.Mode = parsed_mode
438+
419439
// Validate purge keep and time limit
420440
keep, err := validatePurgeKeepValue(purgeKeep)
421441
if err != nil {
@@ -527,7 +547,7 @@ func validateConfigurationFile(cfg *ini.File) error {
527547
s, _ := cfg.GetSection(ini.DefaultSection)
528548

529549
known_globals := []string{
530-
"bin_directory", "backup_directory", "timestamp_format", "host", "port", "user",
550+
"bin_directory", "backup_directory", "backup_file_mode", "timestamp_format", "host", "port", "user",
531551
"dbname", "exclude_dbs", "include_dbs", "with_templates", "format",
532552
"parallel_backup_jobs", "compress_level", "jobs", "pause_timeout",
533553
"purge_older_than", "purge_min_keep", "checksum_algorithm", "pre_backup_hook",
@@ -581,7 +601,7 @@ gkLoop:
581601
}
582602

583603
func loadConfigurationFile(path string) (options, error) {
584-
var format, purgeKeep, purgeInterval string
604+
var format, mode, purgeKeep, purgeInterval string
585605

586606
opts := defaultOptions()
587607

@@ -607,6 +627,7 @@ func loadConfigurationFile(path string) (options, error) {
607627
// flags
608628
opts.BinDirectory = s.Key("bin_directory").MustString("")
609629
opts.Directory = s.Key("backup_directory").MustString("/var/backups/postgresql")
630+
mode = s.Key("backup_file_mode").MustString("0600")
610631
timeFormat := s.Key("timestamp_format").MustString("rfc3339")
611632
opts.Host = s.Key("host").MustString("")
612633
opts.Port = s.Key("port").MustInt(0)
@@ -670,6 +691,13 @@ func loadConfigurationFile(path string) (options, error) {
670691
opts.AzureKey = s.Key("azure_key").MustString("")
671692
opts.AzureEndpoint = s.Key("azure_endpoint").MustString("blob.core.windows.net")
672693

694+
// Validate mode and convert to int
695+
m, err := validateMode(mode)
696+
if err != nil {
697+
return opts, err
698+
}
699+
opts.Mode = m
700+
673701
// Validate purge keep and time limit
674702
keep, err := validatePurgeKeepValue(purgeKeep)
675703
if err != nil {
@@ -819,6 +847,8 @@ func mergeCliAndConfigOptions(cliOpts options, configOpts options, onCli []strin
819847
opts.BinDirectory = cliOpts.BinDirectory
820848
case "backup-directory":
821849
opts.Directory = cliOpts.Directory
850+
case "backup-file-mode":
851+
opts.Mode = cliOpts.Mode
822852
case "exclude-dbs":
823853
opts.ExcludeDbs = cliOpts.ExcludeDbs
824854
case "include-dbs":

‎config_test.go‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,35 @@ func TestValidateDumpFormat(t *testing.T) {
6262

6363
}
6464

65+
func TestValidateMode(t *testing.T) {
66+
var tests = []struct {
67+
give string
68+
want int
69+
wantError bool
70+
}{
71+
{"0700", 448, false},
72+
{"070000", 0, true}, // invalid mode (too long)
73+
{"18446744000", 0, true}, // still invalid, positive integer
74+
{"08170", 0, true}, // non valid mode (8 on it)
75+
{"-8170", -8170, false}, // valid and mean do nothing (useful when using umask)
76+
}
77+
78+
l.logger.SetOutput(ioutil.Discard)
79+
for i, st := range tests {
80+
t.Run(fmt.Sprintf("%v", i), func(t *testing.T) {
81+
got, err := validateMode(st.give)
82+
if err == nil && st.wantError {
83+
t.Errorf("excepted an error got nil")
84+
} else if err != nil && !st.wantError {
85+
t.Errorf("did not want an error, got %s", err)
86+
}
87+
if got != st.want {
88+
t.Errorf("got %v, want %v", got, st.want)
89+
}
90+
})
91+
}
92+
}
93+
6594
func TestValidatePurgeKeepValue(t *testing.T) {
6695
var tests = []struct {
6796
give string
@@ -183,6 +212,7 @@ func TestDefaultOptions(t *testing.T) {
183212

184213
var want = options{
185214
Directory: "/var/backups/postgresql",
215+
Mode: 0o600,
186216
Format: 'c',
187217
DirJobs: 1,
188218
CompressLevel: -1,
@@ -228,6 +258,7 @@ func TestParseCli(t *testing.T) {
228258
[]string{"-b", "test", "-Z", "2", "a", "b"},
229259
options{
230260
Directory: "test",
261+
Mode: 0o600,
231262
Dbnames: []string{"a", "b"},
232263
Format: 'c',
233264
DirJobs: 1,
@@ -255,6 +286,7 @@ func TestParseCli(t *testing.T) {
255286
[]string{"-t", "--without-templates"},
256287
options{
257288
Directory: "/var/backups/postgresql",
289+
Mode: 0o600,
258290
WithTemplates: false,
259291
Format: 'c',
260292
DirJobs: 1,
@@ -306,6 +338,7 @@ func TestParseCli(t *testing.T) {
306338
[]string{"--upload", "wrong"},
307339
options{
308340
Directory: "/var/backups/postgresql",
341+
Mode: 0o600,
309342
Format: 'c',
310343
DirJobs: 1,
311344
CompressLevel: -1,
@@ -334,6 +367,7 @@ func TestParseCli(t *testing.T) {
334367
[]string{"--download", "wrong"},
335368
options{
336369
Directory: "/var/backups/postgresql",
370+
Mode: 0o600,
337371
Format: 'c',
338372
DirJobs: 1,
339373
CompressLevel: -1,
@@ -370,6 +404,7 @@ func TestParseCli(t *testing.T) {
370404
[]string{"--cipher-pass", "mypass"},
371405
options{
372406
Directory: "/var/backups/postgresql",
407+
Mode: 0o600,
373408
Format: 'c',
374409
DirJobs: 1,
375410
CompressLevel: -1,
@@ -398,6 +433,7 @@ func TestParseCli(t *testing.T) {
398433
[]string{"--cipher-private-key", "mykey"},
399434
options{
400435
Directory: "/var/backups/postgresql",
436+
Mode: 0o600,
401437
Format: 'c',
402438
DirJobs: 1,
403439
CompressLevel: -1,
@@ -426,6 +462,7 @@ func TestParseCli(t *testing.T) {
426462
[]string{"--cipher-public-key", "fakepubkey"},
427463
options{
428464
Directory: "/var/backups/postgresql",
465+
Mode: 0o600,
429466
Format: 'c',
430467
DirJobs: 1,
431468
CompressLevel: -1,
@@ -596,10 +633,11 @@ func TestLoadConfigurationFile(t *testing.T) {
596633
want options
597634
}{
598635
{
599-
[]string{"backup_directory = test", "port = 5433"},
636+
[]string{"backup_directory = test", "port = 5433", "backup_file_mode = 0700"},
600637
false,
601638
options{
602639
Directory: "test",
640+
Mode: 0o700,
603641
Port: 5433,
604642
Format: 'c',
605643
DirJobs: 1,
@@ -620,10 +658,11 @@ func TestLoadConfigurationFile(t *testing.T) {
620658
},
621659
},
622660
{ // ensure comma separated lists work
623-
[]string{"backup_directory = test", "include_dbs = a, b, postgres", "compress_level = 9"},
661+
[]string{"backup_directory = test", "include_dbs = a, b, postgres", "compress_level = 9", "backup_file_mode = 0400"},
624662
false,
625663
options{
626664
Directory: "test",
665+
Mode: 0o400,
627666
Dbnames: []string{"a", "b", "postgres"},
628667
Format: 'c',
629668
DirJobs: 1,
@@ -648,6 +687,7 @@ func TestLoadConfigurationFile(t *testing.T) {
648687
false,
649688
options{
650689
Directory: "/var/backups/postgresql",
690+
Mode: 0o600,
651691
Format: 'c',
652692
DirJobs: 1,
653693
CompressLevel: -1,
@@ -671,6 +711,7 @@ func TestLoadConfigurationFile(t *testing.T) {
671711
false,
672712
options{
673713
Directory: "/var/backups/postgresql",
714+
Mode: 0o600,
674715
Format: 'c',
675716
DirJobs: 1,
676717
CompressLevel: -1,
@@ -712,6 +753,7 @@ func TestLoadConfigurationFile(t *testing.T) {
712753
false,
713754
options{
714755
Directory: "test",
756+
Mode: 0o600,
715757
Format: 'c',
716758
DirJobs: 1,
717759
CompressLevel: -1,
@@ -756,6 +798,7 @@ func TestLoadConfigurationFile(t *testing.T) {
756798
false,
757799
options{
758800
Directory: "test",
801+
Mode: 0o600,
759802
Format: 'c',
760803
DirJobs: 1,
761804
CompressLevel: 3,
@@ -832,6 +875,7 @@ func TestMergeCliAndConfigoptions(t *testing.T) {
832875
want := options{
833876
BinDirectory: "/bin",
834877
Directory: "test",
878+
Mode: 0o600,
835879
Host: "localhost",
836880
Port: 5433,
837881
Username: "test",

‎crypto.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func ageDecryptInternal(src io.Reader, dst io.Writer, identity age.Identity) err
130130
return nil
131131
}
132132

133-
func encryptFile(path string, params encryptParams, keep bool) ([]string, error) {
133+
func encryptFile(path string, mode int, params encryptParams, keep bool) ([]string, error) {
134134
encrypted := make([]string, 0)
135135

136136
i, err := os.Stat(path)
@@ -210,7 +210,11 @@ func encryptFile(path string, params encryptParams, keep bool) ([]string, error)
210210
}
211211

212212
encrypted = append(encrypted, dstFile)
213-
213+
if mode > 0 {
214+
if err := os.Chmod(dstFile, os.FileMode(mode)); err != nil {
215+
return encrypted, fmt.Errorf("could not chmod to more secure permission for encrypted file: %w", err)
216+
}
217+
}
214218
if !keep {
215219
l.Verboseln("removing source file:", path)
216220
src.Close()

‎hash.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func computeChecksum(path string, h hash.Hash) (string, error) {
5151
return string(h.Sum(nil)), nil
5252
}
5353

54-
func checksumFile(path string, algo string) (string, error) {
54+
func checksumFile(path string, mode int, algo string) (string, error) {
5555
var h hash.Hash
5656

5757
switch algo {
@@ -114,10 +114,16 @@ func checksumFile(path string, algo string) (string, error) {
114114
r, _ := computeChecksum(path, h)
115115
fmt.Fprintf(o, "%x %s\n", r, path)
116116
}
117+
l.Verboseln("computing checksum with MODE", mode, path)
118+
if mode > 0 {
119+
if err := os.Chmod(o.Name(), os.FileMode(mode)); err != nil {
120+
return "", fmt.Errorf("could not chmod checksum file %s: %s", path, err)
121+
}
122+
}
117123
return sumFile, nil
118124
}
119125

120-
func checksumFileList(paths []string, algo string, sumFilePrefix string) (string, error) {
126+
func checksumFileList(paths []string, mode int, algo string, sumFilePrefix string) (string, error) {
121127
var h hash.Hash
122128

123129
switch algo {
@@ -157,6 +163,12 @@ func checksumFileList(paths []string, algo string, sumFilePrefix string) (string
157163
}
158164

159165
fmt.Fprintf(o, "%x *%s\n", r, path)
166+
167+
if mode > 0 {
168+
if err := os.Chmod(o.Name(), os.FileMode(mode)); err != nil {
169+
return "", fmt.Errorf("could not chmod checksum file %s: %s", path, err)
170+
}
171+
}
160172
}
161173

162174
if failed {

‎hash_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,18 @@ func TestChecksumFile(t *testing.T) {
7777
}
7878

7979
// bad algo
80-
if _, err := checksumFile("", "none"); err != nil {
80+
if _, err := checksumFile("", 0o700, "none"); err != nil {
8181
t.Errorf("expected <nil>, got %q\n", err)
8282
}
8383

84-
if _, err := checksumFile("", "other"); err == nil {
84+
if _, err := checksumFile("", 0o700, "other"); err == nil {
8585
t.Errorf("expected err, got <nil>\n")
8686
}
8787

8888
// test each algo with the file
8989
for i, st := range tests {
9090
t.Run(fmt.Sprintf("f%v", i), func(t *testing.T) {
91-
if _, err := checksumFile("test", st.algo); err != nil {
91+
if _, err := checksumFile("test", 0o700, st.algo); err != nil {
9292
t.Errorf("checksumFile returned: %v", err)
9393
}
9494

@@ -111,12 +111,12 @@ func TestChecksumFile(t *testing.T) {
111111
// bad files
112112
var e *os.PathError
113113
l.logger.SetOutput(ioutil.Discard)
114-
if _, err := checksumFile("", "sha1"); !errors.As(err, &e) {
114+
if _, err := checksumFile("", 0o700, "sha1"); !errors.As(err, &e) {
115115
t.Errorf("expected an *os.PathError, got %q\n", err)
116116
}
117117

118118
os.Chmod("test.sha1", 0444)
119-
if _, err := checksumFile("test", "sha1"); !errors.As(err, &e) {
119+
if _, err := checksumFile("test", 0o700, "sha1"); !errors.As(err, &e) {
120120
t.Errorf("expected an *os.PathError, got %q\n", err)
121121
}
122122
os.Chmod("test.sha1", 0644)
@@ -138,7 +138,7 @@ func TestChecksumFile(t *testing.T) {
138138
// test each algo with the directory
139139
for i, st := range tests {
140140
t.Run(fmt.Sprintf("d%v", i), func(t *testing.T) {
141-
if _, err := checksumFile("test.d", st.algo); err != nil {
141+
if _, err := checksumFile("test.d", 0o700, st.algo); err != nil {
142142
t.Errorf("checksumFile returned: %v", err)
143143
}
144144

0 commit comments

Comments
 (0)