Security patch

.github/workflows/security-audit.yml

@@ -0,0 +1,95 @@
+name: Security Audit
+
+on:
+  pull_request:
+  push:
+    branches:
+      - main
+
+permissions:
+  contents: read
+
+jobs:
+  audit:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+
+      - name: Install audit tools
+        run: |
+          go install golang.org/x/vuln/cmd/govulncheck@latest
+          go install github.com/securego/gosec/v2/cmd/gosec@latest
+          echo "${HOME}/go/bin" >> "$GITHUB_PATH"
+
+      - name: Run audit checks and collect reports
+        shell: bash
+        run: |
+          set -uo pipefail
+          mkdir -p build/audit/reports
+
+          status=0
+
+          {
+            echo '$ go test ./...'
+            GODEBUG=x509usefallbackroots=1 go test ./...
+          } 2>&1 | tee build/audit/reports/go-test.txt || status=1
+
+          {
+            echo '$ go vet ./...'
+            go vet ./...
+          } 2>&1 | tee build/audit/reports/go-vet.txt || status=1
+
+          {
+            echo '$ go build -o build/audit/smimesign .'
+            go build -o build/audit/smimesign -ldflags "-X main.versionString=${GITHUB_SHA}" .
+            echo
+            echo '$ go build -o build/audit/git-x509-cert ./cmd/git-x509-cert'
+            go build -o build/audit/git-x509-cert ./cmd/git-x509-cert
+          } 2>&1 | tee build/audit/reports/build.txt || status=1
+
+          {
+            echo '$ govulncheck -mode binary build/audit/smimesign'
+            govulncheck -mode binary build/audit/smimesign
+          } 2>&1 | tee build/audit/reports/govulncheck-smimesign.txt || status=1
+
+          {
+            echo '$ govulncheck -mode binary build/audit/git-x509-cert'
+            govulncheck -mode binary build/audit/git-x509-cert
+          } 2>&1 | tee build/audit/reports/govulncheck-git-x509-cert.txt || status=1
+
+          {
+            echo '$ gosec ./...'
+            gosec ./...
+          } 2>&1 | tee build/audit/reports/gosec.txt || status=1
+
+          {
+            echo '# Security Audit Summary'
+            echo
+            echo '- `go test`: see `go-test.txt`'
+            echo '- `go vet`: see `go-vet.txt`'
+            echo '- `govulncheck` (smimesign): see `govulncheck-smimesign.txt`'
+            echo '- `govulncheck` (git-x509-cert): see `govulncheck-git-x509-cert.txt`'
+            echo '- `gosec`: see `gosec.txt`'
+            echo
+            if [ "$status" -eq 0 ]; then
+              echo 'Result: PASS'
+            else
+              echo 'Result: FAIL'
+            fi
+          } >> "$GITHUB_STEP_SUMMARY"
+
+          exit "$status"
+
+      - name: Upload audit reports
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: security-audit-reports
+          path: build/audit/reports/
+          if-no-files-found: error

Makefile

@@ -1,4 +1,4 @@
-.PHONY := help test-min test-all build-linux build-windows build-darwin build-tools build-all
+.PHONY := help clean test-min test-all audit audit-tools build-linux build-windows build-darwin build-tools build-all
 
 # Default minimal test packages that run safely in sandboxes
 PKG_MIN := ./fakeca ./ietf-cms/timestamp
@@ -23,15 +23,27 @@ endef
 
 help:
 	@echo "Targets:"
+	@echo "  clean     - Remove generated build artifacts"
 	@echo "  test-min  - Run a minimal, sandbox-safe subset of tests"
 	@echo "  test-all  - Run all tests (may require macOS keychain access)"
+	@echo "  audit     - Run security-focused checks (go test, go vet, govulncheck, gosec)"
+	@echo "  audit-tools - Install govulncheck and gosec into GOPATH/bin"
 	@echo "  build-tools - Build helper tools such as git-x509-cert"
 	@echo ""
 	@echo "Environment:"
 	@echo "  GODEBUG   - Defaults to 'x509usefallbackroots=1' to avoid system truststore access issues"
 	@echo "  WINDOWS_CC_AMD64 / WINDOWS_CC_386 - Windows cgo cross-compilers"
 	@echo "  DARWIN_CC_AMD64 / DARWIN_CC_ARM64 - macOS cgo cross-compilers"
 
