DEV: Use constant-time signature comparison for DiscourseConnect HMAC validation (#42424)

## Summary

Replace byte-by-byte string equality comparisons with constant-time
`ActiveSupport::SecurityUtils.secure_compare` when validating HMAC
signatures at both DiscourseConnect boundaries — the consumer-facing
base parser and the provider-secret lookup. The provided signature is
coerced to a string before comparison so missing or malformed values
fail validation cleanly rather than raising, and no behavioral or flow
changes are made to the authentication endpoints.

## Source

- Patch Triage: https://patch.discourse.org/patch-triage/1395

Co-authored-by: discourse-patch-triage
<272280883+discourse-patch-triage[bot]@users.noreply.github.com>
This commit is contained in:
Osama Sayegh
2026-08-07 08:12:32 +03:00
committed by GitHub
parent 183ef04c1d
commit 1ff8a8cc12
2 changed files with 10 additions and 3 deletions
+6 -2
View File
@@ -103,15 +103,19 @@ class DiscourseConnectBase
decoded = Base64.decode64(parsed["sso"])
decoded_hash = Rack::Utils.parse_query(decoded)
raise SignatureError, <<~MSG if sso.sign(parsed["sso"]) != parsed["sig"]
expected_sig = sso.sign(parsed["sso"])
if !ActiveSupport::SecurityUtils.secure_compare(expected_sig, parsed["sig"].to_s)
raise SignatureError, <<~MSG
Bad signature for payload
sso: #{parsed["sso"]}
sig: #{parsed["sig"]}
expected sig: #{sso.sign(parsed["sso"])}
expected sig: #{expected_sig}
MSG
end
ACCESSORS.each do |k|
val = decoded_hash[k.to_s]
+4 -1
View File
@@ -76,7 +76,10 @@ class DiscourseConnectProvider < DiscourseConnectBase
provider_secrets.find do |domain, configured_secret|
if WildcardDomainChecker.check_domain(domain, return_url_host)
first_domain_match ||= configured_secret
sign(parsed_payload["sso"], configured_secret) == parsed_payload["sig"]
ActiveSupport::SecurityUtils.secure_compare(
sign(parsed_payload["sso"], configured_secret),
parsed_payload["sig"].to_s,
)
end
end