From 1eafb638a80a50dde3a146d09946fabca3bd6474 Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Sun, 16 Nov 2025 11:03:12 -0800 Subject: [PATCH] cmd: finish linting fixes --- .circleci/config.yml | 2 +- backoff/backoff_test.go | 2 +- cmd/certverify/main.go | 140 ++++++++++++++++++++++++--------------- cmd/cruntar/main.go | 141 ++++++++++++++++++++++++---------------- cmd/renfnv/main.go | 69 +++++++++++--------- 5 files changed, 213 insertions(+), 141 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a7988d1..24f8e56 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -64,4 +64,4 @@ workflows: testbuild: jobs: - testbuild -# - lint + - lint diff --git a/backoff/backoff_test.go b/backoff/backoff_test.go index 1c62afc..9c093bf 100644 --- a/backoff/backoff_test.go +++ b/backoff/backoff_test.go @@ -91,7 +91,7 @@ func TestReset(t *testing.T) { } } -const decay = 5 * time.Millisecond +const decay = 25 * time.Millisecond const maxDuration = 10 * time.Millisecond const interval = time.Millisecond diff --git a/cmd/certverify/main.go b/cmd/certverify/main.go index a78cd6b..9e32e5a 100644 --- a/cmd/certverify/main.go +++ b/cmd/certverify/main.go @@ -9,7 +9,6 @@ import ( "git.wntrmute.dev/kyle/goutils/certlib" "git.wntrmute.dev/kyle/goutils/certlib/revoke" - "git.wntrmute.dev/kyle/goutils/die" "git.wntrmute.dev/kyle/goutils/lib" ) @@ -29,83 +28,116 @@ func printRevocation(cert *x509.Certificate) { } } -func main() { - var caFile, intFile string - var forceIntermediateBundle, revexp, verbose bool - flag.StringVar(&caFile, "ca", "", "CA certificate `bundle`") - flag.StringVar(&intFile, "i", "", "intermediate `bundle`") - flag.BoolVar(&forceIntermediateBundle, "f", false, +type appConfig struct { + caFile, intFile string + forceIntermediateBundle bool + revexp, verbose bool +} + +func parseFlags() appConfig { + var cfg appConfig + flag.StringVar(&cfg.caFile, "ca", "", "CA certificate `bundle`") + flag.StringVar(&cfg.intFile, "i", "", "intermediate `bundle`") + flag.BoolVar(&cfg.forceIntermediateBundle, "f", false, "force the use of the intermediate bundle, ignoring any intermediates bundled with certificate") - flag.BoolVar(&revexp, "r", false, "print revocation and expiry information") - flag.BoolVar(&verbose, "v", false, "verbose") + flag.BoolVar(&cfg.revexp, "r", false, "print revocation and expiry information") + flag.BoolVar(&cfg.verbose, "v", false, "verbose") flag.Parse() + return cfg +} - var roots *x509.CertPool - if caFile != "" { - var err error - if verbose { - fmt.Println("[+] loading root certificates from", caFile) - } - roots, err = certlib.LoadPEMCertPool(caFile) - die.If(err) +func loadRoots(caFile string, verbose bool) (*x509.CertPool, error) { + if caFile == "" { + return x509.SystemCertPool() } - var ints *x509.CertPool - if intFile != "" { - var err error - if verbose { - fmt.Println("[+] loading intermediate certificates from", intFile) - } - ints, err = certlib.LoadPEMCertPool(caFile) - die.If(err) - } else { - ints = x509.NewCertPool() - } - - if flag.NArg() != 1 { - fmt.Fprintf(os.Stderr, "Usage: %s [-ca bundle] [-i bundle] cert", - lib.ProgName()) - } - - fileData, err := os.ReadFile(flag.Arg(0)) - die.If(err) - - chain, err := certlib.ParseCertificatesPEM(fileData) - die.If(err) if verbose { - fmt.Printf("[+] %s has %d certificates\n", flag.Arg(0), len(chain)) + fmt.Println("[+] loading root certificates from", caFile) } + return certlib.LoadPEMCertPool(caFile) +} - cert := chain[0] - if len(chain) > 1 { - if !forceIntermediateBundle { - for _, intermediate := range chain[1:] { - if verbose { - fmt.Printf("[+] adding intermediate with SKI %x\n", intermediate.SubjectKeyId) - } +func loadIntermediates(intFile string, verbose bool) (*x509.CertPool, error) { + if intFile == "" { + return x509.NewCertPool(), nil + } + if verbose { + fmt.Println("[+] loading intermediate certificates from", intFile) + } + // Note: use intFile here (previously used caFile mistakenly) + return certlib.LoadPEMCertPool(intFile) +} - ints.AddCert(intermediate) - } +func addBundledIntermediates(chain []*x509.Certificate, pool *x509.CertPool, verbose bool) { + for _, intermediate := range chain[1:] { + if verbose { + fmt.Printf("[+] adding intermediate with SKI %x\n", intermediate.SubjectKeyId) } + pool.AddCert(intermediate) } +} +func verifyCert(cert *x509.Certificate, roots, ints *x509.CertPool) error { opts := x509.VerifyOptions{ Intermediates: ints, Roots: roots, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, } + _, err := cert.Verify(opts) + return err +} - _, err = cert.Verify(opts) +func run(cfg appConfig) error { + roots, err := loadRoots(cfg.caFile, cfg.verbose) if err != nil { - fmt.Fprintf(os.Stderr, "Verification failed: %v\n", err) - os.Exit(1) + return err } - if verbose { + ints, err := loadIntermediates(cfg.intFile, cfg.verbose) + if err != nil { + return err + } + + if flag.NArg() != 1 { + fmt.Fprintf(os.Stderr, "Usage: %s [-ca bundle] [-i bundle] cert", lib.ProgName()) + } + + fileData, err := os.ReadFile(flag.Arg(0)) + if err != nil { + return err + } + + chain, err := certlib.ParseCertificatesPEM(fileData) + if err != nil { + return err + } + if cfg.verbose { + fmt.Printf("[+] %s has %d certificates\n", flag.Arg(0), len(chain)) + } + + cert := chain[0] + if len(chain) > 1 && !cfg.forceIntermediateBundle { + addBundledIntermediates(chain, ints, cfg.verbose) + } + + if err = verifyCert(cert, roots, ints); err != nil { + return fmt.Errorf("certificate verification failed: %w", err) + } + + if cfg.verbose { fmt.Println("OK") } - if revexp { + if cfg.revexp { printRevocation(cert) } + return nil +} + +func main() { + cfg := parseFlags() + if err := run(cfg); err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + os.Exit(1) + } } diff --git a/cmd/cruntar/main.go b/cmd/cruntar/main.go index 931476b..d7c181b 100644 --- a/cmd/cruntar/main.go +++ b/cmd/cruntar/main.go @@ -10,6 +10,7 @@ import ( "io" "os" "path/filepath" + "strings" "git.wntrmute.dev/kyle/goutils/die" "git.wntrmute.dev/kyle/goutils/fileutil" @@ -48,7 +49,83 @@ func linkTarget(target, top string) string { return target } - return filepath.Clean(filepath.Join(target, top)) + return filepath.Clean(filepath.Join(top, target)) +} + +// safeJoin joins base and elem and ensures the resulting path does not escape base. +func safeJoin(base, elem string) (string, error) { + cleanBase := filepath.Clean(base) + joined := filepath.Clean(filepath.Join(cleanBase, elem)) + + absBase, err := filepath.Abs(cleanBase) + if err != nil { + return "", err + } + absJoined, err := filepath.Abs(joined) + if err != nil { + return "", err + } + rel, err := filepath.Rel(absBase, absJoined) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { + return "", fmt.Errorf("path traversal detected: %s escapes %s", elem, base) + } + return joined, nil +} + +func handleTypeReg(tfr *tar.Reader, hdr *tar.Header, filePath string) error { + file, err := os.Create(filePath) + if err != nil { + return err + } + defer file.Close() + + if _, err = io.Copy(file, tfr); err != nil { + return err + } + return setupFile(hdr, file) +} + +func handleTypeLink(hdr *tar.Header, top, filePath string) error { + file, err := os.Create(filePath) + if err != nil { + return err + } + defer file.Close() + + srcPath, err := safeJoin(top, hdr.Linkname) + if err != nil { + return err + } + source, err := os.Open(srcPath) + if err != nil { + return err + } + defer source.Close() + + if _, err = io.Copy(file, source); err != nil { + return err + } + return setupFile(hdr, file) +} + +func handleTypeSymlink(hdr *tar.Header, top, filePath string) error { + if !fileutil.ValidateSymlink(hdr.Linkname, top) { + return fmt.Errorf("symlink %s is outside the top-level %s", hdr.Linkname, top) + } + path := linkTarget(hdr.Linkname, top) + if ok, err := filepath.Match(top+"/*", filepath.Clean(path)); !ok { + return fmt.Errorf("symlink %s isn't in %s", hdr.Linkname, top) + } else if err != nil { + return err + } + return os.Symlink(linkTarget(hdr.Linkname, top), filePath) +} + +func handleTypeDir(hdr *tar.Header, filePath string) error { + return os.MkdirAll(filePath, os.FileMode(hdr.Mode&0xFFFFFFFF)) // #nosec G115 } func processFile(tfr *tar.Reader, hdr *tar.Header, top string) error { @@ -56,67 +133,21 @@ func processFile(tfr *tar.Reader, hdr *tar.Header, top string) error { fmt.Println(hdr.Name) } - filePath := filepath.Clean(filepath.Join(top, hdr.Name)) + filePath, err := safeJoin(top, hdr.Name) + if err != nil { + return err + } switch hdr.Typeflag { case tar.TypeReg: - file, err := os.Create(filePath) - if err != nil { - return err - } - - _, err = io.Copy(file, tfr) - if err != nil { - return err - } - - err = setupFile(hdr, file) - if err != nil { - return err - } + return handleTypeReg(tfr, hdr, filePath) case tar.TypeLink: - file, err := os.Create(filePath) - if err != nil { - return err - } - - source, err := os.Open(hdr.Linkname) - if err != nil { - return err - } - - _, err = io.Copy(file, source) - if err != nil { - return err - } - - err = setupFile(hdr, file) - if err != nil { - return err - } + return handleTypeLink(hdr, top, filePath) case tar.TypeSymlink: - if !fileutil.ValidateSymlink(hdr.Linkname, top) { - return fmt.Errorf("symlink %s is outside the top-level %s", - hdr.Linkname, top) - } - path := linkTarget(hdr.Linkname, top) - if ok, err := filepath.Match(top+"/*", filepath.Clean(path)); !ok { - return fmt.Errorf("symlink %s isn't in %s", hdr.Linkname, top) - } else if err != nil { - return err - } - - err := os.Symlink(linkTarget(hdr.Linkname, top), filePath) - if err != nil { - return err - } + return handleTypeSymlink(hdr, top, filePath) case tar.TypeDir: - err := os.MkdirAll(filePath, os.FileMode(hdr.Mode&0xFFFFFFFF)) // #nosec G115 - if err != nil { - return err - } + return handleTypeDir(hdr, filePath) } - return nil } diff --git a/cmd/renfnv/main.go b/cmd/renfnv/main.go index c9c14e6..f3c3017 100644 --- a/cmd/renfnv/main.go +++ b/cmd/renfnv/main.go @@ -96,6 +96,44 @@ func init() { flag.Usage = func() { usage(os.Stdout) } } +type options struct { + dryRun, force, printChanged, verbose bool +} + +func processOne(file string, opt options) error { + renamed, err := newName(file) + if err != nil { + _, _ = lib.Warn(err, "failed to get new file name") + return err + } + if opt.verbose && !opt.printChanged { + fmt.Fprintln(os.Stdout, file) + } + if renamed == file { + return nil + } + if !opt.dryRun { + if err = move(renamed, file, opt.force); err != nil { + _, _ = lib.Warn(err, "failed to rename file from %s to %s", file, renamed) + return err + } + } + if opt.printChanged && !opt.verbose { + fmt.Fprintln(os.Stdout, file, "->", renamed) + } + return nil +} + +func run(dryRun, force, printChanged, verbose bool, files []string) { + if verbose && printChanged { + printChanged = false + } + opt := options{dryRun: dryRun, force: force, printChanged: printChanged, verbose: verbose} + for _, file := range files { + _ = processOne(file, opt) + } +} + func main() { var dryRun, force, printChanged, verbose bool flag.BoolVar(&force, "f", false, "force overwriting of files if there is a collision") @@ -104,34 +142,5 @@ func main() { flag.BoolVar(&verbose, "v", false, "list all processed files") flag.Parse() - - if verbose && printChanged { - printChanged = false - } - - for _, file := range flag.Args() { - renamed, err := newName(file) - if err != nil { - _, _ = lib.Warn(err, "failed to get new file name") - continue - } - - if verbose && !printChanged { - fmt.Fprintln(os.Stdout, file) - } - - if renamed != file { - if !dryRun { - err = move(renamed, file, force) - if err != nil { - _, _ = lib.Warn(err, "failed to rename file from %s to %s", file, renamed) - continue - } - } - - if printChanged && !verbose { - fmt.Fprintln(os.Stdout, file, "->", renamed) - } - } - } + run(dryRun, force, printChanged, verbose, flag.Args()) }