+clean:
+	rm -rf $(BUILD_DIR)/audit
+	rm -rf $(BUILD_DIR)/tools
+	rm -rf $(BUILD_DIR)/linux/amd64
+	rm -rf $(BUILD_DIR)/linux/386
+	rm -rf $(BUILD_DIR)/windows
+	rm -rf $(BUILD_DIR)/darwin
+	rm -rf $(BUILD_DIR)/amd64
+
 test-min:
 	@echo "[test-min] Running: $(PKG_MIN)"
 	GODEBUG=$(GODEBUG) go test -v $(PKG_MIN)
@@ -40,6 +52,28 @@ test-all:
 	@echo "[test-all] Running: ./... (some tests may require unsandboxed macOS keychain access)"
 	GODEBUG=$(GODEBUG) go test -v ./...
 
+audit-tools:
+	go install golang.org/x/vuln/cmd/govulncheck@latest
+	go install github.com/securego/gosec/v2/cmd/gosec@latest
+
+audit:
+	$(call require-tool,govulncheck,Run 'make audit-tools' to install govulncheck.)
+	$(call require-tool,gosec,Run 'make audit-tools' to install gosec.)
+	@echo "[audit] go test ./..."
+	GODEBUG=$(GODEBUG) go test ./...
+	@echo "[audit] go vet ./..."
+	go vet ./...
+	@echo "[audit] build binaries for binary-mode govulncheck"
+	mkdir -p $(BUILD_DIR)/audit
+	go build -o $(BUILD_DIR)/audit/smimesign -ldflags "$(LDFLAGS)" .
+	go build -o $(BUILD_DIR)/audit/git-x509-cert ./cmd/git-x509-cert
+	@echo "[audit] govulncheck -mode binary $(BUILD_DIR)/audit/smimesign"
+	govulncheck -mode binary $(BUILD_DIR)/audit/smimesign
+	@echo "[audit] govulncheck -mode binary $(BUILD_DIR)/audit/git-x509-cert"
+	govulncheck -mode binary $(BUILD_DIR)/audit/git-x509-cert
+	@echo "[audit] gosec ./..."
+	gosec ./...
+
 build-linux:
 	@echo "[build-linux] GOOS=linux GOARCH=amd64"
 	CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -o $(BUILD_DIR)/linux/amd64/smimesign -ldflags "$(LDFLAGS)" .

README.md

@@ -82,13 +82,13 @@ ambiguity still remains, set a persistent certificate fingerprint with
 Example:
 
 ```powershell
-$env:SMIMESIGN_CERT_ID = "0x0C900B6316B1708E09BF5F0695BA0CBC20DCE99F"
+$env:SMIMESIGN_CERT_ID = "0x0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF"
 ```
 
 To persist it for future PowerShell sessions:
 
 ```powershell
-setx SMIMESIGN_CERT_ID 0x0C900B6316B1708E09BF5F0695BA0CBC20DCE99F
+setx SMIMESIGN_CERT_ID 0x0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF
 ```
 
 ### Linux
@@ -159,9 +159,14 @@ signing-capable certificate automatically. To pin one explicitly, set
 `SMIMESIGN_CERT_ID`:
 
 ```bash
-export SMIMESIGN_CERT_ID=0x0C900B6316B1708E09BF5F0695BA0CBC20DCE99F
+export SMIMESIGN_CERT_ID=0x0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF
 ```
 
+`smimesign` now prints and prefers SHA-256 certificate fingerprints by
+default. Exact legacy SHA-1 fingerprints are still accepted for backwards
+compatibility, and longer suffix matching is still supported for manual
+disambiguation.
+
 #### Linux With Forwarded PKCS#11 Access
 
 Some environments do not expose the smart card directly inside the shell or
@@ -276,6 +281,34 @@ implementation does this:
 In practice, this means a certificate with `KU=contentCommitment` is preferred
 over a client-authentication certificate with `EKU=clientAuth`.
 
