cmd: finish linting fixes

.circleci/config.yml

@@ -64,4 +64,4 @@ workflows:
   testbuild:
     jobs:
       - testbuild
-#      - lint
+      - lint

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
 

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() {
+type appConfig struct {
-	var caFile, intFile string
+	caFile, intFile         string
-	var forceIntermediateBundle, revexp, verbose bool
+	forceIntermediateBundle bool
-	flag.StringVar(&caFile, "ca", "", "CA certificate `bundle`")
+	revexp, verbose         bool
-	flag.StringVar(&intFile, "i", "", "intermediate `bundle`")
+}
-	flag.BoolVar(&forceIntermediateBundle, "f", false,
+
+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(&cfg.revexp, "r", false, "print revocation and expiry information")
-	flag.BoolVar(&verbose, "v", false, "verbose")
+	flag.BoolVar(&cfg.verbose, "v", false, "verbose")
 	flag.Parse()
+	return cfg
+}
 
-	var roots *x509.CertPool
+func loadRoots(caFile string, verbose bool) (*x509.CertPool, error) {
-	if caFile != "" {
+	if caFile == "" {
-		var err error
+		return x509.SystemCertPool()
-		if verbose {
-			fmt.Println("[+] loading root certificates from", caFile)
-		}
-		roots, err = certlib.LoadPEMCertPool(caFile)
-		die.If(err)
 	}
 
-	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]
+func loadIntermediates(intFile string, verbose bool) (*x509.CertPool, error) {
-	if len(chain) > 1 {
+	if intFile == "" {
-		if !forceIntermediateBundle {
+		return x509.NewCertPool(), nil
-			for _, intermediate := range chain[1:] {
+	}
-				if verbose {
+	if verbose {
-					fmt.Printf("[+] adding intermediate with SKI %x\n", intermediate.SubjectKeyId)
+		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)
+		return err
-		os.Exit(1)
 	}
 
-	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)
+	}
 }

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)
+		return handleTypeReg(tfr, hdr, 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
-		}
 	case tar.TypeLink:
-		file, err := os.Create(filePath)
+		return handleTypeLink(hdr, top, 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
-		}
 	case tar.TypeSymlink:
-		if !fileutil.ValidateSymlink(hdr.Linkname, top) {
+		return handleTypeSymlink(hdr, top, filePath)
-			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
-		}
 	case tar.TypeDir:
-		err := os.MkdirAll(filePath, os.FileMode(hdr.Mode&0xFFFFFFFF)) // #nosec G115
+		return handleTypeDir(hdr, filePath)
-		if err != nil {
-			return err
-		}
 	}
-
 	return nil
 }
 

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()
-
+	run(dryRun, force, printChanged, verbose, flag.Args())
-	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)
-			}
-		}
-	}
 }