diff --git a/clients/go/client.go b/clients/go/client.go index 490d867..c414f93 100644 --- a/clients/go/client.go +++ b/clients/go/client.go @@ -401,9 +401,15 @@ func (c *Client) RenewToken() (token, expiresAt string, err error) { // Returns a base32 secret and an otpauth:// URI for QR-code generation. // The secret is shown once; it is not retrievable after this call. // TOTP is not enforced until confirmed via ConfirmTOTP. -func (c *Client) EnrollTOTP() (*TOTPEnrollResponse, error) { +// +// Security (SEC-01): the current password is required to prevent a stolen +// session token from being used to enroll attacker-controlled TOTP. +func (c *Client) EnrollTOTP(password string) (*TOTPEnrollResponse, error) { var resp TOTPEnrollResponse - if err := c.do(http.MethodPost, "/v1/auth/totp/enroll", nil, &resp); err != nil { + body := struct { + Password string `json:"password"` + }{Password: password} + if err := c.do(http.MethodPost, "/v1/auth/totp/enroll", body, &resp); err != nil { return nil, err } return &resp, nil diff --git a/clients/go/client_test.go b/clients/go/client_test.go index 5905fba..567e331 100644 --- a/clients/go/client_test.go +++ b/clients/go/client_test.go @@ -275,7 +275,7 @@ func TestEnrollTOTP(t *testing.T) { })) defer srv.Close() c := newTestClient(t, srv.URL) - resp, err := c.EnrollTOTP() + resp, err := c.EnrollTOTP("testpass123") if err != nil { t.Fatalf("EnrollTOTP: %v", err) } diff --git a/clients/python/mcias_client/_client.py b/clients/python/mcias_client/_client.py index de3f543..58d6ce8 100644 --- a/clients/python/mcias_client/_client.py +++ b/clients/python/mcias_client/_client.py @@ -148,11 +148,15 @@ class Client: expires_at = str(data["expires_at"]) self.token = token return token, expires_at - def enroll_totp(self) -> tuple[str, str]: + def enroll_totp(self, password: str) -> tuple[str, str]: """POST /v1/auth/totp/enroll — begin TOTP enrollment. + + Security (SEC-01): current password is required to prevent session-theft + escalation to persistent account takeover. + Returns (secret, otpauth_uri). The secret is shown only once. """ - data = self._request("POST", "/v1/auth/totp/enroll") + data = self._request("POST", "/v1/auth/totp/enroll", json={"password": password}) assert data is not None return str(data["secret"]), str(data["otpauth_uri"]) def confirm_totp(self, code: str) -> None: diff --git a/clients/python/tests/test_client.py b/clients/python/tests/test_client.py index 6c40a6b..968df12 100644 --- a/clients/python/tests/test_client.py +++ b/clients/python/tests/test_client.py @@ -191,7 +191,7 @@ def test_enroll_totp(admin_client: Client) -> None: json={"secret": "JBSWY3DPEHPK3PXP", "otpauth_uri": "otpauth://totp/MCIAS:alice?secret=JBSWY3DPEHPK3PXP&issuer=MCIAS"}, ) ) - secret, uri = admin_client.enroll_totp() + secret, uri = admin_client.enroll_totp("testpass123") assert secret == "JBSWY3DPEHPK3PXP" assert "otpauth://totp/" in uri @respx.mock diff --git a/clients/rust/src/lib.rs b/clients/rust/src/lib.rs index 349b884..28dccc5 100644 --- a/clients/rust/src/lib.rs +++ b/clients/rust/src/lib.rs @@ -484,9 +484,12 @@ impl Client { /// Begin TOTP enrollment. Returns `(secret, otpauth_uri)`. /// The secret is shown once; store it in an authenticator app immediately. - pub async fn enroll_totp(&self) -> Result<(String, String), MciasError> { + /// + /// Security (SEC-01): current password is required to prevent session-theft + /// escalation to persistent account takeover. + pub async fn enroll_totp(&self, password: &str) -> Result<(String, String), MciasError> { let resp: TotpEnrollResponse = - self.post("/v1/auth/totp/enroll", &serde_json::json!({})).await?; + self.post("/v1/auth/totp/enroll", &serde_json::json!({"password": password})).await?; Ok((resp.secret, resp.otpauth_uri)) } diff --git a/clients/rust/tests/client_tests.rs b/clients/rust/tests/client_tests.rs index 06a8980..243d740 100644 --- a/clients/rust/tests/client_tests.rs +++ b/clients/rust/tests/client_tests.rs @@ -449,7 +449,7 @@ async fn test_enroll_totp() { .await; let c = admin_client(&server).await; - let (secret, uri) = c.enroll_totp().await.unwrap(); + let (secret, uri) = c.enroll_totp("testpass123").await.unwrap(); assert_eq!(secret, "JBSWY3DPEHPK3PXP"); assert!(uri.starts_with("otpauth://totp/")); } diff --git a/gen/mcias/v1/auth.pb.go b/gen/mcias/v1/auth.pb.go index 9600107..4a99314 100644 --- a/gen/mcias/v1/auth.pb.go +++ b/gen/mcias/v1/auth.pb.go @@ -304,9 +304,12 @@ func (x *RenewTokenResponse) GetExpiresAt() *timestamppb.Timestamp { return nil } -// EnrollTOTPRequest carries no body; the acting account is from the JWT. +// EnrollTOTPRequest carries the current password for re-authentication. +// Security (SEC-01): password is required to prevent a stolen session token +// from being used to enroll attacker-controlled TOTP on the victim's account. type EnrollTOTPRequest struct { state protoimpl.MessageState `protogen:"open.v1"` + Password string `protobuf:"bytes,1,opt,name=password,proto3" json:"password,omitempty"` // security: current password required; never logged unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -341,6 +344,13 @@ func (*EnrollTOTPRequest) Descriptor() ([]byte, []int) { return file_mcias_v1_auth_proto_rawDescGZIP(), []int{6} } +func (x *EnrollTOTPRequest) GetPassword() string { + if x != nil { + return x.Password + } + return "" +} + // EnrollTOTPResponse returns the TOTP secret and otpauth URI for display. // Security: the secret is shown once; it is stored only in encrypted form. type EnrollTOTPResponse struct { @@ -578,8 +588,9 @@ const file_mcias_v1_auth_proto_rawDesc = "" + "\x12RenewTokenResponse\x12\x14\n" + "\x05token\x18\x01 \x01(\tR\x05token\x129\n" + "\n" + - "expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"\x13\n" + - "\x11EnrollTOTPRequest\"M\n" + + "expires_at\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\texpiresAt\"/\n" + + "\x11EnrollTOTPRequest\x12\x1a\n" + + "\bpassword\x18\x01 \x01(\tR\bpassword\"M\n" + "\x12EnrollTOTPResponse\x12\x16\n" + "\x06secret\x18\x01 \x01(\tR\x06secret\x12\x1f\n" + "\votpauth_uri\x18\x02 \x01(\tR\n" + diff --git a/internal/grpcserver/auth.go b/internal/grpcserver/auth.go index 64a7cf7..2c219d8 100644 --- a/internal/grpcserver/auth.go +++ b/internal/grpcserver/auth.go @@ -195,13 +195,39 @@ func (a *authServiceServer) RenewToken(ctx context.Context, _ *mciasv1.RenewToke } // EnrollTOTP begins TOTP enrollment for the calling account. -func (a *authServiceServer) EnrollTOTP(ctx context.Context, _ *mciasv1.EnrollTOTPRequest) (*mciasv1.EnrollTOTPResponse, error) { +// +// Security (SEC-01): the current password is required to prevent a stolen +// session token from being used to enroll attacker-controlled TOTP on the +// victim's account. Lockout is checked and failures are recorded. +func (a *authServiceServer) EnrollTOTP(ctx context.Context, req *mciasv1.EnrollTOTPRequest) (*mciasv1.EnrollTOTPResponse, error) { claims := claimsFromContext(ctx) acct, err := a.s.db.GetAccountByUUID(claims.Subject) if err != nil { return nil, status.Error(codes.Unauthenticated, "account not found") } + if req.Password == "" { + return nil, status.Error(codes.InvalidArgument, "password is required") + } + + // Security: check lockout before verifying (same as login flow). + locked, lockErr := a.s.db.IsLockedOut(acct.ID) + if lockErr != nil { + a.s.logger.Error("lockout check (gRPC TOTP enroll)", "error", lockErr) + } + if locked { + a.s.db.WriteAuditEvent(model.EventTOTPEnrolled, &acct.ID, &acct.ID, peerIP(ctx), `{"result":"locked"}`) //nolint:errcheck + return nil, status.Error(codes.ResourceExhausted, "account temporarily locked") + } + + // Security: verify the current password with Argon2id (constant-time). + ok, verifyErr := auth.VerifyPassword(req.Password, acct.PasswordHash) + if verifyErr != nil || !ok { + _ = a.s.db.RecordLoginFailure(acct.ID) + a.s.db.WriteAuditEvent(model.EventTOTPEnrolled, &acct.ID, &acct.ID, peerIP(ctx), `{"result":"wrong_password"}`) //nolint:errcheck + return nil, status.Error(codes.Unauthenticated, "password is incorrect") + } + rawSecret, b32Secret, err := auth.GenerateTOTPSecret() if err != nil { return nil, status.Error(codes.Internal, "internal error") diff --git a/internal/server/server.go b/internal/server/server.go index aa135a1..ab7368a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -780,6 +780,10 @@ func (s *Server) handleRevokeRole(w http.ResponseWriter, r *http.Request) { // ---- TOTP endpoints ---- +type totpEnrollRequest struct { + Password string `json:"password"` // security: current password required to prevent session-theft escalation +} + type totpEnrollResponse struct { Secret string `json:"secret"` // base32-encoded OTPAuthURI string `json:"otpauth_uri"` @@ -789,6 +793,12 @@ type totpConfirmRequest struct { Code string `json:"code"` } +// handleTOTPEnroll begins TOTP enrollment for the calling account. +// +// Security (SEC-01): the current password is required in the request body to +// prevent a stolen session token from being used to enroll attacker-controlled +// MFA on the victim's account. Lockout is checked and failures are recorded +// to prevent brute-force use of this endpoint as a password oracle. func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) { claims := middleware.ClaimsFromContext(r.Context()) acct, err := s.db.GetAccountByUUID(claims.Subject) @@ -797,6 +807,38 @@ func (s *Server) handleTOTPEnroll(w http.ResponseWriter, r *http.Request) { return } + var req totpEnrollRequest + if !decodeJSON(w, r, &req) { + return + } + + if req.Password == "" { + middleware.WriteError(w, http.StatusBadRequest, "password is required", "bad_request") + return + } + + // Security: check lockout before verifying (same as login and password-change flows) + // so an attacker cannot use this endpoint to brute-force the current password. + locked, lockErr := s.db.IsLockedOut(acct.ID) + if lockErr != nil { + s.logger.Error("lockout check (TOTP enroll)", "error", lockErr) + } + if locked { + s.writeAudit(r, model.EventTOTPEnrolled, &acct.ID, &acct.ID, `{"result":"locked"}`) + middleware.WriteError(w, http.StatusTooManyRequests, "account temporarily locked", "account_locked") + return + } + + // Security: verify the current password with the same constant-time + // Argon2id path used at login to prevent timing oracles. + ok, verifyErr := auth.VerifyPassword(req.Password, acct.PasswordHash) + if verifyErr != nil || !ok { + _ = s.db.RecordLoginFailure(acct.ID) + s.writeAudit(r, model.EventTOTPEnrolled, &acct.ID, &acct.ID, `{"result":"wrong_password"}`) + middleware.WriteError(w, http.StatusUnauthorized, "password is incorrect", "unauthorized") + return + } + rawSecret, b32Secret, err := auth.GenerateTOTPSecret() if err != nil { middleware.WriteError(w, http.StatusInternalServerError, "internal error", "internal_error") diff --git a/internal/server/server_test.go b/internal/server/server_test.go index f7325fe..3afc5ea 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -519,8 +519,10 @@ func TestTOTPEnrollDoesNotRequireTOTP(t *testing.T) { t.Fatalf("TrackToken: %v", err) } - // Start enrollment. - rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", nil, tokenStr) + // Start enrollment (password required since SEC-01 fix). + rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{ + Password: "testpass123", + }, tokenStr) if rr.Code != http.StatusOK { t.Fatalf("enroll status = %d, want 200; body: %s", rr.Code, rr.Body.String()) } @@ -558,6 +560,61 @@ func TestTOTPEnrollDoesNotRequireTOTP(t *testing.T) { } } +// TestTOTPEnrollRequiresPassword verifies that TOTP enrollment (SEC-01) +// requires the current password. A stolen session token alone must not be +// sufficient to add attacker-controlled MFA to the victim's account. +func TestTOTPEnrollRequiresPassword(t *testing.T) { + srv, _, priv, _ := newTestServer(t) + acct := createTestHumanAccount(t, srv, "totp-pw-check") + handler := srv.Handler() + + tokenStr, claims, err := token.IssueToken(priv, testIssuer, acct.UUID, nil, time.Hour) + if err != nil { + t.Fatalf("IssueToken: %v", err) + } + if err := srv.db.TrackToken(claims.JTI, acct.ID, claims.IssuedAt, claims.ExpiresAt); err != nil { + t.Fatalf("TrackToken: %v", err) + } + + t.Run("no password", func(t *testing.T) { + rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{}, tokenStr) + if rr.Code != http.StatusBadRequest { + t.Errorf("enroll without password: status = %d, want %d; body: %s", + rr.Code, http.StatusBadRequest, rr.Body.String()) + } + }) + + t.Run("wrong password", func(t *testing.T) { + rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{ + Password: "wrong-password", + }, tokenStr) + if rr.Code != http.StatusUnauthorized { + t.Errorf("enroll with wrong password: status = %d, want %d; body: %s", + rr.Code, http.StatusUnauthorized, rr.Body.String()) + } + }) + + t.Run("correct password", func(t *testing.T) { + rr := doRequest(t, handler, "POST", "/v1/auth/totp/enroll", totpEnrollRequest{ + Password: "testpass123", + }, tokenStr) + if rr.Code != http.StatusOK { + t.Fatalf("enroll with correct password: status = %d, want 200; body: %s", + rr.Code, rr.Body.String()) + } + var resp totpEnrollResponse + if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if resp.Secret == "" { + t.Error("expected non-empty TOTP secret") + } + if resp.OTPAuthURI == "" { + t.Error("expected non-empty otpauth URI") + } + }) +} + func TestRenewToken(t *testing.T) { srv, _, priv, _ := newTestServer(t) acct := createTestHumanAccount(t, srv, "renew-user") diff --git a/openapi.yaml b/openapi.yaml index da17178..4fb3ef3 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -550,6 +550,17 @@ paths: tags: [Auth] security: - bearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [password] + properties: + password: + type: string + description: Current account password (required to prevent session-theft escalation). responses: "200": description: TOTP secret generated. diff --git a/proto/mcias/v1/auth.proto b/proto/mcias/v1/auth.proto index 48275ed..ec05c23 100644 --- a/proto/mcias/v1/auth.proto +++ b/proto/mcias/v1/auth.proto @@ -45,8 +45,12 @@ message RenewTokenResponse { // --- TOTP enrollment --- -// EnrollTOTPRequest carries no body; the acting account is from the JWT. -message EnrollTOTPRequest {} +// EnrollTOTPRequest carries the current password for re-authentication. +// Security (SEC-01): password is required to prevent a stolen session token +// from being used to enroll attacker-controlled TOTP on the victim's account. +message EnrollTOTPRequest { + string password = 1; // security: current password required; never logged +} // EnrollTOTPResponse returns the TOTP secret and otpauth URI for display. // Security: the secret is shown once; it is stored only in encrypted form. diff --git a/web/static/openapi.yaml b/web/static/openapi.yaml index 3e10ff5..6417e69 100644 --- a/web/static/openapi.yaml +++ b/web/static/openapi.yaml @@ -435,6 +435,17 @@ paths: tags: [Auth] security: - bearerAuth: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [password] + properties: + password: + type: string + description: Current account password (required to prevent session-theft escalation). responses: "200": description: TOTP secret generated.