+During verification, the defaults are intentionally stricter:
+
+- local signing identities are not treated as trust roots unless explicitly
+  enabled with `--trust-local-certs` or `SMIMESIGN_TRUST_LOCAL_CERTS=1`
+- certificate usage is checked against a signing-oriented policy:
+  `emailProtection`, `codeSigning`, Microsoft commercial code signing, and
+  Microsoft document signing are accepted; plain client-auth certificates are
+  not
+- if your environment needs broader compatibility than that policy, relax it
+  with `--allow-any-eku` or `SMIMESIGN_ALLOW_ANY_EKU=1`
+- revocation checking can be enabled with `--revocation-check=ocsp` or
+  `SMIMESIGN_REVOCATION_CHECK=ocsp`
+- if you want revocation problems to be warnings rather than hard failures,
+  use `--revocation-check=ocsp-soft` or
+  `SMIMESIGN_REVOCATION_CHECK=ocsp-soft`
+
+For Git history inspection, `ocsp-soft` is often the better operational
+default. In many organizations, certificates are revoked when an employee
+leaves the company. That revocation is still useful information, but it should
+not make existing repository history unreadable or unusable. Otherwise, an
+ordinary shared repository would eventually become impossible to verify without
+rewriting history or splitting the project into new repositories every time a
+developer leaves. In that model:
+
+- the signature still proves who signed the commit at the time
+- revocation becomes an important warning about current trust state
+- historical code remains accessible and reviewable
+
 Recommended practice:
 
 - set `user.signingkey` to the Git email address present in the certificate
@@ -401,6 +434,13 @@ This is intentional. It distinguishes:
 - "the signature bytes are valid"
 - "this workstation trusts the issuing CA"
 
+To preserve older local-only workflows, you can opt back into trusting local
+store identities as verification anchors:
+
+```bash
+export SMIMESIGN_TRUST_LOCAL_CERTS=1
+```
+
 ### Install Your CA To Resolve Unknown Authority Warnings
 
 #### Windows
@@ -493,6 +533,30 @@ The signature is valid, but your system does not trust the issuing CA yet.
 Install the relevant CA certificate into the system trust store and rerun the
 verification command.
 
+### Verify fails revocation checking
+
+If you enable `--revocation-check=ocsp`, `smimesign` requires the signing
+certificate to expose a reachable OCSP responder and to return a `good`
+status. In restricted or air-gapped environments, leave revocation checking at
+`none`, or use `ocsp-soft` if you want revocation failures to be surfaced as
+warnings without making `git log --show-signature` fail.
+
+This distinction matters for long-lived repositories. A revoked certificate may
+mean "do not trust this identity for new signing activity" without meaning
+"historical commits signed by this person must disappear from normal use." For
+that reason, `ocsp-soft` is the recommended mode for many repository browsing
+and review workflows, while strict `ocsp` remains available for fail-closed
+policies.
+
+### Timestamping over HTTP is rejected
+
+By default `--timestamp-authority` must use `https://`. If you must use a
+plain HTTP TSA in a controlled environment, set:
+
+```bash
+export SMIMESIGN_ALLOW_HTTP_TSA=1
+```
+
 ### Git commit succeeds but `git log --show-signature` still looks wrong
 
 Check:
