From c1c273c5a0f03f051673f73a1dd94adafb4e228f Mon Sep 17 00:00:00 2001 From: Dominik Schulz Date: Fri, 6 Jan 2023 21:54:00 +0100 Subject: [PATCH 1/2] Improve gopass audit HTML output RELEASE_NOTES=[ENHANCEMENT] Add nicer gopass audit HTML output Signed-off-by: Dominik Schulz --- internal/action/audit.go | 11 +-- internal/action/commands.go | 8 +- internal/audit/audit.go | 116 +++++++++++++++++---------- internal/audit/output.go | 143 ++++++++++++++++++++++++++++------ internal/audit/output_test.go | 48 ++++++++++++ internal/audit/report.go | 75 +++++++++--------- internal/audit/report_test.go | 20 +++++ internal/tpl/funcs.go | 19 +++++ 8 files changed, 329 insertions(+), 111 deletions(-) create mode 100644 internal/audit/output_test.go create mode 100644 internal/audit/report_test.go diff --git a/internal/action/audit.go b/internal/action/audit.go index 246c97f4d7..cc393d6c01 100644 --- a/internal/action/audit.go +++ b/internal/action/audit.go @@ -20,13 +20,8 @@ import ( func (s *Action) Audit(c *cli.Context) error { ctx := ctxutil.WithGlobalFlags(c) - expiry := c.Int("expiry") - if expiry > 0 { - out.Print(ctx, "Auditing password expiration ...") - } else { - _ = s.rem.Reset("audit") - out.Print(ctx, "Auditing passwords for common flaws ...") - } + _ = s.rem.Reset("audit") + out.Print(ctx, "Auditing passwords for common flaws ...") t, err := s.Store.Tree(ctx) if err != nil { @@ -116,5 +111,5 @@ func openReport(path string) (*os.File, error) { return os.CreateTemp("", "gopass-report") } - return os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0o600) + return os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600) } diff --git a/internal/action/commands.go b/internal/action/commands.go index 84ffaf78ef..d18b07c7cc 100644 --- a/internal/action/commands.go +++ b/internal/action/commands.go @@ -78,10 +78,6 @@ func (s *Action) GetCommands() []*cli.Command { Before: s.IsInitialized, Action: s.Audit, Flags: []cli.Flag{ - &cli.IntFlag{ - Name: "expiry", - Usage: "Age in days before a password is considered expired. Setting this will only check expiration.", - }, &cli.StringFlag{ Name: "format", Usage: "Output format. text, csv or html. Default: text", @@ -96,6 +92,10 @@ func (s *Action) GetCommands() []*cli.Command { Name: "template", Usage: "HTML template. If not set use the built-in default.", }, + &cli.BoolFlag{ + Name: "failed", + Usage: "Report only entries that failed validation. Default: false (reports all)", + }, }, }, { diff --git a/internal/audit/audit.go b/internal/audit/audit.go index 93df167da8..c95c635b1b 100644 --- a/internal/audit/audit.go +++ b/internal/audit/audit.go @@ -6,6 +6,7 @@ package audit import ( "context" "fmt" + "path" "sync" "time" @@ -31,7 +32,11 @@ type secretGetter interface { Concurrency() int } -type validator func(string, gopass.Secret) error +type validator struct { + Name string + Description string + Validate func(string, gopass.Secret) error +} // DefaultExpiration is the default expiration time for secrets. var DefaultExpiration = time.Hour * 24 * 365 @@ -53,51 +58,67 @@ func New(ctx context.Context, s secretGetter) *Auditor { cv := crunchy.NewValidator() a.v = []validator{ - func(_ string, sec gopass.Secret) error { - return cv.Check(sec.Password()) + { + Name: "crunchy", + Description: "github.com/muesli/crunchy", + Validate: func(_ string, sec gopass.Secret) error { + return cv.Check(sec.Password()) + }, }, - func(name string, sec gopass.Secret) error { - ui := make([]string, 0, len(sec.Keys())+1) - for _, k := range sec.Keys() { - pw, found := sec.Get(k) - if !found { - continue + { + Name: "zxcvbn", + Description: "github.com/nbutton23/zxcvbn-go", + Validate: func(name string, sec gopass.Secret) error { + ui := make([]string, 0, len(sec.Keys())+1) + for _, k := range sec.Keys() { + pw, found := sec.Get(k) + if !found { + continue + } + ui = append(ui, pw) + } + ui = append(ui, name) + match := zxcvbn.PasswordStrength(sec.Password(), ui) + if match.Score < 3 { + return fmt.Errorf("weak password (%d / 4)", match.Score) } - ui = append(ui, pw) - } - ui = append(ui, name) - match := zxcvbn.PasswordStrength(sec.Password(), ui) - if match.Score < 3 { - return fmt.Errorf("weak password (%d / 4)", match.Score) - } - return nil + return nil + }, }, - func(name string, sec gopass.Secret) error { - if name == sec.Password() { - return fmt.Errorf("password equals name") - } + { + Name: "equals-name", + Description: "Checks for passwords the match the secret name", + Validate: func(name string, sec gopass.Secret) error { + if name == sec.Password() || path.Base(name) == sec.Password() { + return fmt.Errorf("password equals name") + } - return nil + return nil + }, }, } if config.Bool(ctx, "audit.hibp-use-api") { - a.v = append(a.v, func(_ string, sec gopass.Secret) error { - if sec.Password() == "" { - return nil - } + a.v = append(a.v, validator{ + Name: "hibp", + Description: "Checks passwords against the HIBPv2 API. See https://haveibeenpwned.com/", + Validate: func(_ string, sec gopass.Secret) error { + if sec.Password() == "" { + return nil + } - numFound, err := api.Lookup(hashsum.SHA1Hex(sec.Password())) - if err != nil { - return fmt.Errorf("can't check HIBPv2 API: %w", err) - } + numFound, err := api.Lookup(hashsum.SHA1Hex(sec.Password())) + if err != nil { + return fmt.Errorf("can't check HIBPv2 API: %w", err) + } - if numFound > 0 { - return fmt.Errorf("password contained in at least %d public data breaches (HIBP API)", numFound) - } + if numFound > 0 { + return fmt.Errorf("password contained in at least %d public data breaches (HIBP API)", numFound) + } - return nil + return nil + }, }) } @@ -167,6 +188,7 @@ func (a *Auditor) audit(ctx context.Context, secrets <-chan string, done chan st } a.auditSecret(ctx, secret) + a.pcb() } done <- struct{}{} } @@ -177,7 +199,7 @@ func (a *Auditor) auditSecret(ctx context.Context, secret string) { // handle old passwords revs, err := a.s.ListRevisions(ctx, secret) if err != nil { - a.r.AddError(secret, err) + a.r.AddFinding(secret, "error-revisions", err.Error(), "error") } if len(revs) > 0 { a.r.SetAge(secret, time.Since(revs[0].Date)) @@ -187,7 +209,7 @@ func (a *Auditor) auditSecret(ctx context.Context, secret string) { if err != nil { debug.Log("Failed to check %s: %s", secret, err) - a.r.AddError(secret, err) + a.r.AddFinding(secret, "error-read", err.Error(), "error") if sec != nil { a.r.AddPassword(secret, sec.Password()) } @@ -212,9 +234,13 @@ func (a *Auditor) auditSecret(ctx context.Context, secret string) { go func() { defer wg.Done() - if err := v(secret, sec); err != nil { - a.r.AddWarning(secret, err.Error()) + if err := v.Validate(secret, sec); err != nil { + a.r.AddFinding(secret, v.Name, err.Error(), "warning") + + return } + + a.r.AddFinding(secret, v.Name, "ok", "none") }() } wg.Wait() @@ -240,6 +266,8 @@ func (a *Auditor) checkHIBP(ctx context.Context) error { return err } + out.Notice(ctx, "Starting HIBP check (slow) ...") + // look up all known sha1sums. The LookupBatch method will sort the // input so we don't need to. matches := scanner.LookupBatch(ctx, maps.Keys(a.r.sha1sums)) @@ -253,7 +281,17 @@ func (a *Auditor) checkHIBP(ctx context.Context) error { // add a breach warning to each of these secrets. for _, sec := range secs.Elements() { - a.r.AddWarning(sec, "Found in at least one public data breach (HIBP Dump)") + a.r.AddFinding(sec, "hibp", "Found in at least one public data breach (HIBP Dump)", "warning") + } + } + + for name, sr := range a.r.secrets { + if _, found := sr.Findings["hibp"]; !found { + sr.Findings["hibp"] = Finding{ + Severity: "none", + Message: "ok", + } + a.r.secrets[name] = sr } } diff --git a/internal/audit/output.go b/internal/audit/output.go index 261f4ac8c4..1e2c047bc5 100644 --- a/internal/audit/output.go +++ b/internal/audit/output.go @@ -6,7 +6,9 @@ import ( "fmt" "io" "io/ioutil" + "sort" "text/template" + "time" "github.com/gopasspw/gopass/internal/out" "github.com/gopasspw/gopass/internal/set" @@ -25,15 +27,12 @@ func (r *Report) PrintResults(ctx context.Context) error { for _, name := range set.SortedKeys(r.Secrets) { s := r.Secrets[name] out.Printf(ctx, "%s (age: %s)", name, s.Age.String()) - for _, e := range s.Errors { - out.Errorf(ctx, "Error: %s", e) + for k, v := range s.Findings { + if v.Severity == "error" || v.Severity == "warning" { + failed = true + } - failed = true - } - for _, w := range s.Warnings { - out.Warningf(ctx, "Warning: %s", w) - - failed = true + out.Errorf(ctx, "[%s] %s: %s", v.Severity, k, v.Message) } } @@ -47,12 +46,32 @@ func (r *Report) PrintResults(ctx context.Context) error { func (r *Report) RenderCSV(w io.Writer) error { cw := csv.NewWriter(w) + cs := set.New[string]() + for _, v := range r.Secrets { + for k := range v.Findings { + cs.Add(k) + } + } + cats := cs.Elements() + sort.Strings(cats) + for _, name := range set.SortedKeys(r.Secrets) { - if len(r.Secrets[name].Errors) < 1 && len(r.Secrets[name].Warnings) < 1 { - continue + sec := r.Secrets[name] + + rec := make([]string, 0, len(cats)+2) + rec = append(rec, name) + rec = append(rec, sec.Age.String()) + for _, cat := range cats { + if f, found := sec.Findings[cat]; found { + rec = append(rec, f.Message) + + continue + } + + rec = append(rec, "ok") } - if err := cw.Write(r.Secrets[name].Record()); err != nil { + if err := cw.Write(rec); err != nil { return err } } @@ -77,37 +96,117 @@ func (r *Report) RenderHTML(w io.Writer) error { return fmt.Errorf("failed to parse template: %w", err) } - if err := tmpl.Execute(w, r); err != nil { + if err := tmpl.Execute(w, getHTMLPayload(r)); err != nil { return fmt.Errorf("failed to execute template: %w", err) } return nil } +func getHTMLPayload(r *Report) *htmlPayload { + h := &htmlPayload{ + Today: time.Now().UTC(), + Num: len(r.Secrets), + Duration: r.Duration, + Categories: make([]string, 0, 24), + Secrets: make(map[string]SecretReport, len(r.Secrets)), + } + + cs := set.New[string]() + for _, v := range r.Secrets { + for k := range v.Findings { + cs.Add(k) + } + } + h.Categories = cs.Elements() + sort.Strings(h.Categories) + + for k, v := range r.Secrets { + sr := SecretReport{ + Name: v.Name, + Age: v.Age, + Findings: make(map[string]Finding, len(v.Findings)), + } + for _, cat := range h.Categories { + if f, found := v.Findings[cat]; found { + sr.Findings[cat] = f + + continue + } + + sr.Findings[cat] = Finding{ + Severity: "none", + Message: "ok", + } + } + h.Secrets[k] = sr + } + + return h +} + +type htmlPayload struct { + Today time.Time + Num int + Duration time.Duration + Categories []string + Secrets map[string]SecretReport +} + var htmlTpl = ` - gopass audit report + gopass audit report generated on {{ .Today | date }} + - + +Audited {{ .Num }} secrets in {{ .Duration | roundDuration }} on {{ .Today | date }}.
+ +
- - - +{{ $cats := .Categories}} +{{- range .Categories }} + +{{ end }} -{{- range .Secrets }}{{ if or .Errors .Warnings }} +{{- range .Secrets }} - - - +{{- range .Findings }} + +{{- end }} -{{ end }}{{- end }} +{{- end }}
SecretAgeErrorsWarnings{{ . }}
{{ .Name }}{{ .Age | roundDuration }}{{ .Warnings | join ", " }}{{ .Errors | join ", " }} +
{{ .Message | truncate 120 }}
+
diff --git a/internal/audit/output_test.go b/internal/audit/output_test.go new file mode 100644 index 0000000000..d479c37bdc --- /dev/null +++ b/internal/audit/output_test.go @@ -0,0 +1,48 @@ +package audit + +import ( + "bytes" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHTML(t *testing.T) { + r := newReport() + + r.AddPassword("foo", "bar") + r.SetAge("foo", time.Hour) + r.AddFinding("foo", "duplicate", "found duplicates", "warning") + r.AddFinding("foo", "hibp-api", "found match on HIBP", "warning") + + sr := r.Finalize() + out := &bytes.Buffer{} + require.NoError(t, sr.RenderHTML(out)) + assert.Equal(t, ` + + + + + + gopass audit report + + + + + + + + + + + + + + +
SecretAgeFindings
foo60mduplicate: found duplicates (warning)hibp-api: found match on HIBP (warning)
+ + +`, out.String()) +} diff --git a/internal/audit/report.go b/internal/audit/report.go index 054fe3fd4f..6db3fe72de 100644 --- a/internal/audit/report.go +++ b/internal/audit/report.go @@ -2,7 +2,6 @@ package audit import ( "fmt" - "strings" "sync" "time" @@ -10,22 +9,17 @@ import ( "github.com/gopasspw/gopass/internal/set" ) +type Finding struct { + Severity string + Message string +} + type SecretReport struct { Name string - Errors []error - Warnings []string + Findings map[string]Finding Age time.Duration } -func (s SecretReport) Record() []string { - return []string{ - s.Name, - s.Age.String(), - strings.Join(errors(s.Errors), ";"), - strings.Join(s.Warnings, ";"), - } -} - func errors(e []error) []string { s := make([]string, 0, len(e)) for _, es := range e { @@ -38,6 +32,7 @@ func errors(e []error) []string { type Report struct { Secrets map[string]SecretReport Template string + Duration time.Duration } type ReportBuilder struct { @@ -50,6 +45,8 @@ type ReportBuilder struct { // HIBP // SHA1(password) -> secret names sha1sums map[string]set.Set[string] + + t0 time.Time } func (r *ReportBuilder) AddPassword(name, pw string) { @@ -71,18 +68,24 @@ func (r *ReportBuilder) AddPassword(name, pw string) { r.sha1sums[s1] = s } -func (r *ReportBuilder) AddError(name string, e error) { - if name == "" || e == nil { +func (r *ReportBuilder) AddFinding(secret, finding, message, severity string) { + if secret == "" || finding == "" || message == "" || severity == "" { return } r.Lock() defer r.Unlock() - s := r.secrets[name] - s.Name = name - s.Errors = append(s.Errors, e) - r.secrets[name] = s + s := r.secrets[secret] + s.Name = secret + if s.Findings == nil { + s.Findings = make(map[string]Finding, 4) + } + f := s.Findings[finding] + f.Message = message + f.Severity = severity + s.Findings[finding] = f + r.secrets[secret] = s } func (r *ReportBuilder) SetAge(name string, age time.Duration) { @@ -99,28 +102,12 @@ func (r *ReportBuilder) SetAge(name string, age time.Duration) { r.secrets[name] = s } -func (r *ReportBuilder) AddWarning(name, msg string) { - if name == "" || msg == "" { - return - } - - r.Lock() - defer r.Unlock() - - s := r.secrets[name] - s.Name = name - if s.Warnings == nil { - s.Warnings = make([]string, 0, 1) - } - s.Warnings = append(s.Warnings, msg) - r.secrets[name] = s -} - func newReport() *ReportBuilder { return &ReportBuilder{ secrets: make(map[string]SecretReport, 512), duplicates: make(map[string]set.Set[string], 512), sha1sums: make(map[string]set.Set[string], 512), + t0: time.Now().UTC(), } } @@ -128,14 +115,26 @@ func newReport() *ReportBuilder { func (r *ReportBuilder) Finalize() *Report { for k, s := range r.secrets { for _, secs := range r.duplicates { - if secs.Contains(k) { - s.Warnings = append(s.Warnings, fmt.Sprintf("Duplicates detected. Shared with: %+v", secs.Difference(set.New(k)))) + if secs.Len() < 2 { + continue + } + if !secs.Contains(k) { + continue + } + if s.Findings == nil { + s.Findings = make(map[string]Finding, 1) + } + s.Findings["duplicates"] = Finding{ + Severity: "warning", + Message: fmt.Sprintf("Duplicates detected. Shared with: %+v", secs.Difference(set.New(k))), } } + r.secrets[k] = s } ret := &Report{ - Secrets: make(map[string]SecretReport, len(r.secrets)), + Secrets: make(map[string]SecretReport, len(r.secrets)), + Duration: time.Since(r.t0), } for k := range r.secrets { diff --git a/internal/audit/report_test.go b/internal/audit/report_test.go new file mode 100644 index 0000000000..948f252876 --- /dev/null +++ b/internal/audit/report_test.go @@ -0,0 +1,20 @@ +package audit + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFinalize(t *testing.T) { + r := newReport() + r.AddPassword("foo", "bar") + r.AddPassword("baz", "bar") + r.AddPassword("zab", "bar") + r.AddPassword("foo", "bar") + r.AddFinding("foo", "foo", "bar", "warning") + r.AddFinding("bar", "foo", "bar", "warning") + + sr := r.Finalize() + assert.NotNil(t, sr) +} diff --git a/internal/tpl/funcs.go b/internal/tpl/funcs.go index 5132acf631..87137641b5 100644 --- a/internal/tpl/funcs.go +++ b/internal/tpl/funcs.go @@ -39,6 +39,8 @@ const ( FuncBcrypt = "bcrypt" FuncJoin = "join" FuncRoundDuration = "roundDuration" + FuncDate = "date" + FuncTruncate = "truncate" ) func md5sum() func(...string) (string, error) { @@ -296,6 +298,19 @@ func roundDuration(duration any) string { } } +func date(ts time.Time) string { + return ts.Format("2006-01-02") +} + +func truncate(length int, v any) string { + sv := strval(v) + if len(sv) < length-3 { + return sv + } + + return sv[:length-3] + "..." +} + func join(sep string, v any) string { return strings.Join(stringslice(v), sep) } @@ -373,6 +388,8 @@ func funcMap(ctx context.Context, kv kvstore) template.FuncMap { FuncBcrypt: bcryptFunc(), FuncJoin: join, FuncRoundDuration: roundDuration, + FuncDate: date, + FuncTruncate: truncate, } } @@ -392,5 +409,7 @@ func PublicFuncMap() template.FuncMap { FuncBcrypt: bcryptFunc(), FuncJoin: join, FuncRoundDuration: roundDuration, + FuncDate: date, + FuncTruncate: truncate, } } From 3d05a27f6057ebdf7f13a87fe3a9f5bb29b32531 Mon Sep 17 00:00:00 2001 From: Dominik Schulz Date: Sat, 7 Jan 2023 07:54:11 +0100 Subject: [PATCH 2/2] Fix output test Signed-off-by: Dominik Schulz --- internal/audit/output.go | 20 +++++++-------- internal/audit/output_test.go | 47 +++++++++++++++++++++++++++++------ 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/internal/audit/output.go b/internal/audit/output.go index 1e2c047bc5..66af699fa8 100644 --- a/internal/audit/output.go +++ b/internal/audit/output.go @@ -162,19 +162,19 @@ var htmlTpl = ` gopass audit report generated on {{ .Today | date }} - + +Audited 1 secrets in 0s on 2023-01-07.
+ +
- - + + + + + - - - + +
SecretAgeFindingsduplicatehibp-api
foo60mduplicate: found duplicates (warning)hibp-api: found match on HIBP (warning) +
found duplicates
+ 		+
found match on HIBP
+