security(scep-intune): annotate verifyES256/RS256 SHA-256 as RFC-mandated (CodeQL #21 false positive)

CodeQL alert #21 (go/weak-sensitive-data-hashing, severity: High)
flagged the sha256.Sum256(signingInput) call in verifyES256 at
internal/scep/intune/challenge.go:380 as 'weak hashing of sensitive
data', suggesting PBKDF2/Argon2/bcrypt instead.

This is a CodeQL false positive. The CodeQL query triggers when
SHA-256 is used near *x509.Certificate (the trust pool) and infers
'this might be password hashing.' But the actual context is JWS
signature verification:

  - verifyRS256 implements RFC 7518 §3.3 — 'RSASSA-PKCS1-v1_5
    using SHA-256'. SHA-256 is spec-mandated.
  - verifyES256 implements RFC 7518 §3.4 — 'ECDSA using P-256
    and SHA-256'. SHA-256 is spec-mandated.
  - The signing input is the JWS protected header + payload
    (base64url-encoded). It is a public, well-known message with
    full 256-bit-entropy contributed by signer-controlled nonces +
    timestamps + device claims — the opposite of a low-entropy
    password.
  - The output is verified against an asymmetric signature
    (rsa.VerifyPKCS1v15 / ecdsa.Verify), not compared to a
    pre-computed hash digest. This is signature verification,
    not password hashing.
  - Switching to PBKDF2 / Argon2 / bcrypt would BREAK every Intune
    Connector signed challenge — Microsoft + every spec-conforming
    JWS library will only verify against SHA-256 for these algs.

Fix: add explicit RFC-citing comment blocks above each verifier
function explaining the JWS context + add //nolint:gosec
annotations on the sha256.Sum256 calls so CodeQL recognizes the
suppression rationale at the call site. The annotation cites the
specific RFC clause (7518 §3.3 / §3.4) so a future security
reviewer can re-derive the conclusion without re-reading the alert.

The algorithm allowlist itself stays defensively narrow:
  - alg="RS256" → verifyRS256 with SHA-256
  - alg="ES256" → verifyES256 with SHA-256
  - alg="none" → explicit reject (RFC 7515 §3.6 attack vector)
  - any other alg → reject as unsupported

Pinned by existing tests:
  - TestValidateChallenge_HappyPath_RS256
  - TestValidateChallenge_HappyPath_ES256_FixedWidth
  - TestValidateChallenge_HappyPath_ES256_DER
  - TestValidateChallenge_AlgNoneRejected
  - TestValidateChallenge_UnsupportedAlg
The happy-path tests would fail if the verifiers switched to any
non-SHA-256 digest — the alg allowlist makes the SHA-256 dependency
load-bearing, which the existing test suite already proves.

Verified locally:
  gofmt: clean.
  go vet ./internal/scep/intune/...: exit 0.
  go test -short -count=1 ./internal/scep/intune/...: PASS
    (every existing challenge_test.go subtest still green).

Reference: https://github.com/certctl-io/certctl/security/code-scanning/21
Closes CodeQL alert #21 as a documented false positive — the
//nolint annotations + RFC-citing comments are the load-bearing
suppression. Operators can dismiss the alert in the GitHub UI
with reason 'Won't fix' citing this commit.
This commit is contained in:
shankar0123
2026-05-04 05:08:02 +00:00
parent 586308ee71
commit ef5cf2e87a
+26 -2
View File
@@ -350,8 +350,20 @@ func verifyChallengeSignature(alg string, signingInput, signature []byte, trust
// signature against each trust anchor's public key. Constant-time: the
// stdlib's rsa.VerifyPKCS1v15 returns nil on success and an error on
// failure without timing-leak surface area on the hash compare path.
//
// SHA-256 is the spec-mandated digest for RS256 — RFC 7518 §3.3
// defines RS256 as "RSASSA-PKCS1-v1_5 using SHA-256". This is JWS
// signature verification over a public, well-known message (the
// JWS protected header + payload, base64url-encoded). It is NOT
// password hashing — the input has full 256-bit entropy contributed
// by the signer's nonce + timestamp + device-claim payload, and
// the output is checked against an asymmetric signature, not a
// pre-computed hash digest. CodeQL go/weak-sensitive-data-hashing
// triggers on the proximity of *x509.Certificate; the certificate
// here is a verification key, not an input to the hash. Suppressing
// the alert at the call site below.
func verifyRS256(signingInput, signature []byte, trust []*x509.Certificate) error {
h := sha256.Sum256(signingInput)
h := sha256.Sum256(signingInput) //nolint:gosec // RFC 7518 §3.3 RS256 mandates SHA-256; not password hashing
for _, cert := range trust {
pub, ok := cert.PublicKey.(*rsa.PublicKey)
if !ok {
@@ -376,8 +388,20 @@ func verifyRS256(signingInput, signature []byte, trust []*x509.Certificate) erro
// Try fixed-width first (the spec-blessed format); fall back to ASN.1.
// crypto/ecdsa.VerifyASN1 + ecdsa.Verify both return bool — no timing
// leak on the success path.
//
// SHA-256 is the spec-mandated digest for ES256 — RFC 7518 §3.4 defines
// ES256 as "ECDSA using P-256 and SHA-256". This is JWS signature
// verification over a public, well-known message (the JWS protected
// header + payload, base64url-encoded). It is NOT password hashing.
// The signing input is the JWS encoded payload; full 256-bit-entropy
// content from the signer's claim. The output is checked against an
// asymmetric signature, not a pre-computed digest. CodeQL
// go/weak-sensitive-data-hashing triggers on the proximity of
// *x509.Certificate; the certificate here is a verification key, not
// an input to the hash. Suppressing the alert at the call site below
// (CodeQL alert #21).
func verifyES256(signingInput, signature []byte, trust []*x509.Certificate) error {
h := sha256.Sum256(signingInput)
h := sha256.Sum256(signingInput) //nolint:gosec // RFC 7518 §3.4 ES256 mandates SHA-256; not password hashing
for _, cert := range trust {
pub, ok := cert.PublicKey.(*ecdsa.PublicKey)
if !ok {