@@ -531,6 +595,13 @@ make build-tools
 make build-all
 ```
 
+Run the security audit flow:
+
+```bash
+make audit-tools
+make audit
+```
+
 `make build-windows` and `make build-darwin` require cgo cross-toolchains.
 The Makefile now checks explicitly for those tools and fails early with a clear
 message. Override the compiler commands if your environment uses different

certstore/certstore_linux.go

@@ -34,7 +34,7 @@ func openStore() (Store, error) {
 
 	if path := os.Getenv("SMIMESIGN_P12"); path != "" {
 		password := os.Getenv("SMIMESIGN_P12_PASSWORD")
-		data, err := os.ReadFile(path)
+		data, err := os.ReadFile(path) // #nosec G304,G703 -- path is intentionally user-configurable via SMIMESIGN_P12.
 		if err != nil {
 			return nil, fmt.Errorf("failed to read PKCS#12 file %q: %w", path, err)
 		}
@@ -161,17 +161,45 @@ func parsePKCS12(data []byte, password string) (interface{}, *x509.Certificate,
 		return key, cert, certs, nil
 	}
 
-	passin := fmt.Sprintf("pass:%s", password)
-	cmd := exec.Command("openssl", "pkcs12", "-nodes", "-passin", passin)
-	cmd.Stdin = bytes.NewReader(data)
-	out, err := cmd.Output()
+	passwordReader, passwordWriter, err := os.Pipe()
 	if err != nil {
+		return nil, nil, nil, fmt.Errorf("failed to create password pipe for PKCS#12 parsing: %w", err)
+	}
+	defer passwordReader.Close()
+
+	cmd := exec.Command("openssl", "pkcs12", "-nodes", "-passin", "fd:3")
+	cmd.Stdin = bytes.NewReader(data)
+	cmd.ExtraFiles = []*os.File{passwordReader}
+
+	var out bytes.Buffer
+	cmd.Stdout = &out
+	cmd.Stderr = new(bytes.Buffer)
+
+	if err := cmd.Start(); err != nil {
+		_ = passwordWriter.Close()
+		return nil, nil, nil, fmt.Errorf("failed to start openssl command for PKCS#12 parsing: %w", err)
+	}
+
+	if _, err := io.WriteString(passwordWriter, password+"\n"); err != nil {
+		_ = passwordWriter.Close()
+		_ = cmd.Wait()
+		return nil, nil, nil, fmt.Errorf("failed to provide password to openssl command for PKCS#12 parsing: %w", err)
+	}
+	if err := passwordWriter.Close(); err != nil {
+		_ = cmd.Wait()
+		return nil, nil, nil, fmt.Errorf("failed to close password pipe for PKCS#12 parsing: %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		if stderr, ok := cmd.Stderr.(*bytes.Buffer); ok && stderr.Len() > 0 {
+			return nil, nil, nil, fmt.Errorf("failed to execute openssl command for PKCS#12 parsing: %v: %s", err, stderr.String())
+		}
 		return nil, nil, nil, fmt.Errorf("failed to execute openssl command for PKCS#12 parsing: %w", err)
 	}
 
 	var (
 		block *pem.Block
-		rest  = out
+		rest  = out.Bytes()
 		key   interface{}
 		certs []*x509.Certificate
 	)

certstore/certstore_windows.go

@@ -543,7 +543,7 @@ func (wpk *winPrivateKey) Delete() error {
 	} else if wpk.capiProv != 0 {
 		// Delete CryptoAPI key
 		var (
-			param unsafe.Pointer
+			param []byte
 			err   error
 
 			containerName C.LPCTSTR
@@ -554,19 +554,19 @@ func (wpk *winPrivateKey) Delete() error {
 		if param, err = wpk.getProviderParam(C.PP_CONTAINER); err != nil {
 			return fmt.Errorf("failed to get container name for CryptoAPI key deletion: %w", err)
 		} else {
-			containerName = C.LPCTSTR(param)
+			containerName = C.LPCTSTR(unsafe.Pointer(&param[0]))
 		}
 
 		if param, err = wpk.getProviderParam(C.PP_NAME); err != nil {
 			return fmt.Errorf("failed to get provider name for CryptoAPI key deletion: %w", err)
 		} else {
-			providerName = C.LPCTSTR(param)
+			providerName = C.LPCTSTR(unsafe.Pointer(&param[0]))
 		}
 
 		if param, err = wpk.getProviderParam(C.PP_PROVTYPE); err != nil {
 			return fmt.Errorf("failed to get provider type for CryptoAPI key deletion: %w", err)
 		} else {
-			providerType = (*C.DWORD)(param)
+			providerType = (*C.DWORD)(unsafe.Pointer(&param[0]))
 		}
 
 		// use CRYPT_SILENT too?
@@ -582,7 +582,7 @@ func (wpk *winPrivateKey) Delete() error {
 }
 
 // getProviderParam gets a parameter about a provider.
-func (wpk *winPrivateKey) getProviderParam(param C.DWORD) (unsafe.Pointer, error) {
+func (wpk *winPrivateKey) getProviderParam(param C.DWORD) ([]byte, error) {
 	var dataLen C.DWORD
 	if ok := C.CryptGetProvParam(wpk.capiProv, param, nil, &dataLen, 0); ok == winFalse {
 		return nil, lastError("failed to get provider parameter size")
@@ -594,8 +594,7 @@ func (wpk *winPrivateKey) getProviderParam(param C.DWORD) (unsafe.Pointer, error
 		return nil, lastError("failed to get provider parameter")
 	}
 
-	// TODO leaking memory here
-	return C.CBytes(data), nil
+	return data[:dataLen], nil
 }
 
 // Close closes this winPrivateKey.