diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml new file mode 100644 index 0000000..05051c2 --- /dev/null +++ b/.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 diff --git a/Makefile b/Makefile index 90c61b9..df4b486 100644 --- a/Makefile +++ b/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,8 +23,11 @@ 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:" @@ -32,6 +35,15 @@ help: @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)" . diff --git a/README.md b/README.md index 2e4a8c5..d23fe05 100644 --- a/README.md +++ b/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 diff --git a/build/linux/smimesign-amd64 b/build/linux/smimesign-amd64 deleted file mode 100755 index bbd3251..0000000 Binary files a/build/linux/smimesign-amd64 and /dev/null differ diff --git a/certstore/certstore_linux.go b/certstore/certstore_linux.go index f2dd80e..a75feca 100644 --- a/certstore/certstore_linux.go +++ b/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 ) diff --git a/certstore/certstore_windows.go b/certstore/certstore_windows.go index e5f5591..5b0065c 100644 --- a/certstore/certstore_windows.go +++ b/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(¶m[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(¶m[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(¶m[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. diff --git a/certstore/pkcs12_helper.go b/certstore/pkcs12_helper.go index 14aa974..879106d 100644 --- a/certstore/pkcs12_helper.go +++ b/certstore/pkcs12_helper.go @@ -12,7 +12,7 @@ func importPKCS12FromEnv(importFn func([]byte, string) error) error { return nil } password := os.Getenv("SMIMESIGN_P12_PASSWORD") - data, err := os.ReadFile(path) // #nosec G304 -- path is intentionally user-configurable. + data, err := os.ReadFile(path) // #nosec G304,G703 -- path is intentionally user-configurable. if err != nil { return err } diff --git a/cmd/git-x509-cert/main.go b/cmd/git-x509-cert/main.go index 0bdc204..e7ee441 100644 --- a/cmd/git-x509-cert/main.go +++ b/cmd/git-x509-cert/main.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "crypto/sha1" "crypto/sha256" "crypto/x509" "encoding/pem" @@ -93,7 +92,7 @@ func run() error { } func gitOutput(args ...string) ([]byte, error) { - cmd := exec.Command("git", args...) + cmd := exec.Command("git", args...) // #nosec G204 -- arguments are passed directly without shell interpolation. cmd.Stderr = os.Stderr out, err := cmd.Output() if err != nil { @@ -180,9 +179,9 @@ func platformCertificateDump(cert *x509.Certificate) (string, error) { var cmd *exec.Cmd switch runtime.GOOS { case "windows": - cmd = exec.Command("certutil", "-dump", tmpName) + cmd = exec.Command("certutil", "-dump", tmpName) // #nosec G204 -- static command with a temp file path. default: - cmd = exec.Command("openssl", "x509", "-in", tmpName, "-text", "-noout") + cmd = exec.Command("openssl", "x509", "-in", tmpName, "-text", "-noout") // #nosec G204 -- static command with a temp file path. } cmd.Env = os.Environ() @@ -216,7 +215,6 @@ func fallbackCertificateDump(cert *x509.Certificate) string { } fmt.Fprintf(&buf, "URIs: %s\n", strings.Join(uris, ", ")) } - fmt.Fprintf(&buf, "SHA1 Fingerprint: %X\n", sha1.Sum(cert.Raw)) fmt.Fprintf(&buf, "SHA256 Fingerprint: %X\n", sha256.Sum256(cert.Raw)) buf.WriteString("\nPEM:\n") _ = pem.Encode(&buf, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}) diff --git a/command_verify.go b/command_verify.go index f23869f..2c547c1 100644 --- a/command_verify.go +++ b/command_verify.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/certifi/gocertifi" cms "github.com/github/smimesign/ietf-cms" @@ -59,10 +60,11 @@ func verifyAttached() error { } // Verify signature - chains, err := sd.Verify(verifyOpts()) + opts, revocationMode := verifyOpts() + chains, err := sd.Verify(opts) if err != nil { if trustErr, ok := err.(x509.UnknownAuthorityError); ok { - return verifyAttachedWithUntrustedCert(sd, trustErr) + return verifyAttachedWithUntrustedCert(sd, trustErr, revocationMode) } if len(chains) > 0 { emitBadSig(chains) @@ -73,6 +75,19 @@ func verifyAttached() error { return errors.Wrap(err, "failed to verify signature") } + if err := verifyRevocation(chains, revocationMode); err != nil { + if isSoftRevocationMode(revocationMode) { + return reportTrustedButRevocationWarning(chains, err) + } + emitBadSig(chains) + return errors.Wrap(err, "failed revocation check") + } + + if err := verifySignerUsagePolicy(chains); err != nil { + emitBadSig(chains) + return errors.Wrap(err, "failed to verify signature") + } + return reportSuccessfulVerification(chains) } @@ -123,10 +138,11 @@ func verifyDetached() error { return errors.Wrap(err, "failed to read message file") } - chains, err := sd.VerifyDetached(buf.Bytes(), verifyOpts()) + opts, revocationMode := verifyOpts() + chains, err := sd.VerifyDetached(buf.Bytes(), opts) if err != nil { if trustErr, ok := err.(x509.UnknownAuthorityError); ok { - return verifyDetachedWithUntrustedCert(sd, buf.Bytes(), trustErr) + return verifyDetachedWithUntrustedCert(sd, buf.Bytes(), trustErr, revocationMode) } if len(chains) > 0 { emitBadSig(chains) @@ -137,10 +153,23 @@ func verifyDetached() error { return errors.Wrap(err, "failed to verify signature") } + if err := verifyRevocation(chains, revocationMode); err != nil { + if isSoftRevocationMode(revocationMode) { + return reportTrustedButRevocationWarning(chains, err) + } + emitBadSig(chains) + return errors.Wrap(err, "failed revocation check") + } + + if err := verifySignerUsagePolicy(chains); err != nil { + emitBadSig(chains) + return errors.Wrap(err, "failed to verify signature") + } + return reportSuccessfulVerification(chains) } -func verifyOpts() x509.VerifyOptions { +func verifyOpts() (x509.VerifyOptions, string) { roots, err := x509.SystemCertPool() if err != nil { // SystemCertPool isn't implemented for Windows. fall back to mozilla trust @@ -152,16 +181,28 @@ func verifyOpts() x509.VerifyOptions { } } - for _, ident := range idents { - if cert, err := ident.Certificate(); err == nil { - roots.AddCert(cert) + if *trustLocalCerts || envBool("SMIMESIGN_TRUST_LOCAL_CERTS") { + for _, ident := range idents { + if cert, err := ident.Certificate(); err == nil { + roots.AddCert(cert) + } } } + revocationMode := strings.TrimSpace(*revocationOpt) + if envMode := strings.TrimSpace(os.Getenv("SMIMESIGN_REVOCATION_CHECK")); envMode != "" { + revocationMode = envMode + } + switch revocationMode { + case "ocsp", "ocsp-soft": + default: + revocationMode = "none" + } + return x509.VerifyOptions{ Roots: roots, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny}, - } + }, revocationMode } func reportSuccessfulVerification(chains [][][]*x509.Certificate) error { @@ -176,24 +217,63 @@ func reportSuccessfulVerification(chains [][][]*x509.Certificate) error { return nil } -func verifyAttachedWithUntrustedCert(sd *cms.SignedData, trustErr x509.UnknownAuthorityError) error { +func verifyAttachedWithUntrustedCert(sd *cms.SignedData, trustErr x509.UnknownAuthorityError, revocationMode string) error { chains, err := sd.VerifySignatureOnly() if err != nil { sErrSig.emit() return errors.Wrap(trustErr, "failed to verify signature") } - return reportUntrustedButValidSignature(chains, trustErr) + return handleUntrustedButValidSignature(chains, trustErr, revocationMode) } -func verifyDetachedWithUntrustedCert(sd *cms.SignedData, message []byte, trustErr x509.UnknownAuthorityError) error { +func verifyDetachedWithUntrustedCert(sd *cms.SignedData, message []byte, trustErr x509.UnknownAuthorityError, revocationMode string) error { chains, err := sd.VerifyDetachedSignatureOnly(message) if err != nil { sErrSig.emit() return errors.Wrap(trustErr, "failed to verify signature") } + return handleUntrustedButValidSignature(chains, trustErr, revocationMode) +} + +func handleUntrustedButValidSignature(chains [][][]*x509.Certificate, trustErr x509.UnknownAuthorityError, revocationMode string) error { + if err := verifySignerUsagePolicy(chains); err != nil { + emitBadSig(chains) + return errors.Wrap(err, "failed to verify signature") + } + + if err := verifyRevocation(chains, revocationMode); err != nil { + if isSoftRevocationMode(revocationMode) { + return reportUntrustedButValidSignatureWithRevocationWarning(chains, trustErr, err) + } + emitBadSig(chains) + return errors.Wrap(err, "failed revocation check") + } + return reportUntrustedButValidSignature(chains, trustErr) } +func isSoftRevocationMode(mode string) bool { + return mode == "ocsp-soft" +} + +func verifySignerUsagePolicy(chains [][][]*x509.Certificate) error { + if *allowAnyEKUFlag || envBool("SMIMESIGN_ALLOW_ANY_EKU") { + return nil + } + + for _, signerChains := range chains { + if len(signerChains) == 0 || len(signerChains[0]) == 0 { + continue + } + cert := signerChains[0][0] + if !certAllowedForCommitSigning(cert) { + return fmt.Errorf("certificate specifies an incompatible key usage") + } + } + + return nil +} + func reportUntrustedButValidSignature(chains [][][]*x509.Certificate, trustErr x509.UnknownAuthorityError) error { cert := chains[0][0][0] fpr := certHexFingerprint(cert) @@ -210,3 +290,26 @@ func reportUntrustedButValidSignature(chains [][][]*x509.Certificate, trustErr x fmt.Fprintf(stderr, "smimesign: Then rerun verification.\n") return nil } + +func reportTrustedButRevocationWarning(chains [][][]*x509.Certificate, revocationErr error) error { + cert := chains[0][0][0] + fpr := certHexFingerprint(cert) + subj := cert.Subject.String() + + fmt.Fprintf(stderr, "smimesign: Signature made using certificate ID 0x%s\n", fpr) + emitGoodSig(chains) + fmt.Fprintf(stderr, "smimesign: Good signature from \"%s\"\n", subj) + emitTrustUndefined("revocation-check-failed") + fmt.Fprintf(stderr, "smimesign: WARNING: certificate chain is trusted, but revocation checking failed: %v\n", revocationErr) + fmt.Fprintf(stderr, "smimesign: Verification succeeded because revocation mode is set to warn. Use --revocation-check=ocsp to fail closed.\n") + return nil +} + +func reportUntrustedButValidSignatureWithRevocationWarning(chains [][][]*x509.Certificate, trustErr x509.UnknownAuthorityError, revocationErr error) error { + if err := reportUntrustedButValidSignature(chains, trustErr); err != nil { + return err + } + fmt.Fprintf(stderr, "smimesign: WARNING: revocation checking failed: %v\n", revocationErr) + fmt.Fprintf(stderr, "smimesign: Verification succeeded because revocation mode is set to warn. Use --revocation-check=ocsp to fail closed.\n") + return nil +} diff --git a/command_verify_test.go b/command_verify_test.go new file mode 100644 index 0000000..35ba929 --- /dev/null +++ b/command_verify_test.go @@ -0,0 +1,145 @@ +package main + +import ( + "crypto/x509" + "encoding/asn1" + "errors" + "net/http" + "strings" + "testing" + + "github.com/github/smimesign/certstore" + "github.com/github/smimesign/fakeca" +) + +func TestVerifyOptsDoesNotTrustLocalCertsByDefault(t *testing.T) { + defer testSetup(t, "--verify")() + + self := fakeca.New() + idents = []certstore.Identity{identity{self}} + + opts, mode := verifyOpts() + if mode != "none" { + t.Fatalf("expected default revocation mode none, got %q", mode) + } + + if _, err := self.Certificate.Verify(opts); err == nil { + t.Fatal("expected self-signed local certificate to remain untrusted by default") + } +} + +func TestVerifyOptsCanTrustLocalCertsExplicitly(t *testing.T) { + defer testSetup(t, "--verify")() + + self := fakeca.New() + idents = []certstore.Identity{identity{self}} + t.Setenv("SMIMESIGN_TRUST_LOCAL_CERTS", "1") + + opts, _ := verifyOpts() + if _, err := self.Certificate.Verify(opts); err != nil { + t.Fatalf("expected self-signed local certificate to verify when SMIMESIGN_TRUST_LOCAL_CERTS=1: %v", err) + } +} + +func TestVerifyOptsRestrictsKeyUsageByDefault(t *testing.T) { + defer testSetup(t, "--verify")() + + opts, _ := verifyOpts() + if len(opts.KeyUsages) != 1 || opts.KeyUsages[0] != x509.ExtKeyUsageAny { + t.Fatalf("expected chain verification to use ExtKeyUsageAny and enforce signer policy separately, got %v", opts.KeyUsages) + } +} + +func TestCertAllowedForCommitSigningAcceptsDocumentSigningOID(t *testing.T) { + cert := fakeca.New().Certificate + cert.UnknownExtKeyUsage = []asn1.ObjectIdentifier{oidMSDocumentSigning} + if !certAllowedForCommitSigning(cert) { + t.Fatal("expected Microsoft document-signing OID to be accepted for commit signing") + } +} + +func TestCertAllowedForCommitSigningRejectsClientAuthOnly(t *testing.T) { + cert := fakeca.New().Certificate + cert.ExtKeyUsage = []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth} + if certAllowedForCommitSigning(cert) { + t.Fatal("expected client-auth-only certificate to be rejected for commit signing") + } +} + +func TestVerifyOptsCanAllowAnyEKUExplicitly(t *testing.T) { + defer testSetup(t, "--verify")() + + t.Setenv("SMIMESIGN_ALLOW_ANY_EKU", "1") + opts, _ := verifyOpts() + if len(opts.KeyUsages) != 1 || opts.KeyUsages[0] != x509.ExtKeyUsageAny { + t.Fatalf("expected ExtKeyUsageAny when explicitly enabled, got %v", opts.KeyUsages) + } +} + +func TestVerifyOptsCanEnableOCSPRevocation(t *testing.T) { + defer testSetup(t, "--verify")() + + t.Setenv("SMIMESIGN_REVOCATION_CHECK", "ocsp") + _, mode := verifyOpts() + if mode != "ocsp" { + t.Fatalf("expected ocsp revocation mode, got %q", mode) + } +} + +func TestVerifyOptsCanEnableSoftOCSPRevocation(t *testing.T) { + defer testSetup(t, "--verify")() + + t.Setenv("SMIMESIGN_REVOCATION_CHECK", "ocsp-soft") + _, mode := verifyOpts() + if mode != "ocsp-soft" { + t.Fatalf("expected ocsp-soft revocation mode, got %q", mode) + } +} + +type failingRevocationHTTPClient struct{} + +func (failingRevocationHTTPClient) Do(*http.Request) (*http.Response, error) { + return nil, errors.New("ocsp fetch failed") +} + +func TestHandleUntrustedButValidSignatureEnforcesOCSP(t *testing.T) { + defer testSetup(t, "--verify")() + + prevClient := revocationHTTPClient + revocationHTTPClient = failingRevocationHTTPClient{} + defer func() { revocationHTTPClient = prevClient }() + + root := fakeca.New(fakeca.IsCA) + leaf := root.Issue(fakeca.OCSPServer("https://ocsp.example.invalid")) + chains := [][][]*x509.Certificate{{{leaf.Certificate, root.Certificate}}} + trustErr := x509.UnknownAuthorityError{Cert: leaf.Certificate} + + err := handleUntrustedButValidSignature(chains, trustErr, "ocsp") + if err == nil { + t.Fatal("expected revocation error for unknown-authority signature when ocsp is enabled") + } + if !strings.Contains(err.Error(), "failed revocation check") { + t.Fatalf("expected revocation failure, got %v", err) + } +} + +func TestHandleUntrustedButValidSignatureWarnsOnSoftOCSP(t *testing.T) { + defer testSetup(t, "--verify")() + + prevClient := revocationHTTPClient + revocationHTTPClient = failingRevocationHTTPClient{} + defer func() { revocationHTTPClient = prevClient }() + + root := fakeca.New(fakeca.IsCA) + leaf := root.Issue(fakeca.OCSPServer("https://ocsp.example.invalid")) + chains := [][][]*x509.Certificate{{{leaf.Certificate, root.Certificate}}} + trustErr := x509.UnknownAuthorityError{Cert: leaf.Certificate} + + err := handleUntrustedButValidSignature(chains, trustErr, "ocsp-soft") + if err != nil { + t.Fatalf("expected soft OCSP mode to warn but succeed, got %v", err) + } + if !strings.Contains(stderrBuf.String(), "Verification succeeded because revocation mode is set to warn") { + t.Fatalf("expected soft OCSP warning, got %q", stderrBuf.String()) + } +} diff --git a/go.mod b/go.mod index 5496642..6f75324 100644 --- a/go.mod +++ b/go.mod @@ -2,20 +2,20 @@ module github.com/github/smimesign go 1.25.0 -toolchain go1.25.7 +toolchain go1.25.8 require ( - github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261 + github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d + github.com/miekg/pkcs11 v1.1.1 github.com/pborman/getopt v0.0.0-20180811024354-2b5b3bfb099b github.com/pkg/errors v0.8.1 github.com/stretchr/testify v1.3.0 golang.org/x/crypto v0.35.0 golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da + software.sslmate.com/src/go-pkcs12 v0.7.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect - github.com/miekg/pkcs11 v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - software.sslmate.com/src/go-pkcs12 v0.7.0 // indirect ) diff --git a/go.sum b/go.sum index d9199b8..c6162b4 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261 h1:6/yVvBsKeAw05IUj4AzvrxaCnDjN4nUqKjW9+w5wixg= -github.com/certifi/gocertifi v0.0.0-20180118203423-deb3ae2ef261/go.mod h1:GJKEexRPVJrBSOjoqN5VNOIKJ5Q3RViH6eu3puDRwx4= +github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d h1:S2NE3iHSwP0XV47EEXL8mWmRdEfGscSJ+7EgePNgt0s= +github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d/go.mod h1:sGbDF6GwGcLpkNXPUTkMRoywsNa/ol15pxFe6ERfguA= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/ietf-cms/protocol/ber.go b/ietf-cms/protocol/ber.go index 54493df..0d65287 100644 --- a/ietf-cms/protocol/ber.go +++ b/ietf-cms/protocol/ber.go @@ -2,6 +2,7 @@ package protocol import ( "bytes" + "errors" ) var encodeIndent = 0 @@ -89,7 +90,8 @@ func marshalLongLength(out *bytes.Buffer, i int) (err error) { for ; n > 0; n-- { shift := (n - 1) * 8 - err = out.WriteByte(byte(i >> shift)) + b := (i >> shift) & 0xff + err = out.WriteByte(uint8(b)) if err != nil { return } @@ -125,7 +127,10 @@ func lengthLength(i int) (numBytes uint) { func encodeLength(out *bytes.Buffer, length int) (err error) { if length >= 128 { l := lengthLength(length) - err = out.WriteByte(0x80 | byte(l)) + if l > 0x7f { + return errors.New("length-of-length exceeds DER encoding limit") + } + err = out.WriteByte(0x80 | uint8(l)) if err != nil { return } @@ -134,7 +139,10 @@ func encodeLength(out *bytes.Buffer, length int) (err error) { return } } else { - err = out.WriteByte(byte(length)) + if length < 0 || length > 127 { + return errors.New("short DER length out of range") + } + err = out.WriteByte(uint8(length)) // #nosec G115 -- guarded above to fit in one byte. if err != nil { return } diff --git a/ietf-cms/timestamp/timestamp.go b/ietf-cms/timestamp/timestamp.go index 656b315..844362b 100644 --- a/ietf-cms/timestamp/timestamp.go +++ b/ietf-cms/timestamp/timestamp.go @@ -10,6 +10,8 @@ import ( "io" "math/big" "net/http" + "net/url" + "os" "strings" "time" @@ -87,7 +89,7 @@ func (req Request) Matches(tsti Info) bool { // Do sends this timestamp request to the specified timestamp service, returning // the parsed response. The timestamp.HTTPClient is used to make the request and // HTTP behavior can be modified by changing that variable. -func (req Request) Do(url string) (Response, error) { +func (req Request) Do(tsaURL string) (Response, error) { var nilResp Response reqDER, err := asn1.Marshal(req) @@ -95,7 +97,15 @@ func (req Request) Do(url string) (Response, error) { return nilResp, err } - httpReq, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(reqDER)) + parsedURL, err := url.Parse(tsaURL) + if err != nil { + return nilResp, err + } + if parsedURL.Scheme != "https" && !(parsedURL.Scheme == "http" && os.Getenv("SMIMESIGN_ALLOW_HTTP_TSA") == "1") { + return nilResp, fmt.Errorf("timestamp authority URL must use HTTPS (or set SMIMESIGN_ALLOW_HTTP_TSA=1 for HTTP)") + } + + httpReq, err := http.NewRequest(http.MethodPost, tsaURL, bytes.NewReader(reqDER)) if err != nil { return nilResp, err } diff --git a/ietf-cms/timestamp/timestamp_test.go b/ietf-cms/timestamp/timestamp_test.go index c1be104..248cf94 100644 --- a/ietf-cms/timestamp/timestamp_test.go +++ b/ietf-cms/timestamp/timestamp_test.go @@ -73,6 +73,47 @@ func TestRequestDo(t *testing.T) { } } +func TestRequestDoRejectsNonHTTPSByDefault(t *testing.T) { + DefaultHTTPClient = testHTTPClient{} + lastRequest = nil + + req := Request{Version: 1} + var err error + req.CertReq = true + req.Nonce = GenerateNonce() + if req.MessageImprint, err = NewMessageImprint(crypto.SHA256, bytes.NewReader([]byte("hello"))); err != nil { + t.Fatal(err) + } + + if _, err := req.Do("http://example.com"); err == nil || !strings.Contains(err.Error(), "must use HTTPS") { + t.Fatalf("expected HTTPS validation error, got %v", err) + } + if lastRequest != nil { + t.Fatal("expected no outbound request when TSA URL scheme is rejected") + } +} + +func TestRequestDoAllowsHTTPWhenExplicitlyEnabled(t *testing.T) { + DefaultHTTPClient = testHTTPClient{} + lastRequest = nil + t.Setenv("SMIMESIGN_ALLOW_HTTP_TSA", "1") + + req := Request{Version: 1} + var err error + req.CertReq = true + req.Nonce = GenerateNonce() + if req.MessageImprint, err = NewMessageImprint(crypto.SHA256, bytes.NewReader([]byte("hello"))); err != nil { + t.Fatal(err) + } + + if _, err := req.Do("http://example.com"); err != errFakeClient { + t.Fatalf("expected errFakeClient after allowing HTTP TSA, got %v", err) + } + if lastRequest == nil { + t.Fatal("expected outbound request after allowing HTTP TSA") + } +} + func TestRequestMatches(t *testing.T) { var err error diff --git a/main.go b/main.go index 04cf9a2..01e90be 100644 --- a/main.go +++ b/main.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/github/smimesign/certstore" "github.com/pborman/getopt/v2" @@ -39,6 +40,9 @@ var ( keyFormatOpt = getopt.EnumLong("keyid-format", 0, []string{"long"}, "long", "select how to display key IDs.", "{long}") tsaOpt = getopt.StringLong("timestamp-authority", 't', defaultTSA, "URL of RFC3161 timestamp authority to use for timestamping", "url") includeCertsOpt = getopt.IntLong("include-certs", 0, -2, "-3 is the same as -2, but ommits issuer when cert has Authority Information Access extension. -2 includes all certs except root. -1 includes all certs. 0 includes no certs. 1 includes leaf cert. >1 includes n from the leaf. Default -2.", "n") + allowAnyEKUFlag = getopt.BoolLong("allow-any-eku", 0, "accept any extended key usage during verification") + trustLocalCerts = getopt.BoolLong("trust-local-certs", 0, "treat local identities as trust anchors during verification") + revocationOpt = getopt.EnumLong("revocation-check", 0, []string{"none", "ocsp", "ocsp-soft"}, "none", "perform revocation checking during verification", "{none|ocsp|ocsp-soft}") // Remaining arguments fileArgs []string @@ -172,6 +176,20 @@ func runCommand() error { return errors.New("specify --help, --sign, --verify, --dump-certs, or --list-keys") } +func envBool(name string) bool { + value, ok := os.LookupEnv(name) + if !ok { + return false + } + + switch strings.ToLower(strings.TrimSpace(value)) { + case "1", "true", "yes", "on": + return true + default: + return false + } +} + func openIdentities() (func(), error) { store, err := certstore.Open() if err != nil { diff --git a/revocation.go b/revocation.go new file mode 100644 index 0000000..1fe12a3 --- /dev/null +++ b/revocation.go @@ -0,0 +1,105 @@ +package main + +import ( + "bytes" + "crypto/x509" + "fmt" + "io" + "net/http" + "net/url" + "time" + + "golang.org/x/crypto/ocsp" +) + +const maxOCSPResponseBytes = 1 << 20 + +type ocspHTTPClient interface { + Do(*http.Request) (*http.Response, error) +} + +var revocationHTTPClient ocspHTTPClient = &http.Client{Timeout: 15 * time.Second} + +func verifyRevocation(chains [][][]*x509.Certificate, mode string) error { + if mode != "ocsp" && mode != "ocsp-soft" { + return nil + } + + for _, signerChains := range chains { + if len(signerChains) == 0 { + continue + } + if err := verifyChainOCSP(signerChains[0]); err != nil { + return err + } + } + + return nil +} + +func verifyChainOCSP(chain []*x509.Certificate) error { + if len(chain) < 2 { + return fmt.Errorf("ocsp revocation checking requires signer and issuer certificates") + } + + cert := chain[0] + issuer := chain[1] + + if len(cert.OCSPServer) == 0 { + return fmt.Errorf("certificate %q does not advertise an OCSP responder", cert.Subject.String()) + } + + responderURL, err := url.Parse(cert.OCSPServer[0]) + if err != nil { + return fmt.Errorf("invalid OCSP responder URL %q: %w", cert.OCSPServer[0], err) + } + if responderURL.Scheme != "http" && responderURL.Scheme != "https" { + return fmt.Errorf("unsupported OCSP responder URL scheme %q", responderURL.Scheme) + } + + reqDER, err := ocsp.CreateRequest(cert, issuer, nil) + if err != nil { + return fmt.Errorf("failed to build OCSP request: %w", err) + } + + httpReq, err := http.NewRequest(http.MethodPost, responderURL.String(), bytes.NewReader(reqDER)) + if err != nil { + return fmt.Errorf("failed to build OCSP HTTP request: %w", err) + } + httpReq.Header.Set("Content-Type", "application/ocsp-request") + httpReq.Header.Set("Accept", "application/ocsp-response") + + resp, err := revocationHTTPClient.Do(httpReq) + if err != nil { + return fmt.Errorf("failed to fetch OCSP response: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("OCSP responder returned HTTP %d", resp.StatusCode) + } + + body, err := io.ReadAll(io.LimitReader(resp.Body, maxOCSPResponseBytes+1)) + if err != nil { + return fmt.Errorf("failed to read OCSP response: %w", err) + } + if len(body) > maxOCSPResponseBytes { + return fmt.Errorf("OCSP response exceeded limit of %d bytes", maxOCSPResponseBytes) + } + + ocspResp, err := ocsp.ParseResponseForCert(body, cert, issuer) + if err != nil { + return fmt.Errorf("failed to parse OCSP response: %w", err) + } + + switch ocspResp.Status { + case ocsp.Good: + return nil + case ocsp.Revoked: + return fmt.Errorf("certificate %q was revoked at %s", cert.Subject.String(), ocspResp.RevokedAt.UTC().Format(time.RFC3339)) + case ocsp.Unknown: + return fmt.Errorf("OCSP responder returned unknown status for certificate %q", cert.Subject.String()) + default: + return fmt.Errorf("OCSP responder returned status %d for certificate %q", ocspResp.Status, cert.Subject.String()) + } +} diff --git a/utils.go b/utils.go index 58a27ca..f40f10c 100644 --- a/utils.go +++ b/utils.go @@ -2,7 +2,8 @@ package main import ( "bytes" - "crypto/sha1" // #nosec G505 -- required for X.509 SHA1 fingerprints. + "crypto/sha1" // #nosec G505 -- retained for legacy fingerprint compatibility. + "crypto/sha256" "crypto/x509" "encoding/asn1" "encoding/hex" @@ -40,21 +41,45 @@ func certHasFingerprint(cert *x509.Certificate, fpr []byte) bool { return false } - return bytes.HasSuffix(certFingerprint(cert), fpr) + sha256Fpr := certFingerprint(cert) + sha1Fpr := certLegacyFingerprint(cert) + + switch len(fpr) { + case len(sha256Fpr): + return bytes.Equal(sha256Fpr, fpr) + case len(sha1Fpr): + return bytes.Equal(sha1Fpr, fpr) + } + + if len(fpr) < 8 { + return false + } + + return bytes.HasSuffix(sha256Fpr, fpr) || bytes.HasSuffix(sha1Fpr, fpr) } -// certHexFingerprint calculated the hex SHA1 fingerprint of a certificate. +// certHexFingerprint calculates the default hex SHA256 fingerprint of a certificate. func certHexFingerprint(cert *x509.Certificate) string { return hex.EncodeToString(certFingerprint(cert)) } -// certFingerprint calculated the SHA1 fingerprint of a certificate. +// certFingerprint calculates the default SHA256 fingerprint of a certificate. func certFingerprint(cert *x509.Certificate) []byte { if len(cert.Raw) == 0 { return nil } - fpr := sha1.Sum(cert.Raw) // #nosec G401 -- SHA1 is used for legacy cert fingerprints. + fpr := sha256.Sum256(cert.Raw) + return fpr[:] +} + +// certLegacyFingerprint calculates the legacy SHA1 fingerprint of a certificate. +func certLegacyFingerprint(cert *x509.Certificate) []byte { + if len(cert.Raw) == 0 { + return nil + } + + fpr := sha1.Sum(cert.Raw) // #nosec G401 -- SHA1 is retained for legacy fingerprint matching. return fpr[:] } @@ -74,8 +99,9 @@ func normalizeEmail(email string) string { } var ( - oidEmailAddress = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 1} - oidCommonName = asn1.ObjectIdentifier{2, 5, 4, 3} + oidEmailAddress = asn1.ObjectIdentifier{1, 2, 840, 113549, 1, 9, 1} + oidCommonName = asn1.ObjectIdentifier{2, 5, 4, 3} + oidMSDocumentSigning = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 311, 10, 3, 12} ) // certHasEmail checks if a certificate contains the given email address in its @@ -112,3 +138,28 @@ func certEmails(cert *x509.Certificate) []string { return emails } + +func certAllowedForCommitSigning(cert *x509.Certificate) bool { + if cert == nil { + return false + } + + if len(cert.ExtKeyUsage) == 0 && len(cert.UnknownExtKeyUsage) == 0 { + return true + } + + for _, usage := range cert.ExtKeyUsage { + switch usage { + case x509.ExtKeyUsageCodeSigning, x509.ExtKeyUsageEmailProtection, x509.ExtKeyUsageMicrosoftCommercialCodeSigning: + return true + } + } + + for _, usage := range cert.UnknownExtKeyUsage { + if usage.Equal(oidMSDocumentSigning) { + return true + } + } + + return false +} diff --git a/utils_test.go b/utils_test.go index d4e0c3e..f0a4a3f 100644 --- a/utils_test.go +++ b/utils_test.go @@ -1,30 +1,52 @@ package main import ( - "encoding/hex" - "strings" - "testing" + "encoding/hex" + "strings" + "testing" - "github.com/github/smimesign/fakeca" + "github.com/github/smimesign/fakeca" ) func TestNormalizeFingerprintAndMatch(t *testing.T) { - ca := fakeca.New(fakeca.IsCA) - cert := ca.Certificate + ca := fakeca.New(fakeca.IsCA) + cert := ca.Certificate - hexFpr := certHexFingerprint(cert) + hexFpr := certHexFingerprint(cert) - // With 0x prefix and upper-case input. - in := "0x" + strings.ToUpper(hexFpr) - norm := normalizeFingerprint(in) + // With 0x prefix and upper-case input. + in := "0x" + strings.ToUpper(hexFpr) + norm := normalizeFingerprint(in) - // Round-trip back to hex for comparison. - got := hex.EncodeToString(norm) - if !strings.EqualFold(got, hexFpr) { - t.Fatalf("normalizeFingerprint failed: have %s want %s", got, hexFpr) - } + // Round-trip back to hex for comparison. + got := hex.EncodeToString(norm) + if !strings.EqualFold(got, hexFpr) { + t.Fatalf("normalizeFingerprint failed: have %s want %s", got, hexFpr) + } - if !certHasFingerprint(cert, norm) { - t.Fatalf("certHasFingerprint returned false for valid fingerprint") - } + if !certHasFingerprint(cert, norm) { + t.Fatalf("certHasFingerprint returned false for valid SHA-256 fingerprint") + } + + legacyHexFpr := hex.EncodeToString(certLegacyFingerprint(cert)) + legacyNorm := normalizeFingerprint(legacyHexFpr) + if !certHasFingerprint(cert, legacyNorm) { + t.Fatalf("certHasFingerprint returned false for valid legacy SHA-1 fingerprint") + } +} + +func TestCertHasFingerprintRejectsShortSuffix(t *testing.T) { + cert := fakeca.New(fakeca.IsCA).Certificate + shortSuffix := certFingerprint(cert)[len(certFingerprint(cert))-4:] + if certHasFingerprint(cert, shortSuffix) { + t.Fatalf("certHasFingerprint unexpectedly matched a short suffix") + } +} + +func TestCertHasFingerprintAllowsLongSuffix(t *testing.T) { + cert := fakeca.New(fakeca.IsCA).Certificate + longSuffix := certFingerprint(cert)[len(certFingerprint(cert))-8:] + if !certHasFingerprint(cert, longSuffix) { + t.Fatalf("certHasFingerprint failed to match a sufficiently long suffix") + } }