Skip to content

Add badgeverify fuzz/redteam evals and security CI#26

Merged
TeoSlayer merged 2 commits into
mainfrom
feat/badgeverify-fuzz-security-ci
Jun 22, 2026
Merged

Add badgeverify fuzz/redteam evals and security CI#26
TeoSlayer merged 2 commits into
mainfrom
feat/badgeverify-fuzz-security-ci

Conversation

@TeoSlayer

Copy link
Copy Markdown
Contributor

What this PR does

Hardens the badgeverify crown-jewel crypto (offline Ed25519 badge / enrollment / recovery verification) with adversarial fuzz + red-team coverage, and adds a security CI baseline to the common module.

CI security jobs

Added (new)

  • security workflow (.github/workflows/security.yml), push + PR:
    • race-testgo test -race ./... (dedicated gating job)
    • codeql — github/codeql-action init + analyze (Go)
    • gosec — securego/gosec @v2.22.5, scoped to ./badgeverify/...
    • govulncheck — golang/x/vuln @v1.1.4, full module ./...
    • gitleaks — gitleaks/gitleaks-action@v2 (secret scanning)
    • fuzz — bounded 30s/target run of all six badgeverify fuzz targets
  • dependency-review workflow (.github/workflows/dependency-review.yml), PR-only, fail-on-severity: high
  • github-actions dependabot ecosystem added (weekly) alongside the existing gomod entry

Already present (not duplicated)

  • ci.yml test job (coverage + -race) — left untouched
  • dependabot.yml gomod weekly — extended, not replaced
  • badgeverify_test.go and redteam_test.go — kept; new evals are additive

Scanner findings + resolution

  • govulncheck — GO-2026-5037 (inefficient hostname parsing in stdlib crypto/x509, reached via registry/wire TLS + daemonapi). Fixed in go1.25.11. Resolved genuinely by bumping the go directive 1.25.10 → 1.25.11; CI setup-go: '1.25' resolves the patched toolchain. govulncheck ./... now reports No vulnerabilities found.
  • gosec./badgeverify/... is 0 issues. The full module has 74 pre-existing low-severity G104 findings (unhandled f.Close() / flag.Value.Set() errors in fsutil, config, driver) unrelated to this PR. The gosec job is scoped to badgeverify (the crypto being hardened) rather than blanket-suppressed or used to justify rewriting 43 unrelated files. Follow-up: triage/fix the module-wide G104 set in a dedicated PR.
  • No #nosec annotations were added anywhere.

Adversarial / fuzz evals added

New badgeverify/fuzz_test.go (6 native Go fuzz targets, deterministic seeds):

  • FuzzParseBadge, FuzzParseEnrollment, FuzzParseRecovery — no panic; any accepted value re-encodes canonically (round-trip == input), exercising the malleability guard.
  • FuzzVerify, FuzzVerifyEnrollment, FuzzVerifyRecoveryfail-closed (never returns valid for unsigned/garbage input) and never panics, against a real pinned keyring whose private half the fuzzer does not hold.

New badgeverify/redteam_extra_test.go (explicit attack cases not previously covered):

  • Enrollment↔badge↔recovery cross-type confusion in both directions
  • Key valid in the other keyring (online vs cold) rejected for the wrong statement type; key swapped under same kid rejected
  • Non-canonical integer encodings (leading zeros / signs / whitespace) rejected at the verify boundary for all three statement types
  • All-zero placeholder key fails closed for recovery + enrollment, not just badges
  • Signed-but-wrong-node binding mismatch via VerifyForNode
  • Never-expiring badge allowed vs never-expiring recovery disallowed asymmetry

Genuine bug fixed (found by the new fuzzers)

The fuzzers immediately surfaced a real malleability gap: Parse* used strconv.ParseUint/ParseInt, which accept multiple spellings of one value (01, +1, -0, 1). Since the signed bytes are the wire string, two spellings = two distinct signable strings. The existing badge-only verify round-trip check (#24) did not cover ParseEnrollment/ParseRecovery/VerifyEnrollment/VerifyRecovery.

Fix: all three parsers are now canonical-strict (isCanonicalDecimal + parseCanonicalUint32/parseCanonicalInt64), and ParseRecovery rejects exp<=0 at the parse boundary.

Validation

  • GOWORK=off go build ./... — clean
  • go vet ./... — clean
  • go test -race ./... — all 15 packages pass
  • Each fuzz target -fuzztime (15–30s) — all PASS, no crashes (millions of execs)
  • gofmt clean on all changed files
  • gosec ./badgeverify/... rc=0 / 0 issues; govulncheck ./... rc=0 / no vulns

Required status check names (for branch protection)

  • go test -race (job race-test)
  • CodeQL
  • gosec (badgeverify)
  • govulncheck
  • gitleaks
  • badgeverify fuzz
  • dependency-review (PR-only)

Harden the badgeverify badge/enrollment/recovery crypto and the
common-module CI:

- Native Go fuzz tests for all three parsers (round-trip/malleability
  guard) and all three verifiers (fail-closed, no-panic), with
  deterministic seed corpora.
- Strengthen the red-team suite: cross-type confusion in both
  directions, wrong-keyring/other-key rejection, non-canonical integer
  encodings for all three statement types, all-zero placeholder key
  rejection for every type, signed-but-wrong-node binding, and the
  never-expiring badge-allowed vs recovery-disallowed asymmetry.
- Make all three parsers canonical-strict: reject leading zeros, signs,
  and whitespace in node_id/timestamps/exp so a logical value has one
  signable wire spelling; reject recovery exp<=0 at parse. Closes the
  enrollment/recovery malleability gap left by the badge-only verify
  check (uncovered by the new fuzzers).
- Add security workflow: race-test gate, CodeQL (Go), gosec (scoped to
  badgeverify), govulncheck, gitleaks, and a bounded badgeverify fuzz
  job. Add dependency-review on PR; extend dependabot to github-actions.
- Bump go directive to 1.25.11 to clear GO-2026-5037 (stdlib x509).
@github-advanced-security

Copy link
Copy Markdown

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
badgeverify/badgeverify.go 87.50% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

The gitleaks Action requires a paid GITLEAKS_LICENSE on org repos; run
the OSS gitleaks binary instead (no license, fails on any finding).
dependency-review hard-errors until the repo Dependency Graph feature
is enabled, so mark it continue-on-error until an admin enables it.
@TeoSlayer

Copy link
Copy Markdown
Contributor Author

Follow-up commit b6ec4a4 resolves the two infra-gated check failures:

  • gitleaks: the gitleaks/gitleaks-action@v2 wrapper requires a paid GITLEAKS_LICENSE secret on organization repos. Switched the job to run the OSS gitleaks CLI binary directly (pinned v8.21.2) — full-history scan, no license, --exit-code 1 so it still fails on any finding.
  • dependency-review: actions/dependency-review-action@v4 hard-errors until the repo's Dependency Graph feature is enabled (Settings → Code security & analysis). Marked the step continue-on-error: true so it doesn't block PRs before an admin enables it; once enabled it reports real findings and can be promoted to a hard gate.

All other new security jobs (race-test, CodeQL, gosec, govulncheck, badgeverify fuzz) passed on the first run.

@TeoSlayer TeoSlayer merged commit 542658f into main Jun 22, 2026
11 checks passed
@matthew-pilot matthew-pilot deleted the feat/badgeverify-fuzz-security-ci branch June 22, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants