diff --git a/.gitignore b/.gitignore index 9401f81..f2048d7 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,13 @@ build/ .env.local config.local.yaml +# Credentials and key material +secrets.json +*.pem +*.key +*.p12 +*.pfx + # AI assistant files CLAUDE.md .cursorrules diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..2ad6f35 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,106 @@ +# Changelog + +All notable changes to CryptoScan are recorded here. This project follows +[Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +### Fixed + +- **Operational crypto was silently dropped by the noise filter.** The scanner + decided whether a match was low-value by testing the whole source line + against a list of about a hundred substrings, and dropped the finding on any + hit. Three of those entries matched ordinary code: + - `".md"`, intended for Markdown filenames, matched every `.md5(` in the + corpus. `a.md5(b)`, `hashlib.md5(b"a")` and + `return hashlib.md5(data).hexdigest()` all reported zero findings. + - `"cipher ="` matched `Cipher cipher = Cipher.getInstance("DES")`, the + conventional way to name a `javax.crypto.Cipher`. + - `"const "` matched `const cipher = crypto.createCipheriv(...)`, which + `prefer-const` makes the idiomatic form in JavaScript. + + The heuristic has been replaced by an evidence classifier anchored to the + position of the match. The classifier asks whether the token is part of an + operation *before* it applies any noise heuristic, so a call, a member + access, an import or an argument to a crypto factory is always reported, + whatever else appears on the line. + + That ordering is the safety property, and getting it wrong is subtle. An + intermediate version of this change ran the log, comment and path rules + first, which reintroduced the same defect class through a different door: a + variable named `outputs` (containing `puts `) hid + `Cipher.getInstance("RC4")`, a `z--` decrement hid a DES cipher in Java, + `gpg --cipher-algo 3DES` read as a comment, and adding a fifth entry to an + `sshd_config` `MACs` line took the file from three findings to zero. All of + those are now regression tests. + +- **No rule classifies by vocabulary any more.** Two heuristics judged a match + by the words around it, and both were false-negative generators that took + three revisions to stop tuning and simply remove: + + - A quoted string of three or more words counted as prose. A quoted string is + where configuration lives, so this hid + `db.Exec("ALTER SYSTEM SET password_encryption TO 'md5'")`, + `props.put("jdk.tls.client.cipherSuites", "RC4 and DES ...")`, + `conf.set("hive.ssl.ciphers", "DES-CBC3-SHA and RC4-MD5 for all peers")` + and nine other real settings. + - A leading `description:` or `title:` owned the rest of its line, losing a + CRITICAL finding to every key that was not on the list: `note:`, then + `"title" :` with a space, then `:title =>`, then `ssl-title:`. + + What remains is structural. A match is withheld only when it belongs to a + logging or error call, sits in a comment in a language that has that comment + marker, or is inside a URL or document path. Suppressing a description field + now needs no rule at all, because it is simply reported. + +- **Declared cryptographic configuration is reported, whatever the key is + called.** `cipher: DES-CBC`, `note: DES-CBC` and `usage: DES-CBC` are the + same finding. An intermediate version treated `note`, `remarks`, `comment`, + `help`, `usage` and `example` as documentation labels, which meant renaming + a config key by one word hid a CRITICAL DES finding and flipped + `--fail-on critical` from exit 1 to exit 0. Also `cipher: DES-CBC`, + `MACs hmac-md5 ...`, `SSLProtocol +SSLv3` and `ssh-keygen -t rsa -b 1024` + are findings. For a cryptographic inventory a declared algorithm *is* the + inventory, and an Apache config enabling SSLv3 must fail + `--fail-on critical`. Scanning a repository that ships cryptographic + reference data will now report a finding per row; use `--exclude`. + +- **Suppression is counted and recoverable.** Findings held back as prose, log + output, comments, URLs or documentation labels are reported as a count in the + scan summary, and `--include-narrative` (also covered by `--verbose`) shows + them. The count is computed after deduplication, so it is exactly the number + of findings the flag adds, and the default report is a strict subset of what + the flag shows: deduplication now prefers an operational match over a + narrative one at the same location, so enabling the flag can only add. + Detected key material is never held back, since a private key in a comment is + still an exposed private key. + +- **`cryptoscan:ignore` no longer blinds the following line.** A trailing + directive suppressed both its own line and the next one. A directive now + applies to its own line unless it says `cryptoscan:ignore-next-line`. + +- **Go DES usage was undetectable.** `des.NewCipher` and + `des.NewTripleDESCipher` are the Go standard library's only DES constructors + and no pattern matched them. + +- **Every output surface reported a different version.** One binary reported + `1.4.0` from the `version` command, `1.0.0` in SARIF, `1.1.0` in CBOM, and no + version at all in JSON. A CBOM naming the wrong producer is a false + provenance record. All surfaces now read `pkg/version`, and JSON carries a + `tool` object. + +- **Finding order was nondeterministic.** Results were built by ranging over a + map, so two scans of an unchanged tree emitted the same findings in a + different order. This broke diffable CI output, golden-file tests and + reproducible CBOMs. + +### Added + +- `--include-narrative` flag to show algorithm mentions held back as text. +- `tool` object in JSON output, carrying the scanner name and version. +- `narrativeSuppressedCount` in the scan summary. + +## [1.3.0] + +See the [release notes](https://github.com/csnp/cryptoscan/releases) for +versions 1.3.0 and earlier. diff --git a/README.md b/README.md index 74e9718..b8dfbb1 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,10 @@ Flags: -c, --context int Lines of source context to show (default 3) -p, --progress Show scan progress indicator --min-severity string Minimum severity to report: info, low, medium, high, critical + --include-imports Include library import findings + --include-quantum-safe Include quantum-safe algorithm findings (SHA-256, AES-256) + --include-narrative Include algorithms named in prose, logs, docs and config keys + -v, --verbose Show all findings, including all three categories above --no-color Disable colored output --pretty Pretty print JSON output -h, --help Show help @@ -332,6 +336,44 @@ cryptoscan scan . --exclude "vendor/*,node_modules/*,*_test.go" cryptoscan scan . --min-severity critical --format json | jq '.findings | length' ``` +### What is reported, and what is held back + +CryptoScan asks one question first: is the matched token part of an operation? +A call, an instantiation, a member access, an import, or an algorithm name +passed to a crypto factory is always reported, whatever else appears on the +line. So all of these produce findings: + +``` +hashlib.md5(data) +des.NewCipher(key) +const cipher = crypto.createCipheriv('des-ede3-cbc', k, iv) +Cipher.getInstance("RC4") +``` + +Declared configuration is also reported. For a cryptographic inventory, an +algorithm named in a config file is the inventory: + +``` +cipher: DES-CBC +MACs hmac-md5 hmac-sha1 umac-64 +SSLProtocol -all +SSLv3 +TLSv1 +ssh-keygen -t rsa -b 1024 -f id_rsa +``` + +Only mentions in *text* are held back: prose, log and error messages, comments, +URLs, and the values of documentation labels such as `description:` and +`title:`. The scan summary always says how many were held back, and the number +is exactly what the flag returns: + +``` + 14 finding(s) withheld as documentation, log or comment text. + Show them with: cryptoscan scan . --include-narrative +``` + +Scanning a repository that ships cryptographic reference data (a database of +packages and the algorithms they use, for example) will report a finding per +row. Use `--exclude` to skip those paths. + ### Suppressing False Positives Use inline comments to suppress findings that are intentional or not applicable: @@ -352,7 +394,7 @@ legacyKey := oldCrypto.NewKey() ``` Supported directives: -- `cryptoscan:ignore` — Ignore all findings on this line +- `cryptoscan:ignore` — Ignore all findings on this line, and only this line - `cryptoscan:ignore RSA-001` — Ignore specific pattern ID - `cryptoscan:ignore RSA-*` — Ignore pattern family (wildcard) - `cryptoscan:ignore-next-line` — Ignore finding on the following line diff --git a/docs/testing/release-smoke.md b/docs/testing/release-smoke.md index e8a722c..f15e6cf 100644 --- a/docs/testing/release-smoke.md +++ b/docs/testing/release-smoke.md @@ -1,54 +1,121 @@ -# Release Smoke Test: cryptoscan - -Manual pre-release walkthrough. Run this before every deploy or publish. -Use real keyboard and mouse input, not synthetic events. Record the actual -output, not a summary. - -## 1. Build and start clean - -- [ ] Fresh install succeeds (`npm ci` or equivalent) with no errors. -- [ ] Build succeeds (`npm run build`). -- [ ] App starts locally (`npm run dev`) with no console errors. - -## 2. Core user paths - -List the three most important things a real visitor does on this site or -with this tool, then walk each one by hand. - -- [ ] Path 1: -- [ ] Path 2: -- [ ] Path 3: - -## 3. Accessibility and audience fit - -CSNP serves non-technical and vulnerable users. Verify the experience holds -for them. - -- [ ] Plain language: no unexplained jargon in user-facing copy. -- [ ] Keyboard navigable: every interactive element reachable by Tab. -- [ ] Screen-reader labels present on buttons, links, and form fields. -- [ ] Color contrast meets WCAG AA. - -## 4. Responsive parity - -- [ ] Mobile (375px): nav, menus, and layout all work. -- [ ] Desktop (1280px): no overflow or broken grids. -- [ ] Dark mode (if supported) renders correctly. - -## 5. Data and links - -- [ ] Every download link resolves (no 404s). -- [ ] Any displayed number or statistic traces to a real source. -- [ ] No placeholder or fabricated content shipped. - -## 6. Security - -- [ ] No secrets in the bundle or committed files. +# Release smoke test: cryptoscan + +Manual pre-release walkthrough. Run this before every tag push. Record the +actual output, not a summary. CryptoScan is a Go CLI, so build the +CI-equivalent binary rather than a plain `go build`: the release injects the +version, and a plain build carries the `dev` default, which hides every +version-provenance defect. + +## 1. Build the artifact the release will ship + +```bash +go build -ldflags "-X main.version=" -o /tmp/cryptoscan ./cmd/cryptoscan +/tmp/cryptoscan version +``` + +- [ ] `go build ./...` succeeds. +- [ ] `go test -race ./...` passes. +- [ ] `go vet ./...` is clean. +- [ ] `cryptoscan version` prints the version being released, not `dev`. +- [ ] `main.version` is a `var`, not a `const` (a `const` silently ignores `-X`). + +## 2. Detection: operational crypto is reported + +Every line below must produce at least one finding. Each one reported zero +findings at some point in the tool's history, because a noise filter matched a +substring elsewhere on the line. + +```bash +d=$(mktemp -d) +printf 'a.md5(b)\n' > $d/a.py +printf 'return hashlib.md5(data).hexdigest()\n' > $d/b.py +printf 'cipher = md5(b"a")\n' > $d/c.py +printf 'block, err := des.NewCipher(key)\n' > $d/d.go +printf "const x = crypto.createHash('md5')\n" > $d/e.js +printf 'const rc4 = CryptoJS.RC4.encrypt(data, key)\n' > $d/f.js +printf 'Cipher cipher = Cipher.getInstance("RC4");\n' > $d/g.java +/tmp/cryptoscan scan $d --format json | python3 -c 'import sys,json;print(len(json.load(sys.stdin)["findings"]),"findings")' +``` + +- [ ] At least 7 findings (one per file). Zero for any file is a release blocker. +- [ ] Scan the repo's own `crypto-samples/` tree: the deliberately vulnerable + samples are reported, including the DES, 3DES, RC4 and MD5 cases. + +## 3. Detection: mentions in text are withheld, visibly and recoverably + +- [ ] A log line (`log.Printf("falling back to md5")`) produces no finding. +- [ ] The summary states how many mentions were withheld. +- [ ] `--include-narrative` brings them back. +- [ ] Detected key material is never withheld, even inside a comment. + +## 4. Suppression comments + +```bash +printf 'import hashlib\nA=hashlib.sha1(b"1") # cryptoscan:ignore\nB=hashlib.sha1(b"2")\nC=hashlib.sha1(b"3")\n' > $d/ig.py +``` + +- [ ] Findings are reported on lines 3 and 4 only. Line 2 is suppressed and no + other line is affected by it. +- [ ] `# cryptoscan:ignore-next-line` suppresses the following line. +- [ ] `cryptoscan:ignore RSA-001` suppresses only that pattern. + +## 5. Every output surface agrees + +```bash +for f in json sarif cbom; do /tmp/cryptoscan scan $d --format $f; done +``` + +- [ ] JSON `tool.version`, SARIF `runs[0].tool.driver.version` and + `.semanticVersion`, and CBOM `metadata.tools[0].version` all equal the + output of `cryptoscan version`. +- [ ] The CBOM validates against the official CycloneDX 1.6 schema, fetched + from the CycloneDX specification repository, not against the tool's own + tests. Resolve the `jsf-0.82` and `spdx` `$ref`s and validate with + python `jsonschema`. +- [ ] SARIF result locations point at real files. + +## 6. Determinism + +```bash +for i in 1 2 3; do + /tmp/cryptoscan scan crypto-samples --format json \ + | python3 -c 'import sys,json;d=json.load(sys.stdin);print([(f["file"],f["line"],f["id"]) for f in d["findings"]])' \ + | shasum -a 256 +done +``` + +- [ ] All three hashes match. Finding order must not vary between runs of an + unchanged tree, or CI diffs and CBOMs are not reproducible. + +## 7. Output and exit codes + +- [ ] Piped (non-TTY) output contains no ANSI escape sequences. +- [ ] `--no-color` output contains no ANSI escape sequences and no emoji. +- [ ] `--help` lists every flag, and every flag it lists actually parses. +- [ ] An invalid `--format` value is rejected with a clear message and a + non-zero exit code, rather than silently falling back to text. +- [ ] Exit codes match what `--fail-on` documents. + +## 8. Findings read well + +For each finding in a real scan, check all five: + +- [ ] It names the file and line. +- [ ] It says what specifically is wrong. +- [ ] The remediation is a concrete action, not general advice. +- [ ] Severity is coherent against the other findings on the same file. +- [ ] A non-developer security manager would understand what to do. + +## 9. Security and hygiene + +- [ ] No secrets in committed files. - [ ] `.env` and credential files are gitignored and excluded. -- [ ] Forms validate and sanitize input. +- [ ] `npx hackmyagent secure --ci` reports no CRITICAL or HIGH findings, or + each one is a documented and verified false positive. ## Result +- Version: - Date: - Tester: - Verdict: PASS / FAIL diff --git a/internal/cli/root.go b/internal/cli/root.go index d5fd977..6dd94c7 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -7,10 +7,11 @@ import ( "fmt" "github.com/spf13/cobra" + + "github.com/csnp/cryptoscan/pkg/version" ) var ( - version = "dev" commit = "none" buildDate = "unknown" ) @@ -47,8 +48,11 @@ Examples: Learn more at https://qramm.org`, } +// SetVersionInfo records the build metadata injected by GoReleaser. The version +// string is stored in pkg/version so that every output surface reports the same +// value; commit and build date are only shown by the version command. func SetVersionInfo(v, c, d string) { - version = v + version.Set(v) commit = c buildDate = d } @@ -66,7 +70,7 @@ var versionCmd = &cobra.Command{ Use: "version", Short: "Print version information", Run: func(cmd *cobra.Command, args []string) { - fmt.Printf("cryptoscan %s\n", version) + fmt.Printf("cryptoscan %s\n", version.Get()) if commit != "none" { fmt.Printf(" commit: %s\n", commit) } diff --git a/internal/cli/scan.go b/internal/cli/scan.go index 72c2b2b..1c1ac57 100644 --- a/internal/cli/scan.go +++ b/internal/cli/scan.go @@ -35,6 +35,7 @@ var ( streamFindings bool includeImports bool includeQuantumSafe bool + includeNarrative bool verbose bool // CI/CD flexibility flags @@ -108,7 +109,8 @@ func init() { scanCmd.Flags().BoolVar(&streamFindings, "stream", true, "Show findings as they are discovered") scanCmd.Flags().BoolVar(&includeImports, "include-imports", false, "Include library import findings (normally suppressed as low-value)") scanCmd.Flags().BoolVar(&includeQuantumSafe, "include-quantum-safe", false, "Include quantum-safe algorithm findings (SHA-256, AES-256)") - scanCmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show all findings including imports and quantum-safe algorithms") + scanCmd.Flags().BoolVar(&includeNarrative, "include-narrative", false, "Include algorithms named in prose, log messages, documentation and config keys") + scanCmd.Flags().BoolVarP(&verbose, "verbose", "v", false, "Show all findings including imports, quantum-safe algorithms and narrative mentions") // CI/CD flexibility flags scanCmd.Flags().StringVar(&ignorePatterns, "ignore", "", "Pattern IDs to ignore (comma-separated, e.g., \"RSA-001,CERT-*\")") @@ -121,6 +123,16 @@ func init() { func runScan(cmd *cobra.Command, args []string) error { target := args[0] + // Disable colour when the output is not a terminal, so that redirecting to + // a file or piping into another tool produces clean text rather than ANSI + // escape sequences. An explicit --no-color still wins, and NO_COLOR is + // honoured per https://no-color.org. + if !cmd.Flags().Changed("no-color") { + if os.Getenv("NO_COLOR") != "" || !isTerminal(os.Stdout) { + noColor = true + } + } + // Load config file (explicit or auto-detected) var cfgFile *config.Config cfgPath := configFile @@ -222,6 +234,7 @@ func runScan(cmd *cobra.Command, args []string) error { MinSeverity: parseSeverity(effectiveMinSeverity), IncludeImports: includeImports || verbose, // Include if explicitly set or verbose mode IncludeQuantumSafe: includeQuantumSafe || verbose, // Include if explicitly set or verbose mode + IncludeNarrative: includeNarrative || verbose, // Include if explicitly set or verbose mode IgnoreIDs: ignoreIDs, IgnoreCategories: ignoreCats, } @@ -236,7 +249,7 @@ func runScan(cmd *cobra.Command, args []string) error { findingCount++ num := findingCount // Clear the progress line before printing finding - fmt.Print("\r\033[K") + clearProgressLine() printStreamFinding(f, num, !noColor) outputMu.Unlock() } @@ -246,13 +259,16 @@ func runScan(cmd *cobra.Command, args []string) error { count := fileCount outputMu.Unlock() // Show progress every 50 files (less frequent for parallel scanning) - if count%50 == 0 { + // The progress line rewrites itself in place, which only makes + // sense on a terminal. Piped output would otherwise collect a + // carriage return and an erase-line sequence per update. + if count%50 == 0 && !noColor { shortPath := path if len(shortPath) > 50 { shortPath = "..." + shortPath[len(shortPath)-47:] } outputMu.Lock() - fmt.Printf("\r\033[K \033[2m📂 %d files scanned | %s\033[0m", count, shortPath) + fmt.Printf("\r\033[K \033[2m%d files scanned | %s\033[0m", count, shortPath) outputMu.Unlock() } } @@ -289,7 +305,7 @@ func runScan(cmd *cobra.Command, args []string) error { // Print streaming footer with summary if streamFindings && outputFormat == "text" { // Clear any remaining progress line - fmt.Print("\r\033[K") + clearProgressLine() printScanningFooter(findingCount, fileCount, duration, !noColor) } @@ -391,7 +407,7 @@ func printBanner() { fmt.Println(" ║ ║") fmt.Println(" ╚═══════════════════════════════════════════════════════════════╝" + colorReset) fmt.Println() - fmt.Println(colorBlue + " Crypto Scan — QRAMM Cryptographic Discovery" + colorReset) + fmt.Println(colorBlue + " Crypto Scan: QRAMM Cryptographic Discovery" + colorReset) fmt.Println(colorDim + " Quantum Readiness Assurance & Migration Tool" + colorReset) fmt.Println() fmt.Println(colorGreen + " ┌─────────────────────────────────────────────────────────────┐") @@ -435,10 +451,10 @@ func printScanningFooter(findingCount, fileCount int, duration time.Duration, us fmt.Println() if useColor { - fmt.Printf("%s%s ✓ Scan complete%s — %d findings in %d files (%s)\n\n", + fmt.Printf("%s%s \u2713 Scan complete%s: %d findings in %d files (%s)\n\n", colorGreen, colorBold, colorReset, findingCount, fileCount, duration.Round(time.Millisecond)) } else { - fmt.Printf(" Scan complete — %d findings in %d files (%s)\n\n", findingCount, fileCount, duration.Round(time.Millisecond)) + fmt.Printf(" Scan complete: %d findings in %d files (%s)\n\n", findingCount, fileCount, duration.Round(time.Millisecond)) } } @@ -459,24 +475,24 @@ func printStreamFinding(f scanner.Finding, num int, useColor bool) { var sevIcon, sevColor string switch f.Severity { case scanner.SeverityCritical: - sevIcon, sevColor = "🔴", colorRed+colorBold + sevIcon, sevColor = "●", colorRed+colorBold case scanner.SeverityHigh: - sevIcon, sevColor = "🟠", colorRed + sevIcon, sevColor = "●", colorRed case scanner.SeverityMedium: - sevIcon, sevColor = "🟡", colorYellow + sevIcon, sevColor = "●", colorYellow case scanner.SeverityLow: - sevIcon, sevColor = "🔵", colorBlue + sevIcon, sevColor = "●", colorBlue default: - sevIcon, sevColor = "⚪", colorCyan + sevIcon, sevColor = "●", colorCyan } // Quantum risk indicator var qIcon string switch f.Quantum { case scanner.QuantumVulnerable: - qIcon = "⚠️ " + qIcon = "[!]" case scanner.QuantumPartial: - qIcon = "⚡" + qIcon = "[~]" default: qIcon = " " } @@ -630,3 +646,23 @@ func calculateSummary(findings []scanner.Finding) scanner.Summary { return summary } + +// clearProgressLine erases the in-place progress line. It is a no-op when +// colour is off, since a redirected stream has no cursor to move. +func clearProgressLine() { + if noColor { + return + } + fmt.Print("\r\033[K") +} + +// isTerminal reports whether f is attached to a terminal. A redirected or +// piped stream is a character device only when it is a terminal, so this +// distinguishes interactive output from output being captured. +func isTerminal(f *os.File) bool { + info, err := f.Stat() + if err != nil { + return false + } + return info.Mode()&os.ModeCharDevice != 0 +} diff --git a/internal/cli/scan_test.go b/internal/cli/scan_test.go index ecdad99..c45c04d 100644 --- a/internal/cli/scan_test.go +++ b/internal/cli/scan_test.go @@ -12,6 +12,7 @@ import ( "time" "github.com/csnp/cryptoscan/pkg/scanner" + "github.com/csnp/cryptoscan/pkg/version" "github.com/spf13/cobra" ) @@ -89,8 +90,8 @@ func TestRootCmdExists(t *testing.T) { func TestSetVersionInfo(t *testing.T) { SetVersionInfo("1.0.0", "abc123", "2025-01-01") - if version != "1.0.0" { - t.Errorf("version = %q, want %q", version, "1.0.0") + if got := version.Get(); got != "1.0.0" { + t.Errorf("version.Get() = %q, want %q", got, "1.0.0") } if commit != "abc123" { t.Errorf("commit = %q, want %q", commit, "abc123") diff --git a/pkg/patterns/matcher.go b/pkg/patterns/matcher.go index b67027b..fa61a0b 100644 --- a/pkg/patterns/matcher.go +++ b/pkg/patterns/matcher.go @@ -423,7 +423,11 @@ func (m *Matcher) loadPatterns() { ID: "DES-001", Name: "DES Algorithm", Category: "Deprecated Algorithm", - Regex: regexp.MustCompile(`(?i)\bDES[-_](CBC|ECB|CFB|OFB)\b|\bDESede\b|Cipher\.getInstance\s*\(\s*["']DES["']|createCipher\s*\(\s*["']des|crypto\.createCipher.*["']des|\bDES\.(new|encrypt|decrypt)\b|\bDES\.MODE_`), + // des.NewCipher is the Go standard library's only way to construct a + // DES cipher, and it was not covered: \bDES\.(new|...)\b cannot match + // "des.NewCipher" because the \b after "new" requires a non-word + // character. Go DES usage was therefore invisible to the scanner. + Regex: regexp.MustCompile(`(?i)\bDES[-_](CBC|ECB|CFB|OFB)\b|\bDESede\b|Cipher\.getInstance\s*\(\s*["']DES["']|createCipher\s*\(\s*["']des|crypto\.createCipher.*["']des|\bDES\.(new|encrypt|decrypt)\b|\bDES\.MODE_|\bdes\.NewCipher\s*\(`), Severity: types.SeverityCritical, Quantum: types.QuantumVulnerable, Algorithm: "DES", @@ -436,7 +440,9 @@ func (m *Matcher) loadPatterns() { ID: "3DES-001", Name: "Triple DES Algorithm", Category: "Deprecated Algorithm", - Regex: regexp.MustCompile(`(?i)\b(3DES|Triple[-_]?DES|DESede|TDEA)\b`), + // des.NewTripleDESCipher is the Go standard library constructor; the + // \b(3DES|...)\b alternation cannot reach it. + Regex: regexp.MustCompile(`(?i)\b(3DES|Triple[-_]?DES|DESede|TDEA)\b|\bdes\.NewTripleDESCipher\s*\(`), Severity: types.SeverityHigh, Quantum: types.QuantumVulnerable, Algorithm: "3DES", diff --git a/pkg/reporter/cbom.go b/pkg/reporter/cbom.go index a46f3e3..5ddc22b 100644 --- a/pkg/reporter/cbom.go +++ b/pkg/reporter/cbom.go @@ -15,6 +15,7 @@ import ( "github.com/csnp/cryptoscan/pkg/analyzer" "github.com/csnp/cryptoscan/pkg/scanner" "github.com/csnp/cryptoscan/pkg/types" + "github.com/csnp/cryptoscan/pkg/version" ) // CBOMReporter generates Cryptographic Bill of Materials output @@ -529,7 +530,7 @@ func (r *CBOMReporter) Generate(results *scanner.Results) (string, error) { { Vendor: "CSNP", Name: "CryptoScan", - Version: "1.1.0", + Version: version.Get(), }, }, Component: summaryComponent, diff --git a/pkg/reporter/sarif.go b/pkg/reporter/sarif.go index c0306e4..be297b3 100644 --- a/pkg/reporter/sarif.go +++ b/pkg/reporter/sarif.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/csnp/cryptoscan/pkg/scanner" + "github.com/csnp/cryptoscan/pkg/version" ) // SARIFReporter generates SARIF 2.1.0 format output @@ -196,8 +197,8 @@ func (r *SARIFReporter) Generate(results *scanner.Results) (string, error) { Tool: sarifTool{ Driver: sarifDriver{ Name: "CryptoScan", - Version: "1.0.0", - SemanticVersion: "1.0.0", + Version: version.Get(), + SemanticVersion: version.Get(), InformationURI: "https://qramm.org", Rules: rules, }, diff --git a/pkg/reporter/text.go b/pkg/reporter/text.go index 4ed93aa..cc15186 100644 --- a/pkg/reporter/text.go +++ b/pkg/reporter/text.go @@ -63,9 +63,9 @@ func (r *TextReporter) severityColor(s scanner.Severity) string { func (r *TextReporter) quantumIcon(q scanner.QuantumRisk) string { switch q { case scanner.QuantumVulnerable: - return "⚠️ QUANTUM VULNERABLE" + return "[!] QUANTUM VULNERABLE" case scanner.QuantumPartial: - return "⚡ QUANTUM WEAKENED" + return "[~] QUANTUM WEAKENED" case scanner.QuantumSafe: return "✓ QUANTUM SAFE" default: @@ -131,12 +131,22 @@ func (r *TextReporter) Generate(results *scanner.Results) (string, error) { r.color(sev.color, bar))) } } + + // Report what was withheld. A scanner may reduce noise, but it must never + // reduce it invisibly: the reader has to be able to tell the difference + // between "nothing was found" and "something was found and not shown". + if results.Summary.NarrativeSuppressed > 0 { + b.WriteString(r.color(colorCyan, fmt.Sprintf( + "\n %d finding(s) withheld as documentation, log or comment text.\n"+ + " Show them with: cryptoscan scan %s --include-narrative\n", + results.Summary.NarrativeSuppressed, results.ScanTarget))) + } b.WriteString("\n") // Quantum Risk Assessment with visual emphasis b.WriteString(r.color(colorBold, " Quantum Risk Assessment:\n")) if results.Summary.QuantumVulnCount > 0 { - b.WriteString(r.color(colorRed+colorBold, fmt.Sprintf(" ⚠️ %d quantum-vulnerable findings require migration planning\n", results.Summary.QuantumVulnCount))) + b.WriteString(r.color(colorRed+colorBold, fmt.Sprintf(" [!] %d quantum-vulnerable findings require migration planning\n", results.Summary.QuantumVulnCount))) } quantumRisks := []struct { name string @@ -418,7 +428,7 @@ func (r *TextReporter) writeMigrationScore(b *strings.Builder, score *scanner.Mi // Status breakdown b.WriteString(r.color(colorCyan, "│") + r.color(colorBold, " INVENTORY ") + r.color(colorCyan, "│\n")) b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %s Safe (PQC): %-5d", r.color(colorGreen, "✓"), score.SafeCount) + fmt.Sprintf(" %s Vulnerable: %-5d", r.color(colorRed, "✗"), score.VulnerableCount) + r.color(colorCyan, " │\n")) - b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %s Hybrid: %-5d", r.color(colorGreen, "◐"), score.HybridCount) + fmt.Sprintf(" %s Critical: %-5d", r.color(colorRed+colorBold, "⚠"), score.CriticalCount) + r.color(colorCyan, " │\n")) + b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %s Hybrid: %-5d", r.color(colorGreen, "◐"), score.HybridCount) + fmt.Sprintf(" %s Critical: %-5d", r.color(colorRed+colorBold, "[!]"), score.CriticalCount) + r.color(colorCyan, " │\n")) b.WriteString(r.color(colorCyan, "│") + fmt.Sprintf(" %s Partial: %-5d", r.color(colorYellow, "◑"), score.PartialCount) + fmt.Sprintf(" Total: %-5d", score.TotalCount) + r.color(colorCyan, " │\n")) // QRAMM Readiness diff --git a/pkg/reporter/version_test.go b/pkg/reporter/version_test.go new file mode 100644 index 0000000..040a630 --- /dev/null +++ b/pkg/reporter/version_test.go @@ -0,0 +1,131 @@ +// Copyright 2025 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package reporter + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/csnp/cryptoscan/pkg/scanner" + "github.com/csnp/cryptoscan/pkg/version" +) + +// TestEveryEmitterReportsTheSameVersion guards the provenance defect found by +// the v1.4.0 release test: one binary reported four different versions. The +// version command said 1.4.0, SARIF said 1.0.0, CBOM said 1.1.0 and JSON +// carried no version at all. +// +// SARIF and CBOM are evidence artifacts. A CBOM claiming it was produced by +// CryptoScan 1.1.0 is a false provenance record, and a SARIF consumer cannot +// attribute a result to a scanner build. Every surface must read pkg/version. +func TestEveryEmitterReportsTheSameVersion(t *testing.T) { + const want = "9.8.7-test" + + original := version.Get() + version.Set(want) + t.Cleanup(func() { version.Set(original) }) + + results := &scanner.Results{ + Tool: scanner.ToolInfo{Name: "CryptoScan", Version: version.Get()}, + ScanTarget: ".", + Findings: []scanner.Finding{}, + Summary: scanner.Summary{}, + } + + t.Run("json", func(t *testing.T) { + out := generate(t, NewJSONReporter(false), results) + var doc struct { + Tool struct { + Version string `json:"version"` + } `json:"tool"` + } + decode(t, out, &doc) + if doc.Tool.Version != want { + t.Errorf("JSON tool.version = %q, want %q", doc.Tool.Version, want) + } + }) + + t.Run("sarif", func(t *testing.T) { + out := generate(t, NewSARIFReporter(), results) + var doc struct { + Runs []struct { + Tool struct { + Driver struct { + Version string `json:"version"` + SemanticVersion string `json:"semanticVersion"` + } `json:"driver"` + } `json:"tool"` + } `json:"runs"` + } + decode(t, out, &doc) + if len(doc.Runs) == 0 { + t.Fatal("SARIF report has no runs") + } + d := doc.Runs[0].Tool.Driver + if d.Version != want { + t.Errorf("SARIF driver.version = %q, want %q", d.Version, want) + } + if d.SemanticVersion != want { + t.Errorf("SARIF driver.semanticVersion = %q, want %q", d.SemanticVersion, want) + } + }) + + t.Run("cbom", func(t *testing.T) { + out := generate(t, NewCBOMReporter(), results) + var doc struct { + Metadata struct { + Tools []struct { + Version string `json:"version"` + } `json:"tools"` + } `json:"metadata"` + } + decode(t, out, &doc) + if len(doc.Metadata.Tools) == 0 { + t.Fatal("CBOM has no metadata.tools entry") + } + if got := doc.Metadata.Tools[0].Version; got != want { + t.Errorf("CBOM metadata.tools[0].version = %q, want %q", got, want) + } + }) + + // A literal left behind anywhere in the emitters would defeat the single + // source of truth without failing the assertions above, so check that no + // output still carries the old hardcoded values. + t.Run("no stale literals", func(t *testing.T) { + for name, out := range map[string]string{ + "json": generate(t, NewJSONReporter(false), results), + "sarif": generate(t, NewSARIFReporter(), results), + "cbom": generate(t, NewCBOMReporter(), results), + } { + for _, stale := range []string{`"1.0.0"`, `"1.1.0"`} { + // SARIF's own spec version is legitimately 2.1.0; only the + // tool version fields are checked here. + if strings.Contains(out, `"version": `+stale) { + t.Errorf("%s still emits a hardcoded tool version %s", name, stale) + } + } + } + }) +} + +type reportGenerator interface { + Generate(*scanner.Results) (string, error) +} + +func generate(t *testing.T, r reportGenerator, results *scanner.Results) string { + t.Helper() + out, err := r.Generate(results) + if err != nil { + t.Fatalf("generate: %v", err) + } + return out +} + +func decode(t *testing.T, out string, into any) { + t.Helper() + if err := json.Unmarshal([]byte(out), into); err != nil { + t.Fatalf("decode report: %v\n%s", err, out) + } +} diff --git a/pkg/scanner/determinism_test.go b/pkg/scanner/determinism_test.go new file mode 100644 index 0000000..b9ea34f --- /dev/null +++ b/pkg/scanner/determinism_test.go @@ -0,0 +1,60 @@ +// Copyright 2025 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package scanner + +import ( + "fmt" + "path/filepath" + "testing" + + "github.com/csnp/cryptoscan/pkg/types" +) + +// TestFindingOrderIsDeterministic guards against the map-iteration ordering +// found by the v1.4.0 release test. deduplicateFindings built its result by +// ranging over a map, and Go randomises map iteration, so repeated scans of an +// unchanged tree produced the same findings in a different order every time. +// +// That breaks diffable CI output, golden-file tests and reproducible CBOMs. +// Several files with several findings each are needed to make the old +// behaviour fail reliably rather than by luck. +func TestFindingOrderIsDeterministic(t *testing.T) { + dir := t.TempDir() + for i := 0; i < 6; i++ { + write(t, filepath.Join(dir, fmt.Sprintf("f%d.py", i)), `import hashlib +a = hashlib.md5(b"x") +b = hashlib.sha1(b"y") +c = hashlib.new("md5") +`) + } + + var first []string + for run := 0; run < 5; run++ { + results, err := New(Config{Target: dir, MinSeverity: types.SeverityInfo}).Scan() + if err != nil { + t.Fatalf("scan: %v", err) + } + + order := make([]string, 0, len(results.Findings)) + for _, f := range results.Findings { + order = append(order, fmt.Sprintf("%s:%d:%d:%s", f.File, f.Line, f.Column, f.ID)) + } + + if run == 0 { + if len(order) < 6 { + t.Fatalf("fixture produced only %d findings; too few to detect an ordering change", len(order)) + } + first = order + continue + } + if len(order) != len(first) { + t.Fatalf("run %d returned %d findings, run 0 returned %d", run, len(order), len(first)) + } + for i := range order { + if order[i] != first[i] { + t.Fatalf("run %d differs from run 0 at position %d: %q vs %q", run, i, order[i], first[i]) + } + } + } +} diff --git a/pkg/scanner/evidence.go b/pkg/scanner/evidence.go new file mode 100644 index 0000000..d677f7b --- /dev/null +++ b/pkg/scanner/evidence.go @@ -0,0 +1,518 @@ +// Copyright 2025 CyberSecurity NonProfit (CSNP) +// SPDX-License-Identifier: Apache-2.0 + +package scanner + +import "strings" + +// Evidence classification for a single pattern match. +// +// The scanner needs to separate "this codebase uses MD5" from "this line of +// prose mentions MD5". The original implementation answered that question by +// testing the whole source line against a list of about a hundred substrings +// and dropping the finding on any hit. That is unsound in a way that produced +// silent false negatives: +// +// a.md5(b) suppressed by ".md" +// return hashlib.md5(b"data").hexdigest() suppressed by ".md" +// Cipher cipher = Cipher.getInstance("DES") suppressed by "cipher =" +// const rc4 = CryptoJS.RC4.encrypt(d, k) suppressed by "const " +// +// None of those substrings had anything to do with the matched token. ".md" +// was meant to catch Markdown filenames and instead matched every ".md5(" in +// the corpus. +// +// # Ordering is the safety property +// +// The replacement asks the operational question FIRST, and never suppresses a +// match that answers it. This ordering is not cosmetic. An earlier revision of +// this file ran the log-message, comment and URL rules ahead of the +// operational check, and that alone reintroduced the same bug class through a +// different door: +// +// outputs = Cipher.getInstance("RC4") suppressed: "puts " matched "outputs " +// inputs = crypto.createHash("md5") suppressed: "puts " matched "inputs " +// h = hashlib.md5(load("a.md")) suppressed: ".md" again, via the token +// +// Any narrative rule is a heuristic over weak evidence. Running them after the +// operational check bounds the damage a wrong heuristic can do to lines that +// are not obviously code. +// +// # Suppression is bounded, counted and recoverable +// +// Narrative rules are deliberately narrow, because a missed finding in a +// security scanner costs far more than an extra line of output: +// +// Only three narrative rules survive, and all three are structural. They ask +// what the match belongs to, never which words surround it: +// +// - The match is inside a string owned by a logging or error call. The callee +// is matched on identifier boundaries, so "puts" does not match "outputs". +// - The match is inside a comment, in a language that actually has that +// comment marker. "#" is a comment in Python and a private field in +// JavaScript; "--" is a comment in SQL and a decrement in Java. +// - The match is inside a URL or a document path, tested on the token's real +// suffix rather than by substring. +// +// Rules that classified by vocabulary were tried and removed. A quoted string +// of three or more words was treated as prose, which hid +// `password_encryption TO 'md5'` and a dozen other real settings; a leading +// `description:` or `title:` owned the rest of its line, which lost a CRITICAL +// finding to every key that was not on the list. Configuration is where +// algorithm names legitimately live, so no rule may key off the words in a +// value. +// +// Everything the classifier does withhold is counted and recoverable with +// --include-narrative. + +// evidenceClass describes what a matched token tells us about the codebase. +type evidenceClass int + +const ( + // evidenceOperational means the token participates in a cryptographic + // operation: a call, an instantiation, a member access, an import, or an + // algorithm name handed to a crypto factory. + evidenceOperational evidenceClass = iota + + // evidenceNarrative means the token appears in text rather than in code: + // prose, a log message, a comment, a URL, or a documentation label. + evidenceNarrative +) + +// matchContext carries what the classifier knows about where a match came +// from. Language and file type come from the file analyzer and are needed to +// interpret comment markers and to decide whether whole-line prose detection +// applies at all. +type matchContext struct { + Line string + Start int + End int + Language string + FileType string +} + +// classifyMatch reports what kind of evidence a matched token represents, +// along with the rule that decided it. The reason string is for diagnostics +// and tests; callers should not parse it. +// +// Start and End are byte offsets into Line. An offset the classifier cannot +// interpret yields evidenceOperational: an unclassifiable match must be +// reported, never silently dropped. +func classifyMatch(mc matchContext) (evidenceClass, string) { + line := mc.Line + if mc.Start < 0 || mc.End > len(line) || mc.Start >= mc.End { + return evidenceOperational, "unclassifiable" + } + + quoteStart, _, inString := stringLiteralAt(line, mc.Start, mc.End) + + // An import names a library the codebase depends on. Evidence of use, even + // though a module path looks like a file path. + if isImportLine(line) { + return evidenceOperational, "import" + } + + // The operational question, asked before any heuristic. + if reason, ok := operationalUse(line, mc.Start, mc.End, quoteStart, inString); ok { + return evidenceOperational, reason + } + + // The match sits inside a string that is being logged or turned into an + // error message. The call prefix is matched on identifier boundaries. + if inString && isLoggingCall(line[:quoteStart]) { + return evidenceNarrative, "log-message" + } + + if at := commentStartsAt(line, mc.Language); at >= 0 && at < mc.Start { + return evidenceNarrative, "comment" + } + + if isPathLikeToken(tokenAround(line, mc.Start, mc.End)) { + return evidenceNarrative, "url-or-path" + } + + // Everything else is reported. + // + // There used to be two more rules here and both were false-negative + // generators, so they are gone rather than tuned: + // + // - A quoted string of three or more words counted as prose. A quoted + // string is the single most common place an algorithm name legitimately + // appears as configuration, so this hid + // `db.Exec("ALTER SYSTEM SET password_encryption TO 'md5'")`, + // `props.put("jdk.tls.client.cipherSuites", "RC4 and DES ...")` and ten + // other real settings. Log and error strings are still withheld, but by + // the structural rule above, which asks what call the string belongs to + // rather than what words are in it. + // - A leading documentation label (`description:`, `title:`) owned the + // rest of the line. Every revision of that rule lost a CRITICAL finding + // to a key that was not on the list: first `note:`, then `"title" :` + // with a space, then `:title =>`, then any key ending in a label such + // as `ssl-title:`. The word list was never the problem; handing a label + // the rest of the line is. + // + // What remains is structural: a string belonging to a logging call, a + // comment in a language that has that comment marker, and a URL or document + // path. None of those depend on which words a value happens to contain. + return evidenceOperational, "default" +} + +// operationalUse reports whether the token at [start,end) participates in a +// cryptographic operation. +func operationalUse(line string, start, end, quoteStart int, inString bool) (string, bool) { + // A call: md5(...), MD5(...), or a matched span ending in a call such as + // DES.encrypt(...). + if next := skipSpaces(line, end); next < len(line) && line[next] == '(' { + return "call", true + } + + // The matched span is itself a call. Several patterns match a whole + // construct rather than a bare token, for example + // `crypto.createCipheriv('des` or `Cipher.getInstance("DES")`. Those spans + // straddle the opening quote, so they are not "inside" a string literal and + // the factory rule below cannot see them. + if strings.Contains(line[start:end], "(") { + return "call-in-match", true + } + + // A member access or member call on the matched token: md5.New(), + // des.NewCipher(k), CryptoJS.MD5.encrypt(d). + // + // Both forms require the surrounding token to be an identifier chain. A + // dot inside a filename looks identical: "legacy-md5.md" would otherwise + // read as a member call on md5. + if !isPathLikeToken(tokenAround(line, start, end)) { + if end < len(line) && line[end] == '.' && end+1 < len(line) && isIdentChar(line[end+1]) { + return "member-call", true + } + + // The token is a member of something: hashlib.md5, a.md5, CryptoJS.MD5. + // Requires an identifier before the dot so that ".md5" inside a + // filename or a sentence does not qualify. + if start > 0 && line[start-1] == '.' && start >= 2 && isIdentChar(line[start-2]) { + return "member-access", true + } + } + + // An algorithm name passed to a crypto factory: + // Cipher.getInstance("RC4"), crypto.createHash('md5'), + // crypto.createCipheriv('aes-256-gcm', key, iv). + if inString && isCryptoFactoryCall(line[:quoteStart]) { + return "factory-argument", true + } + + return "", false +} + +// loggingCallees are call prefixes whose string arguments are program output +// rather than program behaviour. Each is matched on an identifier boundary. +var loggingCallees = []string{ + "log", "logger", "logging", "logrus", "slog", "zap", + "console.log", "console.error", "console.warn", "console.info", "console.debug", + "fmt.print", "fmt.printf", "fmt.println", "fmt.sprint", "fmt.sprintf", + "fmt.fprint", "fmt.fprintf", "fmt.errorf", + "print", "println", "printf", "puts", "echo", "warn", "panic", + "errors.new", "t.logf", "t.errorf", "t.fatalf", +} + +// isLoggingCall reports whether prefix, the text preceding a string literal on +// the same line, ends in a call to a logging, printing or error-construction +// function. +// +// Matching is identifier-anchored. A substring test here is what made "puts " +// match "outputs = ..." and "log." match "catalog.", hiding real crypto behind +// a variable name. +func isLoggingCall(prefix string) bool { + return hasCalleeSuffix(prefix, loggingCallees) +} + +// cryptoFactoryCallees are call prefixes whose string argument names the +// algorithm being instantiated. A match inside one of these is a use. +var cryptoFactoryCallees = []string{ + "getinstance", "createcipher", "createcipheriv", "createdecipher", + "createdecipheriv", "createhash", "createhmac", "createsign", "createverify", + "creatediffiehellman", "messagedigest", "keypairgenerator", "keygenerator", + "keyfactory", "secretkeyspec", "ivparameterspec", + "hashlib.new", "hmac.new", "cipher.new", "digest.new", +} + +// isCryptoFactoryCall reports whether prefix ends in a call that takes an +// algorithm name as an argument. +func isCryptoFactoryCall(prefix string) bool { + return hasCalleeSuffix(prefix, cryptoFactoryCallees) +} + +// hasCalleeSuffix reports whether prefix ends in a call to one of callees. +// +// The prefix is expected to end at an opening quote, so the shape being +// matched is `... callee ( args? "`. The callee has to end at an identifier +// boundary, which is what stops "puts" from matching inside "outputs". +func hasCalleeSuffix(prefix string, callees []string) bool { + // Walk back over the argument list to the opening parenthesis of the call + // that owns this string literal. + trimmed := strings.TrimRight(prefix, " \t") + depth := 0 + open := -1 + for i := len(trimmed) - 1; i >= 0; i-- { + switch trimmed[i] { + case ')': + depth++ + case '(': + if depth == 0 { + open = i + i = -1 + continue + } + depth-- + } + if open >= 0 { + break + } + } + if open < 0 { + return false + } + + name := strings.ToLower(strings.TrimRight(trimmed[:open], " \t")) + for _, callee := range callees { + if !strings.HasSuffix(name, callee) { + continue + } + // Identifier boundary: the character before the callee must not + // continue an identifier. A dot is allowed so that "fmt.print" and + // "console.log" match, and so that "crypto.createHash" matches the + // bare "createhash" entry. + if at := len(name) - len(callee); at == 0 || !isIdentChar(name[at-1]) { + return true + } + } + return false +} + +// commentMarkers returns the line-comment and block-comment openers for a +// language. +// +// An unknown language gets no markers rather than a permissive default: the +// cost of missing a comment is one noisy finding, and the cost of guessing +// wrong is a silently hidden one. +func commentMarkers(language string) []string { + switch language { + case "go", "java", "javascript", "typescript", "c", "cpp", "csharp", + "swift", "kotlin", "rust", "php": + return []string{"//", "/*"} + case "python", "ruby", "shell", "yaml", "toml": + return []string{"#"} + case "markdown", "xml": + return []string{"