Fix F-07: pre-compute real Argon2 dummy hash via sync.Once
- auth/auth.go: add DummyHash() which uses sync.Once to compute
HashPassword("dummy-password-for-timing-only", DefaultArgonParams())
on first call; subsequent calls return the cached PHC string;
add sync to imports
- auth/auth_test.go: TestDummyHashIsValidPHC verifies the hash
parses and verifies correctly; TestDummyHashIsCached verifies
sync.Once behaviour; TestDummyHashMatchesDefaultParams verifies
embedded m/t/p match DefaultArgonParams()
- server/server.go, grpcserver/auth.go, ui/ui.go: replace five
hardcoded PHC strings with auth.DummyHash() calls
- AUDIT.md: mark F-07 as fixed
Security: the previous hardcoded hash used a 6-byte salt and
6-byte output ("testsalt"/"testhash" in base64), which Argon2id
verifies faster than a real 16-byte-salt / 32-byte-output hash.
This timing gap was measurable and could aid user enumeration.
auth.DummyHash() uses identical parameters and full-length salt
and output, so dummy verification timing matches real timing
exactly, regardless of future parameter changes.
This commit is contained in:
2
AUDIT.md
2
AUDIT.md
@@ -227,7 +227,7 @@ The REST `handleTokenIssue` and gRPC `IssueServiceToken` both revoke the existin
|
|||||||
| Yes | F-11 | MEDIUM | Missing security headers on UI responses | Small |
|
| Yes | F-11 | MEDIUM | Missing security headers on UI responses | Small |
|
||||||
| No | F-05 | LOW | No `nbf` claim in issued JWTs | Trivial |
|
| No | F-05 | LOW | No `nbf` claim in issued JWTs | Trivial |
|
||||||
| No | F-06 | LOW | `HasRole` uses non-constant-time comparison | Trivial |
|
| No | F-06 | LOW | `HasRole` uses non-constant-time comparison | Trivial |
|
||||||
| No | F-07 | LOW | Dummy Argon2 hash timing mismatch | Small |
|
| Yes | F-07 | LOW | Dummy Argon2 hash timing mismatch | Small |
|
||||||
| No | F-08 | LOW | No account lockout after repeated failures | Medium |
|
| No | F-08 | LOW | No account lockout after repeated failures | Medium |
|
||||||
| No | F-09 | LOW | `synchronous=NORMAL` risks audit data loss | Trivial |
|
| No | F-09 | LOW | `synchronous=NORMAL` risks audit data loss | Trivial |
|
||||||
| No | F-10 | LOW | No maximum token expiry validation | Small |
|
| No | F-10 | LOW | No maximum token expiry validation | Small |
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ import (
|
|||||||
"math"
|
"math"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"golang.org/x/crypto/argon2"
|
"golang.org/x/crypto/argon2"
|
||||||
@@ -54,6 +55,38 @@ func DefaultArgonParams() ArgonParams {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// dummyHashOnce ensures the dummy hash is computed exactly once at first use.
|
||||||
|
// Security: computing it lazily (rather than at init time) keeps startup fast;
|
||||||
|
// using sync.Once makes it safe to call from concurrent goroutines.
|
||||||
|
var (
|
||||||
|
dummyHashOnce sync.Once
|
||||||
|
dummyHashVal string
|
||||||
|
)
|
||||||
|
|
||||||
|
// DummyHash returns a pre-computed Argon2id PHC hash of a fixed dummy password
|
||||||
|
// using DefaultArgonParams. It is computed once on first call and cached.
|
||||||
|
//
|
||||||
|
// Security (F-07): using a real hash with the exact same parameters as
|
||||||
|
// production password verification ensures that dummy operations (run for
|
||||||
|
// unknown users or inactive accounts to prevent timing-based enumeration)
|
||||||
|
// take as long as real verifications, regardless of parameter changes.
|
||||||
|
// The previous hardcoded string used a 6-byte salt and 6-byte hash which
|
||||||
|
// was faster to verify than a real 16-byte-salt / 32-byte-hash record.
|
||||||
|
func DummyHash() string {
|
||||||
|
dummyHashOnce.Do(func() {
|
||||||
|
h, err := HashPassword("dummy-password-for-timing-only", DefaultArgonParams())
|
||||||
|
if err != nil {
|
||||||
|
// This should be unreachable in production — HashPassword only fails
|
||||||
|
// if crypto/rand fails or the password is empty, neither of which
|
||||||
|
// applies here. Panic so the misconfiguration surfaces immediately
|
||||||
|
// rather than silently degrading security.
|
||||||
|
panic("auth: DummyHash: failed to pre-compute dummy hash: " + err.Error())
|
||||||
|
}
|
||||||
|
dummyHashVal = h
|
||||||
|
})
|
||||||
|
return dummyHashVal
|
||||||
|
}
|
||||||
|
|
||||||
// HashPassword hashes a password using Argon2id and returns a PHC-format string.
|
// HashPassword hashes a password using Argon2id and returns a PHC-format string.
|
||||||
// A random 16-byte salt is generated via crypto/rand for each call.
|
// A random 16-byte salt is generated via crypto/rand for each call.
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -214,3 +214,51 @@ func TestDefaultArgonParams(t *testing.T) {
|
|||||||
t.Errorf("default Threads=%d < 1", p.Threads)
|
t.Errorf("default Threads=%d < 1", p.Threads)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDummyHashIsValidPHC verifies DummyHash returns a non-empty string that
|
||||||
|
// is a valid Argon2id PHC hash verifiable by VerifyPassword.
|
||||||
|
func TestDummyHashIsValidPHC(t *testing.T) {
|
||||||
|
h := DummyHash()
|
||||||
|
if h == "" {
|
||||||
|
t.Fatal("DummyHash returned empty string")
|
||||||
|
}
|
||||||
|
// Must parse as a valid PHC string with OWASP-compatible parameters.
|
||||||
|
// VerifyPassword with the correct password must succeed.
|
||||||
|
ok, err := VerifyPassword("dummy-password-for-timing-only", h)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("VerifyPassword against DummyHash: %v", err)
|
||||||
|
}
|
||||||
|
if !ok {
|
||||||
|
t.Error("VerifyPassword against DummyHash returned false (hash mismatch)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDummyHashIsCached verifies that consecutive calls return the same string
|
||||||
|
// (sync.Once caching).
|
||||||
|
func TestDummyHashIsCached(t *testing.T) {
|
||||||
|
h1 := DummyHash()
|
||||||
|
h2 := DummyHash()
|
||||||
|
if h1 != h2 {
|
||||||
|
t.Errorf("DummyHash returned different values on consecutive calls:\n first: %q\n second: %q", h1, h2)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDummyHashMatchesDefaultParams verifies the embedded parameters in the
|
||||||
|
// returned PHC string match DefaultArgonParams (m, t, p).
|
||||||
|
func TestDummyHashMatchesDefaultParams(t *testing.T) {
|
||||||
|
h := DummyHash()
|
||||||
|
params, _, _, err := parsePHC(h)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("parsePHC(DummyHash()): %v", err)
|
||||||
|
}
|
||||||
|
def := DefaultArgonParams()
|
||||||
|
if params.Memory != def.Memory {
|
||||||
|
t.Errorf("Memory = %d, want %d", params.Memory, def.Memory)
|
||||||
|
}
|
||||||
|
if params.Time != def.Time {
|
||||||
|
t.Errorf("Time = %d, want %d", params.Time, def.Time)
|
||||||
|
}
|
||||||
|
if params.Threads != def.Threads {
|
||||||
|
t.Errorf("Threads = %d, want %d", params.Threads, def.Threads)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -40,14 +40,14 @@ func (a *authServiceServer) Login(ctx context.Context, req *mciasv1.LoginRequest
|
|||||||
acct, err := a.s.db.GetAccountByUsername(req.Username)
|
acct, err := a.s.db.GetAccountByUsername(req.Username)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// Security: run dummy Argon2 to equalise timing for unknown users.
|
// Security: run dummy Argon2 to equalise timing for unknown users.
|
||||||
_, _ = auth.VerifyPassword("dummy", "$argon2id$v=19$m=65536,t=3,p=4$dGVzdHNhbHQ$dGVzdGhhc2g")
|
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
|
||||||
a.s.db.WriteAuditEvent(model.EventLoginFail, nil, nil, ip, //nolint:errcheck // audit failure is non-fatal
|
a.s.db.WriteAuditEvent(model.EventLoginFail, nil, nil, ip, //nolint:errcheck // audit failure is non-fatal
|
||||||
fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username))
|
fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username))
|
||||||
return nil, status.Error(codes.Unauthenticated, "invalid credentials")
|
return nil, status.Error(codes.Unauthenticated, "invalid credentials")
|
||||||
}
|
}
|
||||||
|
|
||||||
if acct.Status != model.AccountStatusActive {
|
if acct.Status != model.AccountStatusActive {
|
||||||
_, _ = auth.VerifyPassword("dummy", "$argon2id$v=19$m=65536,t=3,p=4$dGVzdHNhbHQ$dGVzdGhhc2g")
|
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
|
||||||
a.s.db.WriteAuditEvent(model.EventLoginFail, &acct.ID, nil, ip, `{"reason":"account_inactive"}`) //nolint:errcheck
|
a.s.db.WriteAuditEvent(model.EventLoginFail, &acct.ID, nil, ip, `{"reason":"account_inactive"}`) //nolint:errcheck
|
||||||
return nil, status.Error(codes.Unauthenticated, "invalid credentials")
|
return nil, status.Error(codes.Unauthenticated, "invalid credentials")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -174,7 +174,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
// Security: return a generic error whether the user exists or not.
|
// Security: return a generic error whether the user exists or not.
|
||||||
// Always run a dummy Argon2 check to prevent timing-based user enumeration.
|
// Always run a dummy Argon2 check to prevent timing-based user enumeration.
|
||||||
_, _ = auth.VerifyPassword("dummy", "$argon2id$v=19$m=65536,t=3,p=4$dGVzdHNhbHQ$dGVzdGhhc2g")
|
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
|
||||||
s.writeAudit(r, model.EventLoginFail, nil, nil, fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username))
|
s.writeAudit(r, model.EventLoginFail, nil, nil, fmt.Sprintf(`{"username":%q,"reason":"unknown_user"}`, req.Username))
|
||||||
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
|
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
|
||||||
return
|
return
|
||||||
@@ -183,7 +183,7 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
|
|||||||
// Security: Check account status before credential verification to avoid
|
// Security: Check account status before credential verification to avoid
|
||||||
// leaking whether the account exists based on timing differences.
|
// leaking whether the account exists based on timing differences.
|
||||||
if acct.Status != model.AccountStatusActive {
|
if acct.Status != model.AccountStatusActive {
|
||||||
_, _ = auth.VerifyPassword("dummy", "$argon2id$v=19$m=65536,t=3,p=4$dGVzdHNhbHQ$dGVzdGhhc2g")
|
_, _ = auth.VerifyPassword("dummy", auth.DummyHash())
|
||||||
s.writeAudit(r, model.EventLoginFail, &acct.ID, nil, `{"reason":"account_inactive"}`)
|
s.writeAudit(r, model.EventLoginFail, &acct.ID, nil, `{"reason":"account_inactive"}`)
|
||||||
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
|
middleware.WriteError(w, http.StatusUnauthorized, "invalid credentials", "unauthorized")
|
||||||
return
|
return
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import (
|
|||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"git.wntrmute.dev/kyle/mcias/internal/auth"
|
||||||
"git.wntrmute.dev/kyle/mcias/internal/config"
|
"git.wntrmute.dev/kyle/mcias/internal/config"
|
||||||
"git.wntrmute.dev/kyle/mcias/internal/db"
|
"git.wntrmute.dev/kyle/mcias/internal/db"
|
||||||
"git.wntrmute.dev/kyle/mcias/internal/model"
|
"git.wntrmute.dev/kyle/mcias/internal/model"
|
||||||
@@ -96,13 +97,11 @@ func (u *UIServer) consumeTOTPNonce(nonce string) (int64, bool) {
|
|||||||
return pl.accountID, true
|
return pl.accountID, true
|
||||||
}
|
}
|
||||||
|
|
||||||
// dummyHash returns a hardcoded Argon2id PHC string used for constant-time
|
// dummyHash returns the pre-computed Argon2id PHC hash for constant-time dummy
|
||||||
// dummy password verification when the account is unknown or inactive.
|
// verification when an account is unknown or inactive (F-07).
|
||||||
// Security: the dummy hash uses OWASP-recommended parameters (m=65536,t=3,p=4)
|
// Delegates to auth.DummyHash() which uses sync.Once for one-time computation.
|
||||||
// to match real verification timing. F-07 will replace this with a
|
|
||||||
// sync.Once-computed real hash for exact parameter parity.
|
|
||||||
func (u *UIServer) dummyHash() string {
|
func (u *UIServer) dummyHash() string {
|
||||||
return "$argon2id$v=19$m=65536,t=3,p=4$dGVzdHNhbHQ$dGVzdGhhc2g"
|
return auth.DummyHash()
|
||||||
}
|
}
|
||||||
|
|
||||||
// New constructs a UIServer, parses all templates, and returns it.
|
// New constructs a UIServer, parses all templates, and returns it.
|
||||||
|
|||||||
Reference in New Issue
Block a user