refactor(passkeys): one sign-count check, one client key, the name given at begin
- refuse a counter that does not advance in the use case only: the adapter returns the counter the library kept, ErrPasskeyCloned is gone, and the refusal logs one warning naming the passkey - count pending logins under clientlimit.Key, the key the failure limiter uses, and drop the use case's own copy - keep the checked passkey name in the registration ceremony: FinishRegistration and the finish body no longer carry it, and it returns only an error - WebPasskeyService embeds PasskeyService, so *usecase.Passkeys satisfies it without a FinishRegistration wrapper - read both finish bodies with one helper - one domain.PasskeyCeremonyTTL for the adapter and the use case - drop the adapter's credential id length check, hasOldestSeq and PasskeyName.IsZero
- commit
- 005cbdae3c6f0b63b938f217e8c536dc3f84b822
- author
- brice · · 2026-10-12 16:18 UTC
- parent
- 79312cb
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
13 files · +467 -383
M internal/adapters/web/passkeys.go +23 -18
| @@ -150,7 +150,7 @@ func (s *Server) passkeyLoginBegin(w http.ResponseWriter, r *http.Request) { | |||
| 150 | 150 | } | |
| 151 | 151 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | |
| 152 | 152 | defer cancel() | |
| 153 | - | ceremony, options, err := s.cfg.Passkeys.BeginLogin(ctx, addr) | |
| 153 | + | ceremony, options, err := s.cfg.Passkeys.BeginLogin(ctx, clientlimit.Key(addr)) | |
| 154 | 154 | if err != nil { | |
| 155 | 155 | s.jsonInternal(ctx, w, r, err) | |
| 156 | 156 | return | |
| @@ -158,13 +158,30 @@ func (s *Server) passkeyLoginBegin(w http.ResponseWriter, r *http.Request) { | |||
| 158 | 158 | jsonReply(w, http.StatusOK, ceremonyReply{Ceremony: ceremony, Options: options}) | |
| 159 | 159 | } | |
| 160 | 160 | ||
| 161 | - | // finishRequest is the body of a finish. | |
| 161 | + | // finishRequest is the body of a finish. The name of a new passkey is given at | |
| 162 | + | // the begin of its registration, not here. | |
| 162 | 163 | type finishRequest struct { | |
| 163 | 164 | Ceremony string `json:"ceremony"` | |
| 164 | - | Name string `json:"name"` | |
| 165 | 165 | Credential json.RawMessage `json:"credential"` | |
| 166 | 166 | } | |
| 167 | 167 | ||
| 168 | + | // readFinish reads the body of a finish: JSON, bounded, with a ceremony and a | |
| 169 | + | // credential. It answers 415, 413 or 400 and returns false when it cannot. A | |
| 170 | + | // body that fails here is a bug of the client, not a guess at a passkey: the | |
| 171 | + | // callers do not count it as a failed sign-in. | |
| 172 | + | func readFinish(w http.ResponseWriter, r *http.Request) (finishRequest, bool) { | |
| 173 | + | body, ok := jsonBody(w, r) | |
| 174 | + | if !ok { | |
| 175 | + | return finishRequest{}, false | |
| 176 | + | } | |
| 177 | + | var req finishRequest | |
| 178 | + | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |
| 179 | + | jsonError(w, http.StatusBadRequest, "malformed request") | |
| 180 | + | return finishRequest{}, false | |
| 181 | + | } | |
| 182 | + | return req, true | |
| 183 | + | } | |
| 184 | + | ||
| 168 | 185 | // passkeyLoginFinish is POST /-/passkeys/login/finish: it checks the response | |
| 169 | 186 | // of the authenticator and sets the session cookie, the same as a login link. | |
| 170 | 187 | // A refusal counts as a failed authentication of the client, whatever the | |
| @@ -175,17 +192,10 @@ func (s *Server) passkeyLoginFinish(w http.ResponseWriter, r *http.Request) { | |||
| 175 | 192 | if blocked { | |
| 176 | 193 | return | |
| 177 | 194 | } | |
| 178 | - | // A wrong content type or a malformed body is a bug of the client, not a | |
| 179 | - | // guess at the owner's passkey: it answers 4xx and is not counted as a failure. | |
| 180 | - | body, ok := jsonBody(w, r) | |
| 195 | + | req, ok := readFinish(w, r) | |
| 181 | 196 | if !ok { | |
| 182 | 197 | return | |
| 183 | 198 | } | |
| 184 | - | var req finishRequest | |
| 185 | - | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |
| 186 | - | jsonError(w, http.StatusBadRequest, "malformed request") | |
| 187 | - | return | |
| 188 | - | } | |
| 189 | 199 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | |
| 190 | 200 | defer cancel() | |
| 191 | 201 | token, lifetime, err := s.cfg.Passkeys.FinishLogin(ctx, req.Ceremony, req.Credential) | |
| @@ -286,18 +296,13 @@ func (s *Server) passkeyRegisterBegin(w http.ResponseWriter, r *http.Request) { | |||
| 286 | 296 | ||
| 287 | 297 | // passkeyRegisterFinish is POST /-/passkeys/register/finish. | |
| 288 | 298 | func (s *Server) passkeyRegisterFinish(w http.ResponseWriter, r *http.Request) { | |
| 289 | - | body, ok := jsonBody(w, r) | |
| 299 | + | req, ok := readFinish(w, r) | |
| 290 | 300 | if !ok { | |
| 291 | 301 | return | |
| 292 | 302 | } | |
| 293 | - | var req finishRequest | |
| 294 | - | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |
| 295 | - | jsonError(w, http.StatusBadRequest, "malformed request") | |
| 296 | - | return | |
| 297 | - | } | |
| 298 | 303 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | |
| 299 | 304 | defer cancel() | |
| 300 | - | if err := s.cfg.Passkeys.FinishRegistration(ctx, principal(ctx), req.Ceremony, req.Name, req.Credential); err != nil { | |
| 305 | + | if err := s.cfg.Passkeys.FinishRegistration(ctx, principal(ctx), req.Ceremony, req.Credential); err != nil { | |
| 301 | 306 | s.registerError(ctx, w, r, err) | |
| 302 | 307 | return | |
| 303 | 308 | } | |
| @@ -150,7 +150,7 @@ func (s *Server) passkeyLoginBegin(w http.ResponseWriter, r *http.Request) { | |||||
| 150 | } | 150 | } | ||
| 151 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | 151 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | ||
| 152 | defer cancel() | 152 | defer cancel() | ||
| 153 | - | ceremony, options, err := s.cfg.Passkeys.BeginLogin(ctx, addr) | 153 | + | ceremony, options, err := s.cfg.Passkeys.BeginLogin(ctx, clientlimit.Key(addr)) |
| 154 | if err != nil { | 154 | if err != nil { | ||
| 155 | s.jsonInternal(ctx, w, r, err) | 155 | s.jsonInternal(ctx, w, r, err) | ||
| 156 | return | 156 | return | ||
| @@ -158,13 +158,30 @@ func (s *Server) passkeyLoginBegin(w http.ResponseWriter, r *http.Request) { | |||||
| 158 | jsonReply(w, http.StatusOK, ceremonyReply{Ceremony: ceremony, Options: options}) | 158 | jsonReply(w, http.StatusOK, ceremonyReply{Ceremony: ceremony, Options: options}) | ||
| 159 | } | 159 | } | ||
| 160 | 160 | ||||
| 161 | - | // finishRequest is the body of a finish. | 161 | + | // finishRequest is the body of a finish. The name of a new passkey is given at |
| 162 | + | // the begin of its registration, not here. | |||
| 162 | type finishRequest struct { | 163 | type finishRequest struct { | ||
| 163 | Ceremony string `json:"ceremony"` | 164 | Ceremony string `json:"ceremony"` | ||
| 164 | - | Name string `json:"name"` | |||
| 165 | Credential json.RawMessage `json:"credential"` | 165 | Credential json.RawMessage `json:"credential"` | ||
| 166 | } | 166 | } | ||
| 167 | 167 | ||||
| 168 | + | // readFinish reads the body of a finish: JSON, bounded, with a ceremony and a | |||
| 169 | + | // credential. It answers 415, 413 or 400 and returns false when it cannot. A | |||
| 170 | + | // body that fails here is a bug of the client, not a guess at a passkey: the | |||
| 171 | + | // callers do not count it as a failed sign-in. | |||
| 172 | + | func readFinish(w http.ResponseWriter, r *http.Request) (finishRequest, bool) { | |||
| 173 | + | body, ok := jsonBody(w, r) | |||
| 174 | + | if !ok { | |||
| 175 | + | return finishRequest{}, false | |||
| 176 | + | } | |||
| 177 | + | var req finishRequest | |||
| 178 | + | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |||
| 179 | + | jsonError(w, http.StatusBadRequest, "malformed request") | |||
| 180 | + | return finishRequest{}, false | |||
| 181 | + | } | |||
| 182 | + | return req, true | |||
| 183 | + | } | |||
| 184 | + | ||||
| 168 | // passkeyLoginFinish is POST /-/passkeys/login/finish: it checks the response | 185 | // passkeyLoginFinish is POST /-/passkeys/login/finish: it checks the response | ||
| 169 | // of the authenticator and sets the session cookie, the same as a login link. | 186 | // of the authenticator and sets the session cookie, the same as a login link. | ||
| 170 | // A refusal counts as a failed authentication of the client, whatever the | 187 | // A refusal counts as a failed authentication of the client, whatever the | ||
| @@ -175,17 +192,10 @@ func (s *Server) passkeyLoginFinish(w http.ResponseWriter, r *http.Request) { | |||||
| 175 | if blocked { | 192 | if blocked { | ||
| 176 | return | 193 | return | ||
| 177 | } | 194 | } | ||
| 178 | - | // A wrong content type or a malformed body is a bug of the client, not a | 195 | + | req, ok := readFinish(w, r) |
| 179 | - | // guess at the owner's passkey: it answers 4xx and is not counted as a failure. | |||
| 180 | - | body, ok := jsonBody(w, r) | |||
| 181 | if !ok { | 196 | if !ok { | ||
| 182 | return | 197 | return | ||
| 183 | } | 198 | } | ||
| 184 | - | var req finishRequest | |||
| 185 | - | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |||
| 186 | - | jsonError(w, http.StatusBadRequest, "malformed request") | |||
| 187 | - | return | |||
| 188 | - | } | |||
| 189 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | 199 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | ||
| 190 | defer cancel() | 200 | defer cancel() | ||
| 191 | token, lifetime, err := s.cfg.Passkeys.FinishLogin(ctx, req.Ceremony, req.Credential) | 201 | token, lifetime, err := s.cfg.Passkeys.FinishLogin(ctx, req.Ceremony, req.Credential) | ||
| @@ -286,18 +296,13 @@ func (s *Server) passkeyRegisterBegin(w http.ResponseWriter, r *http.Request) { | |||||
| 286 | 296 | ||||
| 287 | // passkeyRegisterFinish is POST /-/passkeys/register/finish. | 297 | // passkeyRegisterFinish is POST /-/passkeys/register/finish. | ||
| 288 | func (s *Server) passkeyRegisterFinish(w http.ResponseWriter, r *http.Request) { | 298 | func (s *Server) passkeyRegisterFinish(w http.ResponseWriter, r *http.Request) { | ||
| 289 | - | body, ok := jsonBody(w, r) | 299 | + | req, ok := readFinish(w, r) |
| 290 | if !ok { | 300 | if !ok { | ||
| 291 | return | 301 | return | ||
| 292 | } | 302 | } | ||
| 293 | - | var req finishRequest | |||
| 294 | - | if err := json.Unmarshal(body, &req); err != nil || req.Ceremony == "" || len(req.Credential) == 0 { | |||
| 295 | - | jsonError(w, http.StatusBadRequest, "malformed request") | |||
| 296 | - | return | |||
| 297 | - | } | |||
| 298 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | 303 | ctx, cancel := context.WithTimeout(r.Context(), requestTimeout) | ||
| 299 | defer cancel() | 304 | defer cancel() | ||
| 300 | - | if err := s.cfg.Passkeys.FinishRegistration(ctx, principal(ctx), req.Ceremony, req.Name, req.Credential); err != nil { | 305 | + | if err := s.cfg.Passkeys.FinishRegistration(ctx, principal(ctx), req.Ceremony, req.Credential); err != nil { |
| 301 | s.registerError(ctx, w, r, err) | 306 | s.registerError(ctx, w, r, err) | ||
| 302 | return | 307 | return | ||
| 303 | } | 308 | } | ||
M internal/adapters/web/passkeys_test.go +56 -38
| @@ -54,10 +54,15 @@ func newPasskeyServer(t *testing.T) *passkeyFixture { | |||
| 54 | 54 | f.srv = newTestServer(t, Config{ | |
| 55 | 55 | Repos: reposFailing(t, errors.New("x")), Browse: mocks.NewBrowseService(t), | |
| 56 | 56 | PublicURL: forgeOrigin, Sessions: f.sessions, Passkeys: f.passkeys, Limiter: f.lim, | |
| 57 | + | Now: func() time.Time { return fixedNow }, | |
| 57 | 58 | }) | |
| 58 | 59 | return f | |
| 59 | 60 | } | |
| 60 | 61 | ||
| 62 | + | // fixedNow is the clock of the passkey pages: "3d ago" does not depend on when | |
| 63 | + | // the test runs. | |
| 64 | + | var fixedNow = time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC) | |
| 65 | + | ||
| 61 | 66 | type req struct { | |
| 62 | 67 | method, target string | |
| 63 | 68 | origin string // "" sends none | |
| @@ -88,7 +93,7 @@ func postJSON(target, cookie, body string) req { | |||
| 88 | 93 | return req{method: "POST", target: target, origin: forgeOrigin, cookie: cookie, contentType: "application/json", body: body} | |
| 89 | 94 | } | |
| 90 | 95 | ||
| 91 | - | const finishBody = `{"ceremony":"c1","name":"Pixel 9","credential":{"id":"x","type":"public-key"}}` | |
| 96 | + | const finishBody = `{"ceremony":"c1","credential":{"id":"x","type":"public-key"}}` | |
| 92 | 97 | ||
| 93 | 98 | var ( | |
| 94 | 99 | beginLogin = "/-/passkeys/login/begin" | |
| @@ -160,12 +165,17 @@ func TestPasskeyLoginBlocked(t *testing.T) { | |||
| 160 | 165 | t.Parallel() | |
| 161 | 166 | f := newPasskeyServer(t) // no use case call is expected | |
| 162 | 167 | f.lim.blocked = true | |
| 168 | + | // Blocked answers 429 whatever the request holds: the limiter comes before | |
| 169 | + | // the content type and the body are looked at. | |
| 163 | 170 | for _, r := range []req{ | |
| 164 | 171 | {method: "POST", target: beginLogin, origin: forgeOrigin}, | |
| 165 | 172 | postJSON(finishLogin, "", finishBody), | |
| 173 | + | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | |
| 174 | + | postJSON(finishLogin, "", `not json`), | |
| 175 | + | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |
| 166 | 176 | } { | |
| 167 | 177 | rec := f.do(t, r) | |
| 168 | - | assert.Equal(t, http.StatusTooManyRequests, rec.Code, r.target) | |
| 178 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "%s %q", r.target, r.contentType) | |
| 169 | 179 | assert.JSONEq(t, `{"error":"too many attempts"}`, rec.Body.String()) | |
| 170 | 180 | assert.Equal(t, "91", rec.Header().Get("Retry-After")) | |
| 171 | 181 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | |
| @@ -277,7 +287,6 @@ func TestPasskeyFinishBodyLimit(t *testing.T) { | |||
| 277 | 287 | require.Len(t, exact, 64<<10) | |
| 278 | 288 | rec := f.do(t, postJSON(finishLogin, "", exact)) | |
| 279 | 289 | assert.Equal(t, http.StatusBadRequest, rec.Code) | |
| 280 | - | assert.Zero(t, f.lim.fails, "a malformed body is not a failed authentication") | |
| 281 | 290 | } | |
| 282 | 291 | ||
| 283 | 292 | func TestPasskeyFinishMalformed(t *testing.T) { | |
| @@ -301,7 +310,7 @@ func TestPasskeyRegistration(t *testing.T) { | |||
| 301 | 310 | assert.JSONEq(t, `{"ceremony":"cer9","options":{"publicKey":{}}}`, rec.Body.String()) | |
| 302 | 311 | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | |
| 303 | 312 | ||
| 304 | - | f.passkeys.EXPECT().FinishRegistration(mock.Anything, domain.Owner(), "c1", "Pixel 9", mock.MatchedBy(func(b []byte) bool { | |
| 313 | + | f.passkeys.EXPECT().FinishRegistration(mock.Anything, domain.Owner(), "c1", mock.MatchedBy(func(b []byte) bool { | |
| 305 | 314 | return string(b) == `{"id":"x","type":"public-key"}` | |
| 306 | 315 | })).Return(nil).Once() | |
| 307 | 316 | rec = f.do(t, postJSON(finishRegister, "tok", finishBody)) | |
| @@ -327,7 +336,7 @@ func TestPasskeyRegistrationErrors(t *testing.T) { | |||
| 327 | 336 | t.Run(name, func(t *testing.T) { | |
| 328 | 337 | t.Parallel() | |
| 329 | 338 | f := newPasskeyServer(t) | |
| 330 | - | f.passkeys.EXPECT().FinishRegistration(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.err).Once() | |
| 339 | + | f.passkeys.EXPECT().FinishRegistration(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.err).Once() | |
| 331 | 340 | rec := f.do(t, postJSON(finishRegister, "tok", finishBody)) | |
| 332 | 341 | assert.Equal(t, tc.status, rec.Code) | |
| 333 | 342 | assert.Contains(t, rec.Body.String(), `"error"`) | |
| @@ -346,7 +355,7 @@ func TestPasskeyRegistrationErrors(t *testing.T) { | |||
| 346 | 355 | func TestPasskeysPage(t *testing.T) { | |
| 347 | 356 | t.Parallel() | |
| 348 | 357 | f := newPasskeyServer(t) | |
| 349 | - | now := time.Now() | |
| 358 | + | now := fixedNow | |
| 350 | 359 | list := []domain.Passkey{ | |
| 351 | 360 | {ID: []byte{0xfb, 0xff, 'a'}, Name: "Pixel <9>", CreatedAt: now.Add(-72 * time.Hour), LastUsedAt: now.Add(-time.Hour), BackupState: true}, | |
| 352 | 361 | {ID: []byte("hw"), Name: "yubikey", CreatedAt: now.Add(-time.Hour)}, | |
| @@ -364,9 +373,13 @@ func TestPasskeysPage(t *testing.T) { | |||
| 364 | 373 | assert.Contains(t, body, "never") | |
| 365 | 374 | assert.Contains(t, body, `action="/-/passkeys/`+base64.RawURLEncoding.EncodeToString(list[0].ID)+`/remove"`) | |
| 366 | 375 | assert.Contains(t, body, `data-passkey="register"`) | |
| 367 | - | // TS-004: the name field and the add button are one form, so Enter submits. | |
| 368 | - | form := body[strings.Index(body, "<form class=\"passkey-add\""):] | |
| 369 | - | form = form[:strings.Index(form, "</form>")] | |
| 376 | + | // The name field and the add button are one form, so Enter submits. | |
| 377 | + | start := strings.Index(body, "<form class=\"passkey-add\"") | |
| 378 | + | require.GreaterOrEqual(t, start, 0, "the add form is on the page") | |
| 379 | + | form := body[start:] | |
| 380 | + | end := strings.Index(form, "</form>") | |
| 381 | + | require.GreaterOrEqual(t, end, 0) | |
| 382 | + | form = form[:end] | |
| 370 | 383 | assert.Contains(t, form, "data-passkey-form") | |
| 371 | 384 | assert.Contains(t, form, `data-passkey-name`) | |
| 372 | 385 | assert.Contains(t, form, `type="submit"`) | |
| @@ -383,7 +396,7 @@ func TestPasskeysPageFull(t *testing.T) { | |||
| 383 | 396 | f := newPasskeyServer(t) | |
| 384 | 397 | list := make([]domain.Passkey, domain.MaxPasskeys) | |
| 385 | 398 | for i := range list { | |
| 386 | - | list[i] = domain.Passkey{ID: []byte{byte(i + 1)}, Name: "k", CreatedAt: time.Now()} | |
| 399 | + | list[i] = domain.Passkey{ID: []byte{byte(i + 1)}, Name: "k", CreatedAt: fixedNow} | |
| 387 | 400 | } | |
| 388 | 401 | f.passkeys.EXPECT().List(mock.Anything, mock.Anything).Return(list, nil).Once() | |
| 389 | 402 | rec := f.do(t, req{method: "GET", target: "/-/passkeys", cookie: "tok"}) | |
| @@ -554,50 +567,55 @@ func TestIsPasskeyLoginPath(t *testing.T) { | |||
| 554 | 567 | } | |
| 555 | 568 | } | |
| 556 | 569 | ||
| 557 | - | // GO-005: a blocked client gets 429 whatever it sends, and a malformed finish | |
| 558 | - | // is a 4xx that does not count as a failed authentication. | |
| 559 | - | func TestPasskeyLoginFinishBlockedBeforeBodyChecks(t *testing.T) { | |
| 570 | + | // A finish that cannot be read is a 4xx, and is not counted as a failed | |
| 571 | + | // sign-in: the client is buggy, it did not guess at a passkey. | |
| 572 | + | func TestMalformedFinishIsNotAFailedSignIn(t *testing.T) { | |
| 560 | 573 | t.Parallel() | |
| 561 | 574 | f := newPasskeyServer(t) | |
| 562 | - | f.lim.blocked = true | |
| 563 | - | for _, r := range []req{ | |
| 564 | - | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | |
| 565 | - | postJSON(finishLogin, "", `not json`), | |
| 566 | - | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |
| 567 | - | } { | |
| 568 | - | rec := f.do(t, r) | |
| 569 | - | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "%q", r.contentType) | |
| 570 | - | } | |
| 571 | - | assert.Zero(t, f.lim.fails) | |
| 572 | - | ||
| 573 | - | f.lim.blocked = false | |
| 574 | 575 | for _, r := range []req{ | |
| 575 | 576 | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | |
| 576 | 577 | postJSON(finishLogin, "", `not json`), | |
| 577 | 578 | postJSON(finishLogin, "", `{}`), | |
| 579 | + | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |
| 578 | 580 | } { | |
| 579 | 581 | rec := f.do(t, r) | |
| 580 | 582 | assert.GreaterOrEqual(t, rec.Code, 400) | |
| 581 | 583 | assert.Less(t, rec.Code, 500) | |
| 582 | 584 | } | |
| 583 | - | assert.Zero(t, f.lim.fails, "a malformed request is not a failed sign-in") | |
| 585 | + | assert.Zero(t, f.lim.fails) | |
| 584 | 586 | } | |
| 585 | 587 | ||
| 586 | - | // The client key the web hands to the use case is the one the limiter uses. | |
| 587 | - | func TestPasskeyLoginBeginPassesTheClientAddress(t *testing.T) { | |
| 588 | + | // The client key the web hands to the use case is the one the failure limiter | |
| 589 | + | // counts under: the address with trusted proxies applied, an IPv4 address as it | |
| 590 | + | // is (mapped or not), an IPv6 one cut to its /64. | |
| 591 | + | func TestPasskeyLoginBeginPassesTheClientKey(t *testing.T) { | |
| 588 | 592 | t.Parallel() | |
| 589 | - | f := newPasskeyServer(t) | |
| 590 | - | f.passkeys.EXPECT().BeginLogin(mock.Anything, "192.0.2.77").Return("c", []byte(`{}`), nil).Once() | |
| 591 | - | r := httptest.NewRequest("POST", beginLogin, nil) | |
| 592 | - | r.RemoteAddr = "192.0.2.77:51000" | |
| 593 | - | r.Header.Set("Origin", forgeOrigin) | |
| 594 | - | r.Header.Set("X-Forwarded-For", "203.0.113.5") // no trusted proxy: ignored | |
| 595 | - | rec := httptest.NewRecorder() | |
| 596 | - | f.srv.ServeHTTP(rec, r) | |
| 597 | - | assert.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) | |
| 593 | + | for name, tc := range map[string]struct{ remote, xff, want string }{ | |
| 594 | + | "ipv4": {"192.0.2.77:51000", "", "192.0.2.77"}, | |
| 595 | + | "ipv4 mapped": {"[::ffff:192.0.2.77]:51000", "", "192.0.2.77"}, | |
| 596 | + | "ipv6 host": {"[2001:db8:1:2::1]:51000", "", "2001:db8:1:2::/64"}, | |
| 597 | + | "ipv6 same /64": {"[2001:db8:1:2:ffff:ffff:ffff:ffff]:51000", "", "2001:db8:1:2::/64"}, | |
| 598 | + | "ipv6 another /64": {"[2001:db8:1:3::1]:51000", "", "2001:db8:1:3::/64"}, | |
| 599 | + | "forwarded for ignored": {"192.0.2.77:51000", "203.0.113.5", "192.0.2.77"}, | |
| 600 | + | } { | |
| 601 | + | t.Run(name, func(t *testing.T) { | |
| 602 | + | t.Parallel() | |
| 603 | + | f := newPasskeyServer(t) | |
| 604 | + | f.passkeys.EXPECT().BeginLogin(mock.Anything, tc.want).Return("c", []byte(`{}`), nil).Once() | |
| 605 | + | r := httptest.NewRequest("POST", beginLogin, nil) | |
| 606 | + | r.RemoteAddr = tc.remote | |
| 607 | + | r.Header.Set("Origin", forgeOrigin) | |
| 608 | + | if tc.xff != "" { | |
| 609 | + | r.Header.Set("X-Forwarded-For", tc.xff) // no trusted proxy: ignored | |
| 610 | + | } | |
| 611 | + | rec := httptest.NewRecorder() | |
| 612 | + | f.srv.ServeHTTP(rec, r) | |
| 613 | + | assert.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) | |
| 614 | + | }) | |
| 615 | + | } | |
| 598 | 616 | } | |
| 599 | 617 | ||
| 600 | - | // TS-001: the name travels with register/begin and is checked there. | |
| 618 | + | // The name travels with register/begin and is checked there. | |
| 601 | 619 | func TestPasskeyRegisterBeginChecksTheName(t *testing.T) { | |
| 602 | 620 | t.Parallel() | |
| 603 | 621 | f := newPasskeyServer(t) | |
| @@ -54,10 +54,15 @@ func newPasskeyServer(t *testing.T) *passkeyFixture { | |||||
| 54 | f.srv = newTestServer(t, Config{ | 54 | f.srv = newTestServer(t, Config{ | ||
| 55 | Repos: reposFailing(t, errors.New("x")), Browse: mocks.NewBrowseService(t), | 55 | Repos: reposFailing(t, errors.New("x")), Browse: mocks.NewBrowseService(t), | ||
| 56 | PublicURL: forgeOrigin, Sessions: f.sessions, Passkeys: f.passkeys, Limiter: f.lim, | 56 | PublicURL: forgeOrigin, Sessions: f.sessions, Passkeys: f.passkeys, Limiter: f.lim, | ||
| 57 | + | Now: func() time.Time { return fixedNow }, | |||
| 57 | }) | 58 | }) | ||
| 58 | return f | 59 | return f | ||
| 59 | } | 60 | } | ||
| 60 | 61 | ||||
| 62 | + | // fixedNow is the clock of the passkey pages: "3d ago" does not depend on when | |||
| 63 | + | // the test runs. | |||
| 64 | + | var fixedNow = time.Date(2026, 10, 12, 12, 0, 0, 0, time.UTC) | |||
| 65 | + | ||||
| 61 | type req struct { | 66 | type req struct { | ||
| 62 | method, target string | 67 | method, target string | ||
| 63 | origin string // "" sends none | 68 | origin string // "" sends none | ||
| @@ -88,7 +93,7 @@ func postJSON(target, cookie, body string) req { | |||||
| 88 | return req{method: "POST", target: target, origin: forgeOrigin, cookie: cookie, contentType: "application/json", body: body} | 93 | return req{method: "POST", target: target, origin: forgeOrigin, cookie: cookie, contentType: "application/json", body: body} | ||
| 89 | } | 94 | } | ||
| 90 | 95 | ||||
| 91 | - | const finishBody = `{"ceremony":"c1","name":"Pixel 9","credential":{"id":"x","type":"public-key"}}` | 96 | + | const finishBody = `{"ceremony":"c1","credential":{"id":"x","type":"public-key"}}` |
| 92 | 97 | ||||
| 93 | var ( | 98 | var ( | ||
| 94 | beginLogin = "/-/passkeys/login/begin" | 99 | beginLogin = "/-/passkeys/login/begin" | ||
| @@ -160,12 +165,17 @@ func TestPasskeyLoginBlocked(t *testing.T) { | |||||
| 160 | t.Parallel() | 165 | t.Parallel() | ||
| 161 | f := newPasskeyServer(t) // no use case call is expected | 166 | f := newPasskeyServer(t) // no use case call is expected | ||
| 162 | f.lim.blocked = true | 167 | f.lim.blocked = true | ||
| 168 | + | // Blocked answers 429 whatever the request holds: the limiter comes before | |||
| 169 | + | // the content type and the body are looked at. | |||
| 163 | for _, r := range []req{ | 170 | for _, r := range []req{ | ||
| 164 | {method: "POST", target: beginLogin, origin: forgeOrigin}, | 171 | {method: "POST", target: beginLogin, origin: forgeOrigin}, | ||
| 165 | postJSON(finishLogin, "", finishBody), | 172 | postJSON(finishLogin, "", finishBody), | ||
| 173 | + | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | |||
| 174 | + | postJSON(finishLogin, "", `not json`), | |||
| 175 | + | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |||
| 166 | } { | 176 | } { | ||
| 167 | rec := f.do(t, r) | 177 | rec := f.do(t, r) | ||
| 168 | - | assert.Equal(t, http.StatusTooManyRequests, rec.Code, r.target) | 178 | + | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "%s %q", r.target, r.contentType) |
| 169 | assert.JSONEq(t, `{"error":"too many attempts"}`, rec.Body.String()) | 179 | assert.JSONEq(t, `{"error":"too many attempts"}`, rec.Body.String()) | ||
| 170 | assert.Equal(t, "91", rec.Header().Get("Retry-After")) | 180 | assert.Equal(t, "91", rec.Header().Get("Retry-After")) | ||
| 171 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | 181 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | ||
| @@ -277,7 +287,6 @@ func TestPasskeyFinishBodyLimit(t *testing.T) { | |||||
| 277 | require.Len(t, exact, 64<<10) | 287 | require.Len(t, exact, 64<<10) | ||
| 278 | rec := f.do(t, postJSON(finishLogin, "", exact)) | 288 | rec := f.do(t, postJSON(finishLogin, "", exact)) | ||
| 279 | assert.Equal(t, http.StatusBadRequest, rec.Code) | 289 | assert.Equal(t, http.StatusBadRequest, rec.Code) | ||
| 280 | - | assert.Zero(t, f.lim.fails, "a malformed body is not a failed authentication") | |||
| 281 | } | 290 | } | ||
| 282 | 291 | ||||
| 283 | func TestPasskeyFinishMalformed(t *testing.T) { | 292 | func TestPasskeyFinishMalformed(t *testing.T) { | ||
| @@ -301,7 +310,7 @@ func TestPasskeyRegistration(t *testing.T) { | |||||
| 301 | assert.JSONEq(t, `{"ceremony":"cer9","options":{"publicKey":{}}}`, rec.Body.String()) | 310 | assert.JSONEq(t, `{"ceremony":"cer9","options":{"publicKey":{}}}`, rec.Body.String()) | ||
| 302 | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | 311 | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) | ||
| 303 | 312 | ||||
| 304 | - | f.passkeys.EXPECT().FinishRegistration(mock.Anything, domain.Owner(), "c1", "Pixel 9", mock.MatchedBy(func(b []byte) bool { | 313 | + | f.passkeys.EXPECT().FinishRegistration(mock.Anything, domain.Owner(), "c1", mock.MatchedBy(func(b []byte) bool { |
| 305 | return string(b) == `{"id":"x","type":"public-key"}` | 314 | return string(b) == `{"id":"x","type":"public-key"}` | ||
| 306 | })).Return(nil).Once() | 315 | })).Return(nil).Once() | ||
| 307 | rec = f.do(t, postJSON(finishRegister, "tok", finishBody)) | 316 | rec = f.do(t, postJSON(finishRegister, "tok", finishBody)) | ||
| @@ -327,7 +336,7 @@ func TestPasskeyRegistrationErrors(t *testing.T) { | |||||
| 327 | t.Run(name, func(t *testing.T) { | 336 | t.Run(name, func(t *testing.T) { | ||
| 328 | t.Parallel() | 337 | t.Parallel() | ||
| 329 | f := newPasskeyServer(t) | 338 | f := newPasskeyServer(t) | ||
| 330 | - | f.passkeys.EXPECT().FinishRegistration(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.err).Once() | 339 | + | f.passkeys.EXPECT().FinishRegistration(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.err).Once() |
| 331 | rec := f.do(t, postJSON(finishRegister, "tok", finishBody)) | 340 | rec := f.do(t, postJSON(finishRegister, "tok", finishBody)) | ||
| 332 | assert.Equal(t, tc.status, rec.Code) | 341 | assert.Equal(t, tc.status, rec.Code) | ||
| 333 | assert.Contains(t, rec.Body.String(), `"error"`) | 342 | assert.Contains(t, rec.Body.String(), `"error"`) | ||
| @@ -346,7 +355,7 @@ func TestPasskeyRegistrationErrors(t *testing.T) { | |||||
| 346 | func TestPasskeysPage(t *testing.T) { | 355 | func TestPasskeysPage(t *testing.T) { | ||
| 347 | t.Parallel() | 356 | t.Parallel() | ||
| 348 | f := newPasskeyServer(t) | 357 | f := newPasskeyServer(t) | ||
| 349 | - | now := time.Now() | 358 | + | now := fixedNow |
| 350 | list := []domain.Passkey{ | 359 | list := []domain.Passkey{ | ||
| 351 | {ID: []byte{0xfb, 0xff, 'a'}, Name: "Pixel <9>", CreatedAt: now.Add(-72 * time.Hour), LastUsedAt: now.Add(-time.Hour), BackupState: true}, | 360 | {ID: []byte{0xfb, 0xff, 'a'}, Name: "Pixel <9>", CreatedAt: now.Add(-72 * time.Hour), LastUsedAt: now.Add(-time.Hour), BackupState: true}, | ||
| 352 | {ID: []byte("hw"), Name: "yubikey", CreatedAt: now.Add(-time.Hour)}, | 361 | {ID: []byte("hw"), Name: "yubikey", CreatedAt: now.Add(-time.Hour)}, | ||
| @@ -364,9 +373,13 @@ func TestPasskeysPage(t *testing.T) { | |||||
| 364 | assert.Contains(t, body, "never") | 373 | assert.Contains(t, body, "never") | ||
| 365 | assert.Contains(t, body, `action="/-/passkeys/`+base64.RawURLEncoding.EncodeToString(list[0].ID)+`/remove"`) | 374 | assert.Contains(t, body, `action="/-/passkeys/`+base64.RawURLEncoding.EncodeToString(list[0].ID)+`/remove"`) | ||
| 366 | assert.Contains(t, body, `data-passkey="register"`) | 375 | assert.Contains(t, body, `data-passkey="register"`) | ||
| 367 | - | // TS-004: the name field and the add button are one form, so Enter submits. | 376 | + | // The name field and the add button are one form, so Enter submits. |
| 368 | - | form := body[strings.Index(body, "<form class=\"passkey-add\""):] | 377 | + | start := strings.Index(body, "<form class=\"passkey-add\"") |
| 369 | - | form = form[:strings.Index(form, "</form>")] | 378 | + | require.GreaterOrEqual(t, start, 0, "the add form is on the page") |
| 379 | + | form := body[start:] | |||
| 380 | + | end := strings.Index(form, "</form>") | |||
| 381 | + | require.GreaterOrEqual(t, end, 0) | |||
| 382 | + | form = form[:end] | |||
| 370 | assert.Contains(t, form, "data-passkey-form") | 383 | assert.Contains(t, form, "data-passkey-form") | ||
| 371 | assert.Contains(t, form, `data-passkey-name`) | 384 | assert.Contains(t, form, `data-passkey-name`) | ||
| 372 | assert.Contains(t, form, `type="submit"`) | 385 | assert.Contains(t, form, `type="submit"`) | ||
| @@ -383,7 +396,7 @@ func TestPasskeysPageFull(t *testing.T) { | |||||
| 383 | f := newPasskeyServer(t) | 396 | f := newPasskeyServer(t) | ||
| 384 | list := make([]domain.Passkey, domain.MaxPasskeys) | 397 | list := make([]domain.Passkey, domain.MaxPasskeys) | ||
| 385 | for i := range list { | 398 | for i := range list { | ||
| 386 | - | list[i] = domain.Passkey{ID: []byte{byte(i + 1)}, Name: "k", CreatedAt: time.Now()} | 399 | + | list[i] = domain.Passkey{ID: []byte{byte(i + 1)}, Name: "k", CreatedAt: fixedNow} |
| 387 | } | 400 | } | ||
| 388 | f.passkeys.EXPECT().List(mock.Anything, mock.Anything).Return(list, nil).Once() | 401 | f.passkeys.EXPECT().List(mock.Anything, mock.Anything).Return(list, nil).Once() | ||
| 389 | rec := f.do(t, req{method: "GET", target: "/-/passkeys", cookie: "tok"}) | 402 | rec := f.do(t, req{method: "GET", target: "/-/passkeys", cookie: "tok"}) | ||
| @@ -554,50 +567,55 @@ func TestIsPasskeyLoginPath(t *testing.T) { | |||||
| 554 | } | 567 | } | ||
| 555 | } | 568 | } | ||
| 556 | 569 | ||||
| 557 | - | // GO-005: a blocked client gets 429 whatever it sends, and a malformed finish | 570 | + | // A finish that cannot be read is a 4xx, and is not counted as a failed |
| 558 | - | // is a 4xx that does not count as a failed authentication. | 571 | + | // sign-in: the client is buggy, it did not guess at a passkey. |
| 559 | - | func TestPasskeyLoginFinishBlockedBeforeBodyChecks(t *testing.T) { | 572 | + | func TestMalformedFinishIsNotAFailedSignIn(t *testing.T) { |
| 560 | t.Parallel() | 573 | t.Parallel() | ||
| 561 | f := newPasskeyServer(t) | 574 | f := newPasskeyServer(t) | ||
| 562 | - | f.lim.blocked = true | |||
| 563 | - | for _, r := range []req{ | |||
| 564 | - | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | |||
| 565 | - | postJSON(finishLogin, "", `not json`), | |||
| 566 | - | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |||
| 567 | - | } { | |||
| 568 | - | rec := f.do(t, r) | |||
| 569 | - | assert.Equal(t, http.StatusTooManyRequests, rec.Code, "%q", r.contentType) | |||
| 570 | - | } | |||
| 571 | - | assert.Zero(t, f.lim.fails) | |||
| 572 | - | ||||
| 573 | - | f.lim.blocked = false | |||
| 574 | for _, r := range []req{ | 575 | for _, r := range []req{ | ||
| 575 | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | 576 | {method: "POST", target: finishLogin, origin: forgeOrigin, contentType: "text/plain", body: "x"}, | ||
| 576 | postJSON(finishLogin, "", `not json`), | 577 | postJSON(finishLogin, "", `not json`), | ||
| 577 | postJSON(finishLogin, "", `{}`), | 578 | postJSON(finishLogin, "", `{}`), | ||
| 579 | + | postJSON(finishLogin, "", strings.Repeat("a", 64<<10+1)), | |||
| 578 | } { | 580 | } { | ||
| 579 | rec := f.do(t, r) | 581 | rec := f.do(t, r) | ||
| 580 | assert.GreaterOrEqual(t, rec.Code, 400) | 582 | assert.GreaterOrEqual(t, rec.Code, 400) | ||
| 581 | assert.Less(t, rec.Code, 500) | 583 | assert.Less(t, rec.Code, 500) | ||
| 582 | } | 584 | } | ||
| 583 | - | assert.Zero(t, f.lim.fails, "a malformed request is not a failed sign-in") | 585 | + | assert.Zero(t, f.lim.fails) |
| 584 | } | 586 | } | ||
| 585 | 587 | ||||
| 586 | - | // The client key the web hands to the use case is the one the limiter uses. | 588 | + | // The client key the web hands to the use case is the one the failure limiter |
| 587 | - | func TestPasskeyLoginBeginPassesTheClientAddress(t *testing.T) { | 589 | + | // counts under: the address with trusted proxies applied, an IPv4 address as it |
| 590 | + | // is (mapped or not), an IPv6 one cut to its /64. | |||
| 591 | + | func TestPasskeyLoginBeginPassesTheClientKey(t *testing.T) { | |||
| 588 | t.Parallel() | 592 | t.Parallel() | ||
| 589 | - | f := newPasskeyServer(t) | 593 | + | for name, tc := range map[string]struct{ remote, xff, want string }{ |
| 590 | - | f.passkeys.EXPECT().BeginLogin(mock.Anything, "192.0.2.77").Return("c", []byte(`{}`), nil).Once() | 594 | + | "ipv4": {"192.0.2.77:51000", "", "192.0.2.77"}, |
| 591 | - | r := httptest.NewRequest("POST", beginLogin, nil) | 595 | + | "ipv4 mapped": {"[::ffff:192.0.2.77]:51000", "", "192.0.2.77"}, |
| 592 | - | r.RemoteAddr = "192.0.2.77:51000" | 596 | + | "ipv6 host": {"[2001:db8:1:2::1]:51000", "", "2001:db8:1:2::/64"}, |
| 593 | - | r.Header.Set("Origin", forgeOrigin) | 597 | + | "ipv6 same /64": {"[2001:db8:1:2:ffff:ffff:ffff:ffff]:51000", "", "2001:db8:1:2::/64"}, |
| 594 | - | r.Header.Set("X-Forwarded-For", "203.0.113.5") // no trusted proxy: ignored | 598 | + | "ipv6 another /64": {"[2001:db8:1:3::1]:51000", "", "2001:db8:1:3::/64"}, |
| 595 | - | rec := httptest.NewRecorder() | 599 | + | "forwarded for ignored": {"192.0.2.77:51000", "203.0.113.5", "192.0.2.77"}, |
| 596 | - | f.srv.ServeHTTP(rec, r) | 600 | + | } { |
| 597 | - | assert.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) | 601 | + | t.Run(name, func(t *testing.T) { |
| 602 | + | t.Parallel() | |||
| 603 | + | f := newPasskeyServer(t) | |||
| 604 | + | f.passkeys.EXPECT().BeginLogin(mock.Anything, tc.want).Return("c", []byte(`{}`), nil).Once() | |||
| 605 | + | r := httptest.NewRequest("POST", beginLogin, nil) | |||
| 606 | + | r.RemoteAddr = tc.remote | |||
| 607 | + | r.Header.Set("Origin", forgeOrigin) | |||
| 608 | + | if tc.xff != "" { | |||
| 609 | + | r.Header.Set("X-Forwarded-For", tc.xff) // no trusted proxy: ignored | |||
| 610 | + | } | |||
| 611 | + | rec := httptest.NewRecorder() | |||
| 612 | + | f.srv.ServeHTTP(rec, r) | |||
| 613 | + | assert.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) | |||
| 614 | + | }) | |||
| 615 | + | } | |||
| 598 | } | 616 | } | ||
| 599 | 617 | ||||
| 600 | - | // TS-001: the name travels with register/begin and is checked there. | 618 | + | // The name travels with register/begin and is checked there. |
| 601 | func TestPasskeyRegisterBeginChecksTheName(t *testing.T) { | 619 | func TestPasskeyRegisterBeginChecksTheName(t *testing.T) { | ||
| 602 | t.Parallel() | 620 | t.Parallel() | ||
| 603 | f := newPasskeyServer(t) | 621 | f := newPasskeyServer(t) | ||
M internal/adapters/webauthn/webauthn.go +4 -13
| @@ -11,7 +11,6 @@ import ( | |||
| 11 | 11 | "fmt" | |
| 12 | 12 | "net/url" | |
| 13 | 13 | "strings" | |
| 14 | - | "time" | |
| 15 | 14 | ||
| 16 | 15 | "github.com/go-webauthn/webauthn/protocol" | |
| 17 | 16 | gowebauthn "github.com/go-webauthn/webauthn/webauthn" | |
| @@ -20,10 +19,6 @@ import ( | |||
| 20 | 19 | "bnema.dev/ermite/internal/ports" | |
| 21 | 20 | ) | |
| 22 | 21 | ||
| 23 | - | // ceremonyTimeout is how long the browser may take, enforced here too. The | |
| 24 | - | // use case keeps its own, equal TTL for the ceremony data. | |
| 25 | - | const ceremonyTimeout = 2 * time.Minute | |
| 26 | - | ||
| 27 | 22 | // ownerName names the one user in the options sent to the browser. | |
| 28 | 23 | const ownerName = "owner" | |
| 29 | 24 | ||
| @@ -46,7 +41,7 @@ func New(publicURL string) (*Verifier, error) { | |||
| 46 | 41 | if (u.Scheme != "http" && u.Scheme != "https") || host == "" { | |
| 47 | 42 | return nil, fmt.Errorf("passkeys: public URL %q needs an http(s) scheme and a host", publicURL) | |
| 48 | 43 | } | |
| 49 | - | timeout := gowebauthn.TimeoutConfig{Enforce: true, Timeout: ceremonyTimeout, TimeoutUVD: ceremonyTimeout} | |
| 44 | + | timeout := gowebauthn.TimeoutConfig{Enforce: true, Timeout: domain.PasskeyCeremonyTTL, TimeoutUVD: domain.PasskeyCeremonyTTL} | |
| 50 | 45 | web, err := gowebauthn.New(&gowebauthn.Config{ | |
| 51 | 46 | RPID: host, | |
| 52 | 47 | RPDisplayName: host, | |
| @@ -114,9 +109,6 @@ func (v *Verifier) FinishRegistration(userHandle []byte, ceremony, response []by | |||
| 114 | 109 | if err != nil { | |
| 115 | 110 | return domain.Passkey{}, refuse("registration", err) | |
| 116 | 111 | } | |
| 117 | - | if n := len(cred.ID); n == 0 || n > domain.MaxPasskeyIDLen { | |
| 118 | - | return domain.Passkey{}, refuse("registration", fmt.Errorf("credential id has %d bytes", n)) | |
| 119 | - | } | |
| 120 | 112 | transports := make([]string, 0, len(cred.Transport)) | |
| 121 | 113 | for _, t := range cred.Transport { | |
| 122 | 114 | transports = append(transports, string(t)) | |
| @@ -144,7 +136,7 @@ func (v *Verifier) BeginLogin() ([]byte, []byte, error) { | |||
| 144 | 136 | // FinishLogin implements ports.PasskeyVerifier. The library compares the user | |
| 145 | 137 | // handle of the response with userHandle. | |
| 146 | 138 | // | |
| 147 | - | //hexcheck:ignore suspicious-business-logic-in-adapter copies the counter and backup state the library verified into the stored passkey; the clone rule is the library's, the refusal is the port contract | |
| 139 | + | //hexcheck:ignore suspicious-business-logic-in-adapter copies the counter and backup state the library verified into the stored passkey; it decides nothing: the use case refuses a counter that does not advance | |
| 148 | 140 | func (v *Verifier) FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) { | |
| 149 | 141 | var session gowebauthn.SessionData | |
| 150 | 142 | if err := json.Unmarshal(ceremony, &session); err != nil { | |
| @@ -175,9 +167,8 @@ func (v *Verifier) FinishLogin(userHandle []byte, ceremony, response []byte, loo | |||
| 175 | 167 | if err != nil { | |
| 176 | 168 | return domain.Passkey{}, refuse("login", err) | |
| 177 | 169 | } | |
| 178 | - | if cred.Authenticator.CloneWarning { | |
| 179 | - | return domain.Passkey{}, domain.ErrPasskeyCloned | |
| 180 | - | } | |
| 170 | + | // The library keeps the stored counter when the received one does not | |
| 171 | + | // advance (its clone warning); the use case refuses that, under its lock. | |
| 181 | 172 | stored.SignCount = cred.Authenticator.SignCount | |
| 182 | 173 | stored.BackupState = cred.Flags.BackupState | |
| 183 | 174 | return stored, nil | |
| @@ -11,7 +11,6 @@ import ( | |||||
| 11 | "fmt" | 11 | "fmt" | ||
| 12 | "net/url" | 12 | "net/url" | ||
| 13 | "strings" | 13 | "strings" | ||
| 14 | - | "time" | |||
| 15 | 14 | ||||
| 16 | "github.com/go-webauthn/webauthn/protocol" | 15 | "github.com/go-webauthn/webauthn/protocol" | ||
| 17 | gowebauthn "github.com/go-webauthn/webauthn/webauthn" | 16 | gowebauthn "github.com/go-webauthn/webauthn/webauthn" | ||
| @@ -20,10 +19,6 @@ import ( | |||||
| 20 | "bnema.dev/ermite/internal/ports" | 19 | "bnema.dev/ermite/internal/ports" | ||
| 21 | ) | 20 | ) | ||
| 22 | 21 | ||||
| 23 | - | // ceremonyTimeout is how long the browser may take, enforced here too. The | |||
| 24 | - | // use case keeps its own, equal TTL for the ceremony data. | |||
| 25 | - | const ceremonyTimeout = 2 * time.Minute | |||
| 26 | - | ||||
| 27 | // ownerName names the one user in the options sent to the browser. | 22 | // ownerName names the one user in the options sent to the browser. | ||
| 28 | const ownerName = "owner" | 23 | const ownerName = "owner" | ||
| 29 | 24 | ||||
| @@ -46,7 +41,7 @@ func New(publicURL string) (*Verifier, error) { | |||||
| 46 | if (u.Scheme != "http" && u.Scheme != "https") || host == "" { | 41 | if (u.Scheme != "http" && u.Scheme != "https") || host == "" { | ||
| 47 | return nil, fmt.Errorf("passkeys: public URL %q needs an http(s) scheme and a host", publicURL) | 42 | return nil, fmt.Errorf("passkeys: public URL %q needs an http(s) scheme and a host", publicURL) | ||
| 48 | } | 43 | } | ||
| 49 | - | timeout := gowebauthn.TimeoutConfig{Enforce: true, Timeout: ceremonyTimeout, TimeoutUVD: ceremonyTimeout} | 44 | + | timeout := gowebauthn.TimeoutConfig{Enforce: true, Timeout: domain.PasskeyCeremonyTTL, TimeoutUVD: domain.PasskeyCeremonyTTL} |
| 50 | web, err := gowebauthn.New(&gowebauthn.Config{ | 45 | web, err := gowebauthn.New(&gowebauthn.Config{ | ||
| 51 | RPID: host, | 46 | RPID: host, | ||
| 52 | RPDisplayName: host, | 47 | RPDisplayName: host, | ||
| @@ -114,9 +109,6 @@ func (v *Verifier) FinishRegistration(userHandle []byte, ceremony, response []by | |||||
| 114 | if err != nil { | 109 | if err != nil { | ||
| 115 | return domain.Passkey{}, refuse("registration", err) | 110 | return domain.Passkey{}, refuse("registration", err) | ||
| 116 | } | 111 | } | ||
| 117 | - | if n := len(cred.ID); n == 0 || n > domain.MaxPasskeyIDLen { | |||
| 118 | - | return domain.Passkey{}, refuse("registration", fmt.Errorf("credential id has %d bytes", n)) | |||
| 119 | - | } | |||
| 120 | transports := make([]string, 0, len(cred.Transport)) | 112 | transports := make([]string, 0, len(cred.Transport)) | ||
| 121 | for _, t := range cred.Transport { | 113 | for _, t := range cred.Transport { | ||
| 122 | transports = append(transports, string(t)) | 114 | transports = append(transports, string(t)) | ||
| @@ -144,7 +136,7 @@ func (v *Verifier) BeginLogin() ([]byte, []byte, error) { | |||||
| 144 | // FinishLogin implements ports.PasskeyVerifier. The library compares the user | 136 | // FinishLogin implements ports.PasskeyVerifier. The library compares the user | ||
| 145 | // handle of the response with userHandle. | 137 | // handle of the response with userHandle. | ||
| 146 | // | 138 | // | ||
| 147 | - | //hexcheck:ignore suspicious-business-logic-in-adapter copies the counter and backup state the library verified into the stored passkey; the clone rule is the library's, the refusal is the port contract | 139 | + | //hexcheck:ignore suspicious-business-logic-in-adapter copies the counter and backup state the library verified into the stored passkey; it decides nothing: the use case refuses a counter that does not advance |
| 148 | func (v *Verifier) FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) { | 140 | func (v *Verifier) FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) { | ||
| 149 | var session gowebauthn.SessionData | 141 | var session gowebauthn.SessionData | ||
| 150 | if err := json.Unmarshal(ceremony, &session); err != nil { | 142 | if err := json.Unmarshal(ceremony, &session); err != nil { | ||
| @@ -175,9 +167,8 @@ func (v *Verifier) FinishLogin(userHandle []byte, ceremony, response []byte, loo | |||||
| 175 | if err != nil { | 167 | if err != nil { | ||
| 176 | return domain.Passkey{}, refuse("login", err) | 168 | return domain.Passkey{}, refuse("login", err) | ||
| 177 | } | 169 | } | ||
| 178 | - | if cred.Authenticator.CloneWarning { | 170 | + | // The library keeps the stored counter when the received one does not |
| 179 | - | return domain.Passkey{}, domain.ErrPasskeyCloned | 171 | + | // advance (its clone warning); the use case refuses that, under its lock. |
| 180 | - | } | |||
| 181 | stored.SignCount = cred.Authenticator.SignCount | 172 | stored.SignCount = cred.Authenticator.SignCount | ||
| 182 | stored.BackupState = cred.Flags.BackupState | 173 | stored.BackupState = cred.Flags.BackupState | ||
| 183 | return stored, nil | 174 | return stored, nil | ||
M internal/adapters/webauthn/webauthn_test.go +17 -6
| @@ -280,7 +280,6 @@ func TestLoginRefused(t *testing.T) { | |||
| 280 | 280 | err := tc.run(t, d) | |
| 281 | 281 | require.Error(t, err) | |
| 282 | 282 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 283 | - | assert.NotErrorIs(t, err, domain.ErrPasskeyCloned) | |
| 284 | 283 | ||
| 285 | 284 | // Control: the refusal was for the reason of the case. With the device | |
| 286 | 285 | // and the store as they were, a fresh assertion still succeeds, so the | |
| @@ -328,7 +327,11 @@ func TestLoginStoreFailureIsNotAnUnauthenticated(t *testing.T) { | |||
| 328 | 327 | assert.NotErrorIs(t, err, domain.ErrUnauthenticated) | |
| 329 | 328 | } | |
| 330 | 329 | ||
| 331 | - | func TestClonedAuthenticator(t *testing.T) { | |
| 330 | + | // A counter that does not advance is not refused here: the verifier reports the | |
| 331 | + | // stored counter (the library keeps it on a clone warning) and the use case, | |
| 332 | + | // which orders the logins under its lock, refuses it. TestPasskeyClonedAuthenticator | |
| 333 | + | // in package app checks the refusal end to end. | |
| 334 | + | func TestCounterThatDoesNotAdvanceKeepsTheStoredOne(t *testing.T) { | |
| 332 | 335 | t.Parallel() | |
| 333 | 336 | s := sites["localhost"] | |
| 334 | 337 | v, err := webauthn.New(s.url) | |
| @@ -342,11 +345,19 @@ func TestClonedAuthenticator(t *testing.T) { | |||
| 342 | 345 | assert.Equal(t, uint32(1), got.SignCount) | |
| 343 | 346 | d.store[string(got.ID)] = got | |
| 344 | 347 | ||
| 345 | - | // The counter did not advance: a copy of the key is in use. | |
| 348 | + | // The counter did not advance: a copy of the key may be in use. The answer | |
| 349 | + | // is valid, and carries the stored counter, not a lower or equal new one. | |
| 346 | 350 | ceremony, options = d.assertion(t, v, s) | |
| 347 | - | _, err = v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) | |
| 348 | - | require.ErrorIs(t, err, domain.ErrPasskeyCloned) | |
| 349 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 351 | + | again, err := v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) | |
| 352 | + | require.NoError(t, err) | |
| 353 | + | assert.Equal(t, uint32(1), again.SignCount, "the stored counter, unchanged") | |
| 354 | + | ||
| 355 | + | // A response with a lower counter keeps the stored one too. | |
| 356 | + | d.cred.Counter = 0 | |
| 357 | + | ceremony, options = d.assertion(t, v, s) | |
| 358 | + | lower, err := v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) | |
| 359 | + | require.NoError(t, err) | |
| 360 | + | assert.Equal(t, uint32(1), lower.SignCount) | |
| 350 | 361 | } | |
| 351 | 362 | ||
| 352 | 363 | // Synced passkeys always report 0: that is not a clone. | |
| @@ -280,7 +280,6 @@ func TestLoginRefused(t *testing.T) { | |||||
| 280 | err := tc.run(t, d) | 280 | err := tc.run(t, d) | ||
| 281 | require.Error(t, err) | 281 | require.Error(t, err) | ||
| 282 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 282 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 283 | - | assert.NotErrorIs(t, err, domain.ErrPasskeyCloned) | |||
| 284 | 283 | ||||
| 285 | // Control: the refusal was for the reason of the case. With the device | 284 | // Control: the refusal was for the reason of the case. With the device | ||
| 286 | // and the store as they were, a fresh assertion still succeeds, so the | 285 | // and the store as they were, a fresh assertion still succeeds, so the | ||
| @@ -328,7 +327,11 @@ func TestLoginStoreFailureIsNotAnUnauthenticated(t *testing.T) { | |||||
| 328 | assert.NotErrorIs(t, err, domain.ErrUnauthenticated) | 327 | assert.NotErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 329 | } | 328 | } | ||
| 330 | 329 | ||||
| 331 | - | func TestClonedAuthenticator(t *testing.T) { | 330 | + | // A counter that does not advance is not refused here: the verifier reports the |
| 331 | + | // stored counter (the library keeps it on a clone warning) and the use case, | |||
| 332 | + | // which orders the logins under its lock, refuses it. TestPasskeyClonedAuthenticator | |||
| 333 | + | // in package app checks the refusal end to end. | |||
| 334 | + | func TestCounterThatDoesNotAdvanceKeepsTheStoredOne(t *testing.T) { | |||
| 332 | t.Parallel() | 335 | t.Parallel() | ||
| 333 | s := sites["localhost"] | 336 | s := sites["localhost"] | ||
| 334 | v, err := webauthn.New(s.url) | 337 | v, err := webauthn.New(s.url) | ||
| @@ -342,11 +345,19 @@ func TestClonedAuthenticator(t *testing.T) { | |||||
| 342 | assert.Equal(t, uint32(1), got.SignCount) | 345 | assert.Equal(t, uint32(1), got.SignCount) | ||
| 343 | d.store[string(got.ID)] = got | 346 | d.store[string(got.ID)] = got | ||
| 344 | 347 | ||||
| 345 | - | // The counter did not advance: a copy of the key is in use. | 348 | + | // The counter did not advance: a copy of the key may be in use. The answer |
| 349 | + | // is valid, and carries the stored counter, not a lower or equal new one. | |||
| 346 | ceremony, options = d.assertion(t, v, s) | 350 | ceremony, options = d.assertion(t, v, s) | ||
| 347 | - | _, err = v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) | 351 | + | again, err := v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) |
| 348 | - | require.ErrorIs(t, err, domain.ErrPasskeyCloned) | 352 | + | require.NoError(t, err) |
| 349 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 353 | + | assert.Equal(t, uint32(1), again.SignCount, "the stored counter, unchanged") |
| 354 | + | ||||
| 355 | + | // A response with a lower counter keeps the stored one too. | |||
| 356 | + | d.cred.Counter = 0 | |||
| 357 | + | ceremony, options = d.assertion(t, v, s) | |||
| 358 | + | lower, err := v.FinishLogin(userHandle, ceremony, d.respond(s, options), d.lookup) | |||
| 359 | + | require.NoError(t, err) | |||
| 360 | + | assert.Equal(t, uint32(1), lower.SignCount) | |||
| 350 | } | 361 | } | ||
| 351 | 362 | ||||
| 352 | // Synced passkeys always report 0: that is not a clone. | 363 | // Synced passkeys always report 0: that is not a clone. | ||
M internal/app/web.go +4 -9
| @@ -164,10 +164,10 @@ func (s webSessions) Verify(ctx context.Context, token string) (domain.Principal | |||
| 164 | 164 | return p, true, nil | |
| 165 | 165 | } | |
| 166 | 166 | ||
| 167 | - | // webPasskeys adapts the passkey use cases to ports.WebPasskeyService: the | |
| 168 | - | // session a passkey sign-in opens is given to the web as a token and a | |
| 169 | - | // lifetime, like webSessions.Login does for a link, and a refusal is | |
| 170 | - | // ports.ErrLinkRejected. | |
| 167 | + | // webPasskeys adapts the passkey use cases to ports.WebPasskeyService. Only | |
| 168 | + | // FinishLogin differs from *usecase.Passkeys: the session a passkey sign-in | |
| 169 | + | // opens is given to the web as a token and a lifetime, like webSessions.Login | |
| 170 | + | // does for a link, and a refusal is ports.ErrLinkRejected. | |
| 171 | 171 | type webPasskeys struct{ *usecase.Passkeys } | |
| 172 | 172 | ||
| 173 | 173 | var _ ports.WebPasskeyService = webPasskeys{} | |
| @@ -187,11 +187,6 @@ func (p webPasskeys) FinishLogin(ctx context.Context, ceremony string, response | |||
| 187 | 187 | return token, lifetime, nil | |
| 188 | 188 | } | |
| 189 | 189 | ||
| 190 | - | func (p webPasskeys) FinishRegistration(ctx context.Context, owner domain.Principal, ceremony, name string, response []byte) error { | |
| 191 | - | _, err := p.Passkeys.FinishRegistration(ctx, owner, ceremony, name, response) | |
| 192 | - | return err | |
| 193 | - | } | |
| 194 | - | ||
| 195 | 190 | // webRoutes mounts the web on the patterns it serves. | |
| 196 | 191 | func webRoutes(h http.Handler) []httpserver.Route { | |
| 197 | 192 | routes := make([]httpserver.Route, 0, len(web.Patterns)) | |
| @@ -164,10 +164,10 @@ func (s webSessions) Verify(ctx context.Context, token string) (domain.Principal | |||||
| 164 | return p, true, nil | 164 | return p, true, nil | ||
| 165 | } | 165 | } | ||
| 166 | 166 | ||||
| 167 | - | // webPasskeys adapts the passkey use cases to ports.WebPasskeyService: the | 167 | + | // webPasskeys adapts the passkey use cases to ports.WebPasskeyService. Only |
| 168 | - | // session a passkey sign-in opens is given to the web as a token and a | 168 | + | // FinishLogin differs from *usecase.Passkeys: the session a passkey sign-in |
| 169 | - | // lifetime, like webSessions.Login does for a link, and a refusal is | 169 | + | // opens is given to the web as a token and a lifetime, like webSessions.Login |
| 170 | - | // ports.ErrLinkRejected. | 170 | + | // does for a link, and a refusal is ports.ErrLinkRejected. |
| 171 | type webPasskeys struct{ *usecase.Passkeys } | 171 | type webPasskeys struct{ *usecase.Passkeys } | ||
| 172 | 172 | ||||
| 173 | var _ ports.WebPasskeyService = webPasskeys{} | 173 | var _ ports.WebPasskeyService = webPasskeys{} | ||
| @@ -187,11 +187,6 @@ func (p webPasskeys) FinishLogin(ctx context.Context, ceremony string, response | |||||
| 187 | return token, lifetime, nil | 187 | return token, lifetime, nil | ||
| 188 | } | 188 | } | ||
| 189 | 189 | ||||
| 190 | - | func (p webPasskeys) FinishRegistration(ctx context.Context, owner domain.Principal, ceremony, name string, response []byte) error { | |||
| 191 | - | _, err := p.Passkeys.FinishRegistration(ctx, owner, ceremony, name, response) | |||
| 192 | - | return err | |||
| 193 | - | } | |||
| 194 | - | ||||
| 195 | // webRoutes mounts the web on the patterns it serves. | 190 | // webRoutes mounts the web on the patterns it serves. | ||
| 196 | func webRoutes(h http.Handler) []httpserver.Route { | 191 | func webRoutes(h http.Handler) []httpserver.Route { | ||
| 197 | routes := make([]httpserver.Route, 0, len(web.Patterns)) | 192 | routes := make([]httpserver.Route, 0, len(web.Patterns)) | ||
M internal/app/web_passkey_test.go +54 -1
| @@ -4,7 +4,9 @@ package app | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "bytes" | |
| 7 | + | "crypto/sha256" | |
| 7 | 8 | "encoding/base64" | |
| 9 | + | "encoding/hex" | |
| 8 | 10 | "encoding/json" | |
| 9 | 11 | "net/http" | |
| 10 | 12 | "net/url" | |
| @@ -89,7 +91,7 @@ func (p *passkeyHarness) register(token, name string) page { | |||
| 89 | 91 | opts, err := virtualwebauthn.ParseAttestationOptions(string(br.Options)) | |
| 90 | 92 | require.NoError(p.t, err) | |
| 91 | 93 | resp := virtualwebauthn.CreateAttestationResponse(passkeyRP, p.auth, p.cred, *opts) | |
| 92 | - | fin := p.postJSON("/-/passkeys/register/finish", token, map[string]any{"ceremony": br.Ceremony, "name": name, "credential": json.RawMessage(resp)}) | |
| 94 | + | fin := p.postJSON("/-/passkeys/register/finish", token, map[string]any{"ceremony": br.Ceremony, "credential": json.RawMessage(resp)}) | |
| 93 | 95 | if fin.status == http.StatusNoContent { | |
| 94 | 96 | p.auth.AddCredential(p.cred) | |
| 95 | 97 | p.auth.Options.UserHandle = p.userHandle() | |
| @@ -254,3 +256,54 @@ func TestPasskeysDisabledOnHTTPPublicURL(t *testing.T) { | |||
| 254 | 256 | ver := c.cli(t, "", "version", "--json").ok(t) | |
| 255 | 257 | assert.NotContains(t, ver.stdout, `"features": [`+"\n"+` "passkeys"`) | |
| 256 | 258 | } | |
| 259 | + | ||
| 260 | + | // A passkey whose counter does not advance (a hardware key that was cloned) is | |
| 261 | + | // refused end to end, with nothing set and the stored counter kept, while a | |
| 262 | + | // counter that advances signs in. Without the sign-count check of the use case | |
| 263 | + | // the clone would sign in: the WebAuthn adapter accepts it and only reports the | |
| 264 | + | // stored counter back. | |
| 265 | + | func TestPasskeyClonedAuthenticator(t *testing.T) { | |
| 266 | + | p := newPasskeyHarness(t) | |
| 267 | + | link := p.cli(t, adminToken, "web", "login").ok(t) | |
| 268 | + | pg := p.b.do("POST", passkeyLoginPath(t, link.stdout), map[string]string{"Origin": passkeyOrigin}, "") | |
| 269 | + | token := pg.cookie(sessionCookie).Value | |
| 270 | + | ||
| 271 | + | // A hardware key: its counter counts up from 1. | |
| 272 | + | p.cred.Counter = 1 | |
| 273 | + | require.Equal(t, http.StatusNoContent, p.register(token, "key").status) | |
| 274 | + | ||
| 275 | + | p.cred.Counter = 5 | |
| 276 | + | res := p.login() | |
| 277 | + | require.Equal(t, http.StatusNoContent, res.status, res.body) | |
| 278 | + | ||
| 279 | + | // The clone presents the counter it last saw: not higher than the stored 5. | |
| 280 | + | for _, counter := range []uint32{5, 3} { | |
| 281 | + | p.cred.Counter = counter | |
| 282 | + | res = p.login() | |
| 283 | + | assert.Equal(t, http.StatusUnauthorized, res.status, "counter %d: %s", counter, res.body) | |
| 284 | + | assert.Empty(t, res.hdr.Values("Set-Cookie"), "no session for a clone") | |
| 285 | + | } | |
| 286 | + | ||
| 287 | + | // The refusals recorded nothing: the next higher counter still works. | |
| 288 | + | p.cred.Counter = 6 | |
| 289 | + | res = p.login() | |
| 290 | + | assert.Equal(t, http.StatusNoContent, res.status, res.body) | |
| 291 | + | ||
| 292 | + | stored, err := p.forge.store.Passkeys().List(p.ctx) | |
| 293 | + | require.NoError(t, err) | |
| 294 | + | require.Len(t, stored, 1) | |
| 295 | + | assert.Equal(t, uint32(6), stored[0].SignCount) | |
| 296 | + | ||
| 297 | + | // The warning names the passkey by its short reference, never by its id. | |
| 298 | + | sum := sha256.Sum256(p.cred.ID) | |
| 299 | + | ref := hex.EncodeToString(sum[:])[:8] | |
| 300 | + | var warns []string | |
| 301 | + | for _, line := range strings.Split(p.logs.String(), "\n") { | |
| 302 | + | var m map[string]any | |
| 303 | + | if json.Unmarshal([]byte(line), &m) == nil && m["level"] == "warn" && m["passkey"] == ref { | |
| 304 | + | warns = append(warns, line) | |
| 305 | + | } | |
| 306 | + | } | |
| 307 | + | assert.Len(t, warns, 2, "one warning per refused clone") | |
| 308 | + | assert.NotContains(t, p.logs.String(), base64.RawURLEncoding.EncodeToString(p.cred.ID)) | |
| 309 | + | } | |
| @@ -4,7 +4,9 @@ package app | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "bytes" | 6 | "bytes" | ||
| 7 | + | "crypto/sha256" | |||
| 7 | "encoding/base64" | 8 | "encoding/base64" | ||
| 9 | + | "encoding/hex" | |||
| 8 | "encoding/json" | 10 | "encoding/json" | ||
| 9 | "net/http" | 11 | "net/http" | ||
| 10 | "net/url" | 12 | "net/url" | ||
| @@ -89,7 +91,7 @@ func (p *passkeyHarness) register(token, name string) page { | |||||
| 89 | opts, err := virtualwebauthn.ParseAttestationOptions(string(br.Options)) | 91 | opts, err := virtualwebauthn.ParseAttestationOptions(string(br.Options)) | ||
| 90 | require.NoError(p.t, err) | 92 | require.NoError(p.t, err) | ||
| 91 | resp := virtualwebauthn.CreateAttestationResponse(passkeyRP, p.auth, p.cred, *opts) | 93 | resp := virtualwebauthn.CreateAttestationResponse(passkeyRP, p.auth, p.cred, *opts) | ||
| 92 | - | fin := p.postJSON("/-/passkeys/register/finish", token, map[string]any{"ceremony": br.Ceremony, "name": name, "credential": json.RawMessage(resp)}) | 94 | + | fin := p.postJSON("/-/passkeys/register/finish", token, map[string]any{"ceremony": br.Ceremony, "credential": json.RawMessage(resp)}) |
| 93 | if fin.status == http.StatusNoContent { | 95 | if fin.status == http.StatusNoContent { | ||
| 94 | p.auth.AddCredential(p.cred) | 96 | p.auth.AddCredential(p.cred) | ||
| 95 | p.auth.Options.UserHandle = p.userHandle() | 97 | p.auth.Options.UserHandle = p.userHandle() | ||
| @@ -254,3 +256,54 @@ func TestPasskeysDisabledOnHTTPPublicURL(t *testing.T) { | |||||
| 254 | ver := c.cli(t, "", "version", "--json").ok(t) | 256 | ver := c.cli(t, "", "version", "--json").ok(t) | ||
| 255 | assert.NotContains(t, ver.stdout, `"features": [`+"\n"+` "passkeys"`) | 257 | assert.NotContains(t, ver.stdout, `"features": [`+"\n"+` "passkeys"`) | ||
| 256 | } | 258 | } | ||
| 259 | + | ||||
| 260 | + | // A passkey whose counter does not advance (a hardware key that was cloned) is | |||
| 261 | + | // refused end to end, with nothing set and the stored counter kept, while a | |||
| 262 | + | // counter that advances signs in. Without the sign-count check of the use case | |||
| 263 | + | // the clone would sign in: the WebAuthn adapter accepts it and only reports the | |||
| 264 | + | // stored counter back. | |||
| 265 | + | func TestPasskeyClonedAuthenticator(t *testing.T) { | |||
| 266 | + | p := newPasskeyHarness(t) | |||
| 267 | + | link := p.cli(t, adminToken, "web", "login").ok(t) | |||
| 268 | + | pg := p.b.do("POST", passkeyLoginPath(t, link.stdout), map[string]string{"Origin": passkeyOrigin}, "") | |||
| 269 | + | token := pg.cookie(sessionCookie).Value | |||
| 270 | + | ||||
| 271 | + | // A hardware key: its counter counts up from 1. | |||
| 272 | + | p.cred.Counter = 1 | |||
| 273 | + | require.Equal(t, http.StatusNoContent, p.register(token, "key").status) | |||
| 274 | + | ||||
| 275 | + | p.cred.Counter = 5 | |||
| 276 | + | res := p.login() | |||
| 277 | + | require.Equal(t, http.StatusNoContent, res.status, res.body) | |||
| 278 | + | ||||
| 279 | + | // The clone presents the counter it last saw: not higher than the stored 5. | |||
| 280 | + | for _, counter := range []uint32{5, 3} { | |||
| 281 | + | p.cred.Counter = counter | |||
| 282 | + | res = p.login() | |||
| 283 | + | assert.Equal(t, http.StatusUnauthorized, res.status, "counter %d: %s", counter, res.body) | |||
| 284 | + | assert.Empty(t, res.hdr.Values("Set-Cookie"), "no session for a clone") | |||
| 285 | + | } | |||
| 286 | + | ||||
| 287 | + | // The refusals recorded nothing: the next higher counter still works. | |||
| 288 | + | p.cred.Counter = 6 | |||
| 289 | + | res = p.login() | |||
| 290 | + | assert.Equal(t, http.StatusNoContent, res.status, res.body) | |||
| 291 | + | ||||
| 292 | + | stored, err := p.forge.store.Passkeys().List(p.ctx) | |||
| 293 | + | require.NoError(t, err) | |||
| 294 | + | require.Len(t, stored, 1) | |||
| 295 | + | assert.Equal(t, uint32(6), stored[0].SignCount) | |||
| 296 | + | ||||
| 297 | + | // The warning names the passkey by its short reference, never by its id. | |||
| 298 | + | sum := sha256.Sum256(p.cred.ID) | |||
| 299 | + | ref := hex.EncodeToString(sum[:])[:8] | |||
| 300 | + | var warns []string | |||
| 301 | + | for _, line := range strings.Split(p.logs.String(), "\n") { | |||
| 302 | + | var m map[string]any | |||
| 303 | + | if json.Unmarshal([]byte(line), &m) == nil && m["level"] == "warn" && m["passkey"] == ref { | |||
| 304 | + | warns = append(warns, line) | |||
| 305 | + | } | |||
| 306 | + | } | |||
| 307 | + | assert.Len(t, warns, 2, "one warning per refused clone") | |||
| 308 | + | assert.NotContains(t, p.logs.String(), base64.RawURLEncoding.EncodeToString(p.cred.ID)) | |||
| 309 | + | } | |||
M internal/domain/passkey.go +4 -8
| @@ -17,13 +17,12 @@ const ( | |||
| 17 | 17 | MaxPasskeyNameLen = 64 | |
| 18 | 18 | // DefaultPasskeyName names a passkey registered without a name. | |
| 19 | 19 | DefaultPasskeyName = "passkey" | |
| 20 | + | // PasskeyCeremonyTTL is how long a registration or a sign-in may take, from | |
| 21 | + | // its begin to its finish. The WebAuthn adapter enforces it on the | |
| 22 | + | // challenge and the use case on the ceremony it keeps. | |
| 23 | + | PasskeyCeremonyTTL = 2 * time.Minute | |
| 20 | 24 | ) | |
| 21 | 25 | ||
| 22 | - | // ErrPasskeyCloned means a passkey signed in with a counter that did not | |
| 23 | - | // advance, which hints at a cloned authenticator (ADR 0026). It is an | |
| 24 | - | // ErrUnauthenticated. | |
| 25 | - | var ErrPasskeyCloned = fmt.Errorf("%w: passkey counter did not advance (possible clone)", ErrUnauthenticated) | |
| 26 | - | ||
| 27 | 26 | // Passkey is a WebAuthn credential that can sign the owner in. It holds the | |
| 28 | 27 | // public key only. | |
| 29 | 28 | type Passkey struct { | |
| @@ -67,6 +66,3 @@ func ParsePasskeyName(s string) (PasskeyName, error) { | |||
| 67 | 66 | ||
| 68 | 67 | // String returns the name. | |
| 69 | 68 | func (n PasskeyName) String() string { return n.s } | |
| 70 | - | ||
| 71 | - | // IsZero reports whether n is the zero value. | |
| 72 | - | func (n PasskeyName) IsZero() bool { return n.s == "" } | |
| @@ -17,13 +17,12 @@ const ( | |||||
| 17 | MaxPasskeyNameLen = 64 | 17 | MaxPasskeyNameLen = 64 | ||
| 18 | // DefaultPasskeyName names a passkey registered without a name. | 18 | // DefaultPasskeyName names a passkey registered without a name. | ||
| 19 | DefaultPasskeyName = "passkey" | 19 | DefaultPasskeyName = "passkey" | ||
| 20 | + | // PasskeyCeremonyTTL is how long a registration or a sign-in may take, from | |||
| 21 | + | // its begin to its finish. The WebAuthn adapter enforces it on the | |||
| 22 | + | // challenge and the use case on the ceremony it keeps. | |||
| 23 | + | PasskeyCeremonyTTL = 2 * time.Minute | |||
| 20 | ) | 24 | ) | ||
| 21 | 25 | ||||
| 22 | - | // ErrPasskeyCloned means a passkey signed in with a counter that did not | |||
| 23 | - | // advance, which hints at a cloned authenticator (ADR 0026). It is an | |||
| 24 | - | // ErrUnauthenticated. | |||
| 25 | - | var ErrPasskeyCloned = fmt.Errorf("%w: passkey counter did not advance (possible clone)", ErrUnauthenticated) | |||
| 26 | - | ||||
| 27 | // Passkey is a WebAuthn credential that can sign the owner in. It holds the | 26 | // Passkey is a WebAuthn credential that can sign the owner in. It holds the | ||
| 28 | // public key only. | 27 | // public key only. | ||
| 29 | type Passkey struct { | 28 | type Passkey struct { | ||
| @@ -67,6 +66,3 @@ func ParsePasskeyName(s string) (PasskeyName, error) { | |||||
| 67 | 66 | ||||
| 68 | // String returns the name. | 67 | // String returns the name. | ||
| 69 | func (n PasskeyName) String() string { return n.s } | 68 | func (n PasskeyName) String() string { return n.s } | ||
| 70 | - | ||||
| 71 | - | // IsZero reports whether n is the zero value. | |||
| 72 | - | func (n PasskeyName) IsZero() bool { return n.s == "" } | |||
M internal/domain/passkey_test.go +0 -7
| @@ -43,14 +43,12 @@ func TestParsePasskeyName(t *testing.T) { | |||
| 43 | 43 | if tc.reason == "" { | |
| 44 | 44 | require.NoError(t, err) | |
| 45 | 45 | assert.Equal(t, tc.in, got.String()) | |
| 46 | - | assert.False(t, got.IsZero()) | |
| 47 | 46 | return | |
| 48 | 47 | } | |
| 49 | 48 | require.Error(t, err) | |
| 50 | 49 | assert.ErrorIs(t, err, domain.ErrInvalidText) | |
| 51 | 50 | assert.True(t, domain.IsInvalid(err)) | |
| 52 | 51 | assert.Contains(t, err.Error(), tc.reason) | |
| 53 | - | assert.True(t, got.IsZero()) | |
| 54 | 52 | }) | |
| 55 | 53 | } | |
| 56 | 54 | } | |
| @@ -60,8 +58,3 @@ func TestDefaultPasskeyNameIsValid(t *testing.T) { | |||
| 60 | 58 | _, err := domain.ParsePasskeyName(domain.DefaultPasskeyName) | |
| 61 | 59 | require.NoError(t, err) | |
| 62 | 60 | } | |
| 63 | - | ||
| 64 | - | func TestErrPasskeyClonedIsUnauthenticated(t *testing.T) { | |
| 65 | - | t.Parallel() | |
| 66 | - | assert.ErrorIs(t, domain.ErrPasskeyCloned, domain.ErrUnauthenticated) | |
| 67 | - | } | |
| @@ -43,14 +43,12 @@ func TestParsePasskeyName(t *testing.T) { | |||||
| 43 | if tc.reason == "" { | 43 | if tc.reason == "" { | ||
| 44 | require.NoError(t, err) | 44 | require.NoError(t, err) | ||
| 45 | assert.Equal(t, tc.in, got.String()) | 45 | assert.Equal(t, tc.in, got.String()) | ||
| 46 | - | assert.False(t, got.IsZero()) | |||
| 47 | return | 46 | return | ||
| 48 | } | 47 | } | ||
| 49 | require.Error(t, err) | 48 | require.Error(t, err) | ||
| 50 | assert.ErrorIs(t, err, domain.ErrInvalidText) | 49 | assert.ErrorIs(t, err, domain.ErrInvalidText) | ||
| 51 | assert.True(t, domain.IsInvalid(err)) | 50 | assert.True(t, domain.IsInvalid(err)) | ||
| 52 | assert.Contains(t, err.Error(), tc.reason) | 51 | assert.Contains(t, err.Error(), tc.reason) | ||
| 53 | - | assert.True(t, got.IsZero()) | |||
| 54 | }) | 52 | }) | ||
| 55 | } | 53 | } | ||
| 56 | } | 54 | } | ||
| @@ -60,8 +58,3 @@ func TestDefaultPasskeyNameIsValid(t *testing.T) { | |||||
| 60 | _, err := domain.ParsePasskeyName(domain.DefaultPasskeyName) | 58 | _, err := domain.ParsePasskeyName(domain.DefaultPasskeyName) | ||
| 61 | require.NoError(t, err) | 59 | require.NoError(t, err) | ||
| 62 | } | 60 | } | ||
| 63 | - | ||||
| 64 | - | func TestErrPasskeyClonedIsUnauthenticated(t *testing.T) { | |||
| 65 | - | t.Parallel() | |||
| 66 | - | assert.ErrorIs(t, domain.ErrPasskeyCloned, domain.ErrUnauthenticated) | |||
| 67 | - | } | |||
M internal/ports/mocks/WebPasskeyService.go +10 -16
| @@ -281,16 +281,16 @@ func (_c *WebPasskeyService_FinishLogin_Call) RunAndReturn(run func(ctx context. | |||
| 281 | 281 | } | |
| 282 | 282 | ||
| 283 | 283 | // FinishRegistration provides a mock function for the type WebPasskeyService | |
| 284 | - | func (_mock *WebPasskeyService) FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte) error { | |
| 285 | - | ret := _mock.Called(ctx, p, ceremony, name, response) | |
| 284 | + | func (_mock *WebPasskeyService) FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, response []byte) error { | |
| 285 | + | ret := _mock.Called(ctx, p, ceremony, response) | |
| 286 | 286 | ||
| 287 | 287 | if len(ret) == 0 { | |
| 288 | 288 | panic("no return value specified for FinishRegistration") | |
| 289 | 289 | } | |
| 290 | 290 | ||
| 291 | 291 | var r0 error | |
| 292 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, string, string, []byte) error); ok { | |
| 293 | - | r0 = returnFunc(ctx, p, ceremony, name, response) | |
| 292 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, string, []byte) error); ok { | |
| 293 | + | r0 = returnFunc(ctx, p, ceremony, response) | |
| 294 | 294 | } else { | |
| 295 | 295 | r0 = ret.Error(0) | |
| 296 | 296 | } | |
| @@ -306,13 +306,12 @@ type WebPasskeyService_FinishRegistration_Call struct { | |||
| 306 | 306 | // - ctx context.Context | |
| 307 | 307 | // - p domain.Principal | |
| 308 | 308 | // - ceremony string | |
| 309 | - | // - name string | |
| 310 | 309 | // - response []byte | |
| 311 | - | func (_e *WebPasskeyService_Expecter) FinishRegistration(ctx any, p any, ceremony any, name any, response any) *WebPasskeyService_FinishRegistration_Call { | |
| 312 | - | return &WebPasskeyService_FinishRegistration_Call{Call: _e.mock.On("FinishRegistration", ctx, p, ceremony, name, response)} | |
| 310 | + | func (_e *WebPasskeyService_Expecter) FinishRegistration(ctx any, p any, ceremony any, response any) *WebPasskeyService_FinishRegistration_Call { | |
| 311 | + | return &WebPasskeyService_FinishRegistration_Call{Call: _e.mock.On("FinishRegistration", ctx, p, ceremony, response)} | |
| 313 | 312 | } | |
| 314 | 313 | ||
| 315 | - | func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte)) *WebPasskeyService_FinishRegistration_Call { | |
| 314 | + | func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Context, p domain.Principal, ceremony string, response []byte)) *WebPasskeyService_FinishRegistration_Call { | |
| 316 | 315 | _c.Call.Run(func(args mock.Arguments) { | |
| 317 | 316 | var arg0 context.Context | |
| 318 | 317 | if args[0] != nil { | |
| @@ -326,20 +325,15 @@ func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Co | |||
| 326 | 325 | if args[2] != nil { | |
| 327 | 326 | arg2 = args[2].(string) | |
| 328 | 327 | } | |
| 329 | - | var arg3 string | |
| 328 | + | var arg3 []byte | |
| 330 | 329 | if args[3] != nil { | |
| 331 | - | arg3 = args[3].(string) | |
| 332 | - | } | |
| 333 | - | var arg4 []byte | |
| 334 | - | if args[4] != nil { | |
| 335 | - | arg4 = args[4].([]byte) | |
| 330 | + | arg3 = args[3].([]byte) | |
| 336 | 331 | } | |
| 337 | 332 | run( | |
| 338 | 333 | arg0, | |
| 339 | 334 | arg1, | |
| 340 | 335 | arg2, | |
| 341 | 336 | arg3, | |
| 342 | - | arg4, | |
| 343 | 337 | ) | |
| 344 | 338 | }) | |
| 345 | 339 | return _c | |
| @@ -350,7 +344,7 @@ func (_c *WebPasskeyService_FinishRegistration_Call) Return(err error) *WebPassk | |||
| 350 | 344 | return _c | |
| 351 | 345 | } | |
| 352 | 346 | ||
| 353 | - | func (_c *WebPasskeyService_FinishRegistration_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte) error) *WebPasskeyService_FinishRegistration_Call { | |
| 347 | + | func (_c *WebPasskeyService_FinishRegistration_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, ceremony string, response []byte) error) *WebPasskeyService_FinishRegistration_Call { | |
| 354 | 348 | _c.Call.Return(run) | |
| 355 | 349 | return _c | |
| 356 | 350 | } | |
| @@ -281,16 +281,16 @@ func (_c *WebPasskeyService_FinishLogin_Call) RunAndReturn(run func(ctx context. | |||||
| 281 | } | 281 | } | ||
| 282 | 282 | ||||
| 283 | // FinishRegistration provides a mock function for the type WebPasskeyService | 283 | // FinishRegistration provides a mock function for the type WebPasskeyService | ||
| 284 | - | func (_mock *WebPasskeyService) FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte) error { | 284 | + | func (_mock *WebPasskeyService) FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, response []byte) error { |
| 285 | - | ret := _mock.Called(ctx, p, ceremony, name, response) | 285 | + | ret := _mock.Called(ctx, p, ceremony, response) |
| 286 | 286 | ||||
| 287 | if len(ret) == 0 { | 287 | if len(ret) == 0 { | ||
| 288 | panic("no return value specified for FinishRegistration") | 288 | panic("no return value specified for FinishRegistration") | ||
| 289 | } | 289 | } | ||
| 290 | 290 | ||||
| 291 | var r0 error | 291 | var r0 error | ||
| 292 | - | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, string, string, []byte) error); ok { | 292 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, domain.Principal, string, []byte) error); ok { |
| 293 | - | r0 = returnFunc(ctx, p, ceremony, name, response) | 293 | + | r0 = returnFunc(ctx, p, ceremony, response) |
| 294 | } else { | 294 | } else { | ||
| 295 | r0 = ret.Error(0) | 295 | r0 = ret.Error(0) | ||
| 296 | } | 296 | } | ||
| @@ -306,13 +306,12 @@ type WebPasskeyService_FinishRegistration_Call struct { | |||||
| 306 | // - ctx context.Context | 306 | // - ctx context.Context | ||
| 307 | // - p domain.Principal | 307 | // - p domain.Principal | ||
| 308 | // - ceremony string | 308 | // - ceremony string | ||
| 309 | - | // - name string | |||
| 310 | // - response []byte | 309 | // - response []byte | ||
| 311 | - | func (_e *WebPasskeyService_Expecter) FinishRegistration(ctx any, p any, ceremony any, name any, response any) *WebPasskeyService_FinishRegistration_Call { | 310 | + | func (_e *WebPasskeyService_Expecter) FinishRegistration(ctx any, p any, ceremony any, response any) *WebPasskeyService_FinishRegistration_Call { |
| 312 | - | return &WebPasskeyService_FinishRegistration_Call{Call: _e.mock.On("FinishRegistration", ctx, p, ceremony, name, response)} | 311 | + | return &WebPasskeyService_FinishRegistration_Call{Call: _e.mock.On("FinishRegistration", ctx, p, ceremony, response)} |
| 313 | } | 312 | } | ||
| 314 | 313 | ||||
| 315 | - | func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte)) *WebPasskeyService_FinishRegistration_Call { | 314 | + | func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Context, p domain.Principal, ceremony string, response []byte)) *WebPasskeyService_FinishRegistration_Call { |
| 316 | _c.Call.Run(func(args mock.Arguments) { | 315 | _c.Call.Run(func(args mock.Arguments) { | ||
| 317 | var arg0 context.Context | 316 | var arg0 context.Context | ||
| 318 | if args[0] != nil { | 317 | if args[0] != nil { | ||
| @@ -326,20 +325,15 @@ func (_c *WebPasskeyService_FinishRegistration_Call) Run(run func(ctx context.Co | |||||
| 326 | if args[2] != nil { | 325 | if args[2] != nil { | ||
| 327 | arg2 = args[2].(string) | 326 | arg2 = args[2].(string) | ||
| 328 | } | 327 | } | ||
| 329 | - | var arg3 string | 328 | + | var arg3 []byte |
| 330 | if args[3] != nil { | 329 | if args[3] != nil { | ||
| 331 | - | arg3 = args[3].(string) | 330 | + | arg3 = args[3].([]byte) |
| 332 | - | } | |||
| 333 | - | var arg4 []byte | |||
| 334 | - | if args[4] != nil { | |||
| 335 | - | arg4 = args[4].([]byte) | |||
| 336 | } | 331 | } | ||
| 337 | run( | 332 | run( | ||
| 338 | arg0, | 333 | arg0, | ||
| 339 | arg1, | 334 | arg1, | ||
| 340 | arg2, | 335 | arg2, | ||
| 341 | arg3, | 336 | arg3, | ||
| 342 | - | arg4, | |||
| 343 | ) | 337 | ) | ||
| 344 | }) | 338 | }) | ||
| 345 | return _c | 339 | return _c | ||
| @@ -350,7 +344,7 @@ func (_c *WebPasskeyService_FinishRegistration_Call) Return(err error) *WebPassk | |||||
| 350 | return _c | 344 | return _c | ||
| 351 | } | 345 | } | ||
| 352 | 346 | ||||
| 353 | - | func (_c *WebPasskeyService_FinishRegistration_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, ceremony string, name string, response []byte) error) *WebPasskeyService_FinishRegistration_Call { | 347 | + | func (_c *WebPasskeyService_FinishRegistration_Call) RunAndReturn(run func(ctx context.Context, p domain.Principal, ceremony string, response []byte) error) *WebPasskeyService_FinishRegistration_Call { |
| 354 | _c.Call.Return(run) | 348 | _c.Call.Return(run) | ||
| 355 | return _c | 349 | return _c | ||
| 356 | } | 350 | } | ||
M internal/ports/ports.go +5 -3
| @@ -357,9 +357,11 @@ type PasskeyVerifier interface { | |||
| 357 | 357 | // credential request options for the browser ({"publicKey": {...}}). | |
| 358 | 358 | BeginLogin() (options []byte, ceremony []byte, err error) | |
| 359 | 359 | // FinishLogin checks the browser's response and returns the stored passkey | |
| 360 | - | // updated with the new sign count and backup state. lookup returns | |
| 361 | - | // domain.ErrNotFound for an unknown credential id. A counter that did not | |
| 362 | - | // advance gives domain.ErrPasskeyCloned. | |
| 360 | + | // updated with the sign count and backup state the response reports. lookup | |
| 361 | + | // returns domain.ErrNotFound for an unknown credential id. A counter that | |
| 362 | + | // does not advance is not an error here: the returned SignCount is then the | |
| 363 | + | // stored one, and the caller, which holds the lock that orders the logins, | |
| 364 | + | // refuses it. | |
| 363 | 365 | FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) | |
| 364 | 366 | } | |
| 365 | 367 | ||
| @@ -357,9 +357,11 @@ type PasskeyVerifier interface { | |||||
| 357 | // credential request options for the browser ({"publicKey": {...}}). | 357 | // credential request options for the browser ({"publicKey": {...}}). | ||
| 358 | BeginLogin() (options []byte, ceremony []byte, err error) | 358 | BeginLogin() (options []byte, ceremony []byte, err error) | ||
| 359 | // FinishLogin checks the browser's response and returns the stored passkey | 359 | // FinishLogin checks the browser's response and returns the stored passkey | ||
| 360 | - | // updated with the new sign count and backup state. lookup returns | 360 | + | // updated with the sign count and backup state the response reports. lookup |
| 361 | - | // domain.ErrNotFound for an unknown credential id. A counter that did not | 361 | + | // returns domain.ErrNotFound for an unknown credential id. A counter that |
| 362 | - | // advance gives domain.ErrPasskeyCloned. | 362 | + | // does not advance is not an error here: the returned SignCount is then the |
| 363 | + | // stored one, and the caller, which holds the lock that orders the logins, | |||
| 364 | + | // refuses it. | |||
| 363 | FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) | 365 | FinishLogin(userHandle []byte, ceremony, response []byte, lookup func(id []byte) (domain.Passkey, error)) (domain.Passkey, error) | ||
| 364 | } | 366 | } | ||
| 365 | 367 | ||||
M internal/ports/services.go +13 -12
| @@ -129,26 +129,27 @@ type PasskeyService interface { | |||
| 129 | 129 | } | |
| 130 | 130 | ||
| 131 | 131 | // WebPasskeyService is the passkey side of the use cases, as the web needs | |
| 132 | - | // it. The composition root implements it over *usecase.Passkeys. A ceremony is | |
| 133 | - | // the opaque identifier BeginLogin and BeginRegistration return; options is | |
| 134 | - | // the JSON the browser passes to the WebAuthn API. | |
| 132 | + | // it: the API's List and Remove, and the browser ceremonies. The composition | |
| 133 | + | // root implements it over *usecase.Passkeys. A ceremony is the opaque | |
| 134 | + | // identifier BeginLogin and BeginRegistration return; options is the JSON the | |
| 135 | + | // browser passes to the WebAuthn API. | |
| 135 | 136 | type WebPasskeyService interface { | |
| 136 | - | List(ctx context.Context, p domain.Principal) ([]domain.Passkey, error) | |
| 137 | - | Remove(ctx context.Context, p domain.Principal, id []byte) error | |
| 138 | - | // BeginLogin starts a sign-in. Anonymous callers may. client is the | |
| 139 | - | // address the request is attributed to (trusted proxies applied, the one the | |
| 140 | - | // failure limiter uses); it bounds how many sign-ins a client keeps waiting. | |
| 137 | + | PasskeyService | |
| 138 | + | // BeginLogin starts a sign-in. Anonymous callers may. client is the client | |
| 139 | + | // key of the request (clientlimit.Key of the address the failure limiter | |
| 140 | + | // uses); it bounds how many sign-ins a client keeps waiting. | |
| 141 | 141 | BeginLogin(ctx context.Context, client string) (ceremony string, options []byte, err error) | |
| 142 | 142 | // FinishLogin checks the browser's response and returns the session token | |
| 143 | 143 | // and how long it lasts, or ErrLinkRejected for any refusal. | |
| 144 | 144 | FinishLogin(ctx context.Context, ceremony string, response []byte) (token string, lifetime time.Duration, err error) | |
| 145 | 145 | // BeginRegistration starts adding a passkey. Only the owner may. name is | |
| 146 | 146 | // the name the passkey will get (empty means the default): a name that | |
| 147 | - | // breaks the rules is refused before any ceremony starts. | |
| 147 | + | // breaks the rules is refused here, before any ceremony starts, and is the | |
| 148 | + | // one FinishRegistration stores. | |
| 148 | 149 | BeginRegistration(ctx context.Context, p domain.Principal, name string) (ceremony string, options []byte, err error) | |
| 149 | - | // FinishRegistration checks the browser's response and stores the | |
| 150 | - | // passkey under name (empty means the default name). | |
| 151 | - | FinishRegistration(ctx context.Context, p domain.Principal, ceremony, name string, response []byte) error | |
| 150 | + | // FinishRegistration checks the browser's response and stores the passkey | |
| 151 | + | // under the name given to BeginRegistration. | |
| 152 | + | FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, response []byte) error | |
| 152 | 153 | } | |
| 153 | 154 | ||
| 154 | 155 | // PushService opens a repository for a push: it is domain.ErrUnauthenticated | |
| @@ -129,26 +129,27 @@ type PasskeyService interface { | |||||
| 129 | } | 129 | } | ||
| 130 | 130 | ||||
| 131 | // WebPasskeyService is the passkey side of the use cases, as the web needs | 131 | // WebPasskeyService is the passkey side of the use cases, as the web needs | ||
| 132 | - | // it. The composition root implements it over *usecase.Passkeys. A ceremony is | 132 | + | // it: the API's List and Remove, and the browser ceremonies. The composition |
| 133 | - | // the opaque identifier BeginLogin and BeginRegistration return; options is | 133 | + | // root implements it over *usecase.Passkeys. A ceremony is the opaque |
| 134 | - | // the JSON the browser passes to the WebAuthn API. | 134 | + | // identifier BeginLogin and BeginRegistration return; options is the JSON the |
| 135 | + | // browser passes to the WebAuthn API. | |||
| 135 | type WebPasskeyService interface { | 136 | type WebPasskeyService interface { | ||
| 136 | - | List(ctx context.Context, p domain.Principal) ([]domain.Passkey, error) | 137 | + | PasskeyService |
| 137 | - | Remove(ctx context.Context, p domain.Principal, id []byte) error | 138 | + | // BeginLogin starts a sign-in. Anonymous callers may. client is the client |
| 138 | - | // BeginLogin starts a sign-in. Anonymous callers may. client is the | 139 | + | // key of the request (clientlimit.Key of the address the failure limiter |
| 139 | - | // address the request is attributed to (trusted proxies applied, the one the | 140 | + | // uses); it bounds how many sign-ins a client keeps waiting. |
| 140 | - | // failure limiter uses); it bounds how many sign-ins a client keeps waiting. | |||
| 141 | BeginLogin(ctx context.Context, client string) (ceremony string, options []byte, err error) | 141 | BeginLogin(ctx context.Context, client string) (ceremony string, options []byte, err error) | ||
| 142 | // FinishLogin checks the browser's response and returns the session token | 142 | // FinishLogin checks the browser's response and returns the session token | ||
| 143 | // and how long it lasts, or ErrLinkRejected for any refusal. | 143 | // and how long it lasts, or ErrLinkRejected for any refusal. | ||
| 144 | FinishLogin(ctx context.Context, ceremony string, response []byte) (token string, lifetime time.Duration, err error) | 144 | FinishLogin(ctx context.Context, ceremony string, response []byte) (token string, lifetime time.Duration, err error) | ||
| 145 | // BeginRegistration starts adding a passkey. Only the owner may. name is | 145 | // BeginRegistration starts adding a passkey. Only the owner may. name is | ||
| 146 | // the name the passkey will get (empty means the default): a name that | 146 | // the name the passkey will get (empty means the default): a name that | ||
| 147 | - | // breaks the rules is refused before any ceremony starts. | 147 | + | // breaks the rules is refused here, before any ceremony starts, and is the |
| 148 | + | // one FinishRegistration stores. | |||
| 148 | BeginRegistration(ctx context.Context, p domain.Principal, name string) (ceremony string, options []byte, err error) | 149 | BeginRegistration(ctx context.Context, p domain.Principal, name string) (ceremony string, options []byte, err error) | ||
| 149 | - | // FinishRegistration checks the browser's response and stores the | 150 | + | // FinishRegistration checks the browser's response and stores the passkey |
| 150 | - | // passkey under name (empty means the default name). | 151 | + | // under the name given to BeginRegistration. |
| 151 | - | FinishRegistration(ctx context.Context, p domain.Principal, ceremony, name string, response []byte) error | 152 | + | FinishRegistration(ctx context.Context, p domain.Principal, ceremony string, response []byte) error |
| 152 | } | 153 | } | ||
| 153 | 154 | ||||
| 154 | // PushService opens a repository for a push: it is domain.ErrUnauthenticated | 155 | // PushService opens a repository for a push: it is domain.ErrUnauthenticated | ||
M internal/usecase/passkeys.go +57 -94
| @@ -3,7 +3,6 @@ | |||
| 3 | 3 | package usecase | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | - | "bytes" | |
| 7 | 6 | "context" | |
| 8 | 7 | "crypto/rand" | |
| 9 | 8 | "crypto/sha256" | |
| @@ -11,7 +10,6 @@ import ( | |||
| 11 | 10 | "encoding/hex" | |
| 12 | 11 | "errors" | |
| 13 | 12 | "fmt" | |
| 14 | - | "net/netip" | |
| 15 | 13 | "sync" | |
| 16 | 14 | "time" | |
| 17 | 15 | ||
| @@ -24,8 +22,6 @@ import ( | |||
| 24 | 22 | // Owner passkeys (ADR 0026). A ceremony is a registration or a login that the | |
| 25 | 23 | // browser carries out between two requests; its data lives in memory only. | |
| 26 | 24 | const ( | |
| 27 | - | // PasskeyCeremonyTTL is how long a ceremony may take. | |
| 28 | - | PasskeyCeremonyTTL = 2 * time.Minute | |
| 29 | 25 | // MaxPendingPasskeyRegistrations bounds the registrations that wait to be | |
| 30 | 26 | // finished. Only the owner starts one. | |
| 31 | 27 | MaxPendingPasskeyRegistrations = 16 | |
| @@ -33,8 +29,8 @@ const ( | |||
| 33 | 29 | // Anyone may start one, so a login is never refused: on overflow one is | |
| 34 | 30 | // dropped instead, and anonymous callers cannot lock the owner out. | |
| 35 | 31 | MaxPendingPasskeyLogins = 256 | |
| 36 | - | // MaxPendingPasskeyLoginsPerClient bounds the logins one client (see | |
| 37 | - | // clientKey) may have waiting. A client over it loses its own oldest. | |
| 32 | + | // MaxPendingPasskeyLoginsPerClient bounds the logins one client (one client | |
| 33 | + | // key) may have waiting. A client over it loses its own oldest. | |
| 38 | 34 | MaxPendingPasskeyLoginsPerClient = 8 | |
| 39 | 35 | ||
| 40 | 36 | // ceremonyIDBytes is the size of a ceremony identifier: 256 bits. | |
| @@ -53,8 +49,9 @@ type ceremony struct { | |||
| 53 | 49 | kind ceremonyKind | |
| 54 | 50 | data []byte // opaque, from the verifier | |
| 55 | 51 | expires time.Time | |
| 56 | - | seq uint64 // creation order | |
| 57 | - | client string // who began a login (clientKey); "" for a registration | |
| 52 | + | seq uint64 // creation order | |
| 53 | + | client string // who began a login (its client key); "" for a registration | |
| 54 | + | name domain.PasskeyName // what a registration will call the passkey | |
| 58 | 55 | } | |
| 59 | 56 | ||
| 60 | 57 | // Passkeys are the use cases of the owner's passkeys: registering, signing in, | |
| @@ -82,38 +79,17 @@ func NewPasskeys(store ports.PasskeyStore, verifier ports.PasskeyVerifier, sessi | |||
| 82 | 79 | return &Passkeys{store: store, verifier: verifier, sessions: sessions, clock: clock, ceremonies: map[[sha256.Size]byte]ceremony{}} | |
| 83 | 80 | } | |
| 84 | 81 | ||
| 85 | - | // clientKey is what a login ceremony is attributed to: the address of the | |
| 86 | - | // client as the web resolved it (trusted proxies applied), an IPv6 address | |
| 87 | - | // reduced to its /64 network, since one host owns a whole /64. Anything that is | |
| 88 | - | // not an address is kept as it is. | |
| 89 | - | func clientKey(addr string) string { | |
| 90 | - | a, err := netip.ParseAddr(addr) | |
| 91 | - | if err != nil { | |
| 92 | - | return addr | |
| 93 | - | } | |
| 94 | - | a = a.Unmap() | |
| 95 | - | if a.Is6() { | |
| 96 | - | p, err := a.Prefix(64) | |
| 97 | - | if err != nil { | |
| 98 | - | return addr | |
| 99 | - | } | |
| 100 | - | return p.String() | |
| 101 | - | } | |
| 102 | - | return a.String() | |
| 103 | - | } | |
| 104 | - | ||
| 105 | 82 | // pendingLogins is what begin knows of the logins that wait, per client. | |
| 106 | 83 | type pendingLogins struct { | |
| 107 | - | count int | |
| 108 | - | oldest [sha256.Size]byte | |
| 109 | - | oldestSeq uint64 | |
| 110 | - | hasOldestSeq bool | |
| 84 | + | count int | |
| 85 | + | oldest [sha256.Size]byte | |
| 86 | + | oldestSeq uint64 // 0 until a login is added: sequence numbers start at 1 | |
| 111 | 87 | } | |
| 112 | 88 | ||
| 113 | 89 | func (p *pendingLogins) add(h [sha256.Size]byte, seq uint64) { | |
| 114 | 90 | p.count++ | |
| 115 | - | if !p.hasOldestSeq || seq < p.oldestSeq { | |
| 116 | - | p.oldest, p.oldestSeq, p.hasOldestSeq = h, seq, true | |
| 91 | + | if p.oldestSeq == 0 || seq < p.oldestSeq { | |
| 92 | + | p.oldest, p.oldestSeq = h, seq | |
| 117 | 93 | } | |
| 118 | 94 | } | |
| 119 | 95 | ||
| @@ -123,7 +99,7 @@ func (p *pendingLogins) add(h [sha256.Size]byte, seq uint64) { | |||
| 123 | 99 | // per-client cap it drops that client's own oldest; over the global cap it drops | |
| 124 | 100 | // the oldest of the client with the most logins waiting. A client that floods | |
| 125 | 101 | // therefore only ever loses its own ceremonies, never another client's. | |
| 126 | - | func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, error) { | |
| 102 | + | func (s *Passkeys) begin(c ceremony) (string, error) { | |
| 127 | 103 | var raw [ceremonyIDBytes]byte | |
| 128 | 104 | if _, err := rand.Read(raw[:]); err != nil { | |
| 129 | 105 | return "", fmt.Errorf("generate passkey ceremony: %w", err) | |
| @@ -154,19 +130,19 @@ func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, | |||
| 154 | 130 | pl.add(h, c.seq) | |
| 155 | 131 | } | |
| 156 | 132 | } | |
| 157 | - | switch kind { | |
| 133 | + | switch c.kind { | |
| 158 | 134 | case ceremonyRegister: | |
| 159 | 135 | if registrations >= MaxPendingPasskeyRegistrations { | |
| 160 | 136 | return "", fmt.Errorf("%w: %d passkey registrations are already pending, finish one or wait two minutes", domain.ErrResourceExhausted, MaxPendingPasskeyRegistrations) | |
| 161 | 137 | } | |
| 162 | 138 | case ceremonyLogin: | |
| 163 | - | if pl := byClient[client]; pl != nil && pl.count >= MaxPendingPasskeyLoginsPerClient { | |
| 139 | + | if pl := byClient[c.client]; pl != nil && pl.count >= MaxPendingPasskeyLoginsPerClient { | |
| 164 | 140 | delete(s.ceremonies, pl.oldest) | |
| 165 | 141 | logins-- | |
| 166 | 142 | // The client is back under its cap; the global step below needs | |
| 167 | 143 | // its new count and its new oldest. | |
| 168 | 144 | pl.count-- | |
| 169 | - | s.refreshOldest(pl, client) | |
| 145 | + | s.refreshOldest(pl, c.client) | |
| 170 | 146 | } | |
| 171 | 147 | if logins >= MaxPendingPasskeyLogins { | |
| 172 | 148 | // The client with the most waiting loses its oldest; among equals, | |
| @@ -186,50 +162,52 @@ func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, | |||
| 186 | 162 | } | |
| 187 | 163 | } | |
| 188 | 164 | s.seq++ | |
| 189 | - | s.ceremonies[sha256.Sum256([]byte(id))] = ceremony{kind: kind, data: data, expires: now.Add(PasskeyCeremonyTTL), seq: s.seq, client: client} | |
| 165 | + | c.expires, c.seq = now.Add(domain.PasskeyCeremonyTTL), s.seq | |
| 166 | + | s.ceremonies[sha256.Sum256([]byte(id))] = c | |
| 190 | 167 | return id, nil | |
| 191 | 168 | } | |
| 192 | 169 | ||
| 193 | 170 | // refreshOldest recomputes the oldest login of a client after one was dropped. | |
| 194 | 171 | // The caller holds s.mu. | |
| 195 | 172 | func (s *Passkeys) refreshOldest(pl *pendingLogins, client string) { | |
| 196 | - | pl.hasOldestSeq = false | |
| 173 | + | pl.oldestSeq = 0 | |
| 197 | 174 | for h, c := range s.ceremonies { | |
| 198 | 175 | if c.kind != ceremonyLogin || c.client != client { | |
| 199 | 176 | continue | |
| 200 | 177 | } | |
| 201 | - | if !pl.hasOldestSeq || c.seq < pl.oldestSeq { | |
| 202 | - | pl.oldest, pl.oldestSeq, pl.hasOldestSeq = h, c.seq, true | |
| 178 | + | if pl.oldestSeq == 0 || c.seq < pl.oldestSeq { | |
| 179 | + | pl.oldest, pl.oldestSeq = h, c.seq | |
| 203 | 180 | } | |
| 204 | 181 | } | |
| 205 | 182 | } | |
| 206 | 183 | ||
| 207 | - | // take removes the ceremony and returns its data. It is single use, also when | |
| 208 | - | // the ceremony expired or is of another kind. ok is false for an unknown, | |
| 209 | - | // expired or mismatched ceremony. | |
| 210 | - | func (s *Passkeys) take(id string, kind ceremonyKind) (data []byte, ok bool) { | |
| 184 | + | // take removes the ceremony and returns it. It is single use, also when the | |
| 185 | + | // ceremony expired or is of another kind. ok is false for an unknown, expired or | |
| 186 | + | // mismatched ceremony. | |
| 187 | + | func (s *Passkeys) take(id string, kind ceremonyKind) (c ceremony, ok bool) { | |
| 211 | 188 | h := sha256.Sum256([]byte(id)) | |
| 212 | 189 | s.mu.Lock() | |
| 213 | 190 | c, found := s.ceremonies[h] | |
| 214 | 191 | delete(s.ceremonies, h) | |
| 215 | 192 | s.mu.Unlock() | |
| 216 | 193 | if !found || c.kind != kind || !s.clock.Now().Before(c.expires) { | |
| 217 | - | return nil, false | |
| 194 | + | return ceremony{}, false | |
| 218 | 195 | } | |
| 219 | - | return c.data, true | |
| 196 | + | return c, true | |
| 220 | 197 | } | |
| 221 | 198 | ||
| 222 | 199 | // BeginRegistration starts adding a passkey. Only the owner may. name is | |
| 223 | - | // checked now, so that a name the browser would be asked about and then refused | |
| 224 | - | // costs no ceremony (an empty name means the default); finish checks it again. | |
| 225 | - | // It gives domain.ErrResourceExhausted when MaxPasskeys passkeys exist or too | |
| 200 | + | // checked now (an empty name means the default), so that a name the owner would | |
| 201 | + | // be prompted for and then see refused costs no ceremony; the ceremony keeps the | |
| 202 | + | // checked name and FinishRegistration stores it. It gives domain.ErrResourceExhausted when MaxPasskeys passkeys exist or too | |
| 226 | 203 | // many registrations wait. It returns the ceremony identifier and the options | |
| 227 | 204 | // for the browser. | |
| 228 | 205 | func (s *Passkeys) BeginRegistration(ctx context.Context, p domain.Principal, name string) (string, []byte, error) { | |
| 229 | 206 | if err := requireOwner(p); err != nil { | |
| 230 | 207 | return "", nil, err | |
| 231 | 208 | } | |
| 232 | - | if _, err := parsePasskeyName(name); err != nil { | |
| 209 | + | parsed, err := parsePasskeyName(name) | |
| 210 | + | if err != nil { | |
| 233 | 211 | return "", nil, err | |
| 234 | 212 | } | |
| 235 | 213 | existing, err := s.store.List(ctx) | |
| @@ -247,50 +225,45 @@ func (s *Passkeys) BeginRegistration(ctx context.Context, p domain.Principal, na | |||
| 247 | 225 | if err != nil { | |
| 248 | 226 | return "", nil, err | |
| 249 | 227 | } | |
| 250 | - | id, err := s.begin(ceremonyRegister, "", data) | |
| 228 | + | id, err := s.begin(ceremony{kind: ceremonyRegister, data: data, name: parsed}) | |
| 251 | 229 | if err != nil { | |
| 252 | 230 | return "", nil, err | |
| 253 | 231 | } | |
| 254 | 232 | return id, options, nil | |
| 255 | 233 | } | |
| 256 | 234 | ||
| 257 | - | // FinishRegistration checks the browser's response and stores the passkey | |
| 258 | - | // under name; an empty name means domain.DefaultPasskeyName. A ceremony that is | |
| 259 | - | // unknown, expired, used or of the wrong kind, and a response the verifier | |
| 260 | - | // refuses, give domain.ErrUnauthenticated; a bad name matches | |
| 261 | - | // domain.ErrInvalidText; a credential already registered gives | |
| 235 | + | // FinishRegistration checks the browser's response and stores the passkey under | |
| 236 | + | // the name BeginRegistration checked. A ceremony that is unknown, expired, used | |
| 237 | + | // or of the wrong kind, and a response the verifier refuses, give | |
| 238 | + | // domain.ErrUnauthenticated; a credential already registered gives | |
| 262 | 239 | // domain.ErrAlreadyExists; too many passkeys, domain.ErrResourceExhausted. | |
| 263 | - | func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, ceremonyID, name string, response []byte) (domain.Passkey, error) { | |
| 240 | + | func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, ceremonyID string, response []byte) error { | |
| 264 | 241 | if err := requireOwner(p); err != nil { | |
| 265 | - | return domain.Passkey{}, err | |
| 266 | - | } | |
| 267 | - | parsed, err := parsePasskeyName(name) // before take: a bad name does not burn the ceremony | |
| 268 | - | if err != nil { | |
| 269 | - | return domain.Passkey{}, err | |
| 242 | + | return err | |
| 270 | 243 | } | |
| 271 | 244 | log := zerowrap.FromCtx(ctx) | |
| 272 | - | data, ok := s.take(ceremonyID, ceremonyRegister) | |
| 245 | + | c, ok := s.take(ceremonyID, ceremonyRegister) | |
| 273 | 246 | if !ok { | |
| 274 | 247 | err := fmt.Errorf("%w: passkey registration expired or unknown, start again", domain.ErrUnauthenticated) | |
| 275 | 248 | log.Info().Err(err).Msg("passkey registration refused") | |
| 276 | - | return domain.Passkey{}, err | |
| 249 | + | return err | |
| 277 | 250 | } | |
| 278 | 251 | handle, err := s.store.UserHandle(ctx) | |
| 279 | 252 | if err != nil { | |
| 280 | - | return domain.Passkey{}, fmt.Errorf("load passkey user handle: %w", err) | |
| 253 | + | return fmt.Errorf("load passkey user handle: %w", err) | |
| 281 | 254 | } | |
| 282 | - | pk, err := s.verifier.FinishRegistration(handle, data, response) | |
| 255 | + | pk, err := s.verifier.FinishRegistration(handle, c.data, response) | |
| 283 | 256 | if err != nil { | |
| 284 | 257 | if errors.Is(err, domain.ErrUnauthenticated) { | |
| 285 | 258 | // The reason is for the log (it holds no key); the caller gets a 400. | |
| 286 | 259 | log.Info().Err(err).Msg("passkey registration refused") | |
| 287 | 260 | } | |
| 288 | - | return domain.Passkey{}, err | |
| 261 | + | return err | |
| 289 | 262 | } | |
| 290 | 263 | if n := len(pk.ID); n == 0 || n > domain.MaxPasskeyIDLen { | |
| 291 | - | return domain.Passkey{}, fmt.Errorf("%w: credential id has %d bytes", domain.ErrUnauthenticated, n) | |
| 264 | + | return fmt.Errorf("%w: credential id has %d bytes", domain.ErrUnauthenticated, n) | |
| 292 | 265 | } | |
| 293 | - | pk.Name = parsed.String() | |
| 266 | + | pk.Name = c.name.String() | |
| 294 | 267 | pk.CreatedAt = s.clock.Now().UTC() | |
| 295 | 268 | pk.LastUsedAt = time.Time{} | |
| 296 | 269 | ||
| @@ -298,28 +271,28 @@ func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, c | |||
| 298 | 271 | defer s.writeMu.Unlock() | |
| 299 | 272 | existing, err := s.store.List(ctx) | |
| 300 | 273 | if err != nil { | |
| 301 | - | return domain.Passkey{}, fmt.Errorf("list passkeys: %w", err) | |
| 274 | + | return fmt.Errorf("list passkeys: %w", err) | |
| 302 | 275 | } | |
| 303 | 276 | if len(existing) >= domain.MaxPasskeys { | |
| 304 | - | return domain.Passkey{}, fmt.Errorf("%w: %d passkeys are registered, remove one first", domain.ErrResourceExhausted, domain.MaxPasskeys) | |
| 277 | + | return fmt.Errorf("%w: %d passkeys are registered, remove one first", domain.ErrResourceExhausted, domain.MaxPasskeys) | |
| 305 | 278 | } | |
| 306 | 279 | if err := s.store.Create(ctx, pk); err != nil { | |
| 307 | - | return domain.Passkey{}, err | |
| 280 | + | return err | |
| 308 | 281 | } | |
| 309 | 282 | log.Info().Str("passkey", passkeyRef(pk.ID)).Msg("passkey added") | |
| 310 | - | return pk, nil | |
| 283 | + | return nil | |
| 311 | 284 | } | |
| 312 | 285 | ||
| 313 | 286 | // BeginLogin starts a passkey sign-in. Anonymous callers may. client is the | |
| 314 | - | // address the web resolved for the caller (the one the failure limiter uses); | |
| 315 | - | // it bounds how many sign-ins one client keeps waiting. It returns the | |
| 287 | + | // client key of the caller (clientlimit.Key of the address the failure limiter | |
| 288 | + | // uses); it bounds how many sign-ins one client keeps waiting. It returns the | |
| 316 | 289 | // ceremony identifier and the options for the browser. | |
| 317 | 290 | func (s *Passkeys) BeginLogin(_ context.Context, client string) (string, []byte, error) { | |
| 318 | 291 | options, data, err := s.verifier.BeginLogin() | |
| 319 | 292 | if err != nil { | |
| 320 | 293 | return "", nil, err | |
| 321 | 294 | } | |
| 322 | - | id, err := s.begin(ceremonyLogin, clientKey(client), data) | |
| 295 | + | id, err := s.begin(ceremony{kind: ceremonyLogin, data: data, client: client}) | |
| 323 | 296 | if err != nil { | |
| 324 | 297 | return "", nil, err | |
| 325 | 298 | } | |
| @@ -333,7 +306,7 @@ func (s *Passkeys) BeginLogin(_ context.Context, client string) (string, []byte, | |||
| 333 | 306 | // store is returned as it is, so that the caller fails closed. | |
| 334 | 307 | func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response []byte) (token string, expiresAt time.Time, err error) { | |
| 335 | 308 | log := zerowrap.FromCtx(ctx) | |
| 336 | - | data, ok := s.take(ceremonyID, ceremonyLogin) | |
| 309 | + | c, ok := s.take(ceremonyID, ceremonyLogin) | |
| 337 | 310 | if !ok { | |
| 338 | 311 | log.Info().Msg("passkey sign-in refused: the ceremony expired or is unknown") | |
| 339 | 312 | return "", time.Time{}, domain.ErrUnauthenticated | |
| @@ -342,19 +315,8 @@ func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response | |||
| 342 | 315 | if err != nil { | |
| 343 | 316 | return "", time.Time{}, fmt.Errorf("load passkey user handle: %w", err) | |
| 344 | 317 | } | |
| 345 | - | var looked []byte // the credential the response named, for the log | |
| 346 | - | pk, err := s.verifier.FinishLogin(handle, data, response, func(id []byte) (domain.Passkey, error) { | |
| 347 | - | looked = bytes.Clone(id) | |
| 348 | - | return s.store.Get(ctx, id) | |
| 349 | - | }) | |
| 318 | + | pk, err := s.verifier.FinishLogin(handle, c.data, response, func(id []byte) (domain.Passkey, error) { return s.store.Get(ctx, id) }) | |
| 350 | 319 | if err != nil { | |
| 351 | - | if errors.Is(err, domain.ErrPasskeyCloned) { | |
| 352 | - | ev := log.Warn().Err(err) | |
| 353 | - | if looked != nil { | |
| 354 | - | ev = ev.Str("passkey", passkeyRef(looked)) | |
| 355 | - | } | |
| 356 | - | ev.Msg("passkey sign-in refused: the counter did not advance, the authenticator may be cloned") | |
| 357 | - | } | |
| 358 | 320 | if errors.Is(err, domain.ErrUnauthenticated) { | |
| 359 | 321 | // The reason is for the log (it holds no key); the caller learns nothing. | |
| 360 | 322 | log.Info().Err(err).Msg("passkey sign-in refused") | |
| @@ -376,9 +338,10 @@ func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response | |||
| 376 | 338 | } | |
| 377 | 339 | return "", time.Time{}, fmt.Errorf("read passkey: %w", err) | |
| 378 | 340 | } | |
| 379 | - | // The verifier compared the counter with the one it was shown; another | |
| 380 | - | // login may have moved it since. A counter that does not advance is the | |
| 381 | - | // clone signal (synced passkeys report 0 and are exempt). | |
| 341 | + | // The one sign-count check. The verifier returns the counter the response | |
| 342 | + | // reports (the stored one when it does not advance), and another login may | |
| 343 | + | // have moved the stored one since it looked. A counter that does not advance | |
| 344 | + | // is the clone signal; synced passkeys report 0 and are exempt. | |
| 382 | 345 | if cur.SignCount != 0 && pk.SignCount <= cur.SignCount { | |
| 383 | 346 | log.Warn().Str("passkey", passkeyRef(pk.ID)).Msg("passkey sign-in refused: the counter did not advance, the authenticator may be cloned") | |
| 384 | 347 | return "", time.Time{}, domain.ErrUnauthenticated | |
| @@ -3,7 +3,6 @@ | |||||
| 3 | package usecase | 3 | package usecase | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | - | "bytes" | |||
| 7 | "context" | 6 | "context" | ||
| 8 | "crypto/rand" | 7 | "crypto/rand" | ||
| 9 | "crypto/sha256" | 8 | "crypto/sha256" | ||
| @@ -11,7 +10,6 @@ import ( | |||||
| 11 | "encoding/hex" | 10 | "encoding/hex" | ||
| 12 | "errors" | 11 | "errors" | ||
| 13 | "fmt" | 12 | "fmt" | ||
| 14 | - | "net/netip" | |||
| 15 | "sync" | 13 | "sync" | ||
| 16 | "time" | 14 | "time" | ||
| 17 | 15 | ||||
| @@ -24,8 +22,6 @@ import ( | |||||
| 24 | // Owner passkeys (ADR 0026). A ceremony is a registration or a login that the | 22 | // Owner passkeys (ADR 0026). A ceremony is a registration or a login that the | ||
| 25 | // browser carries out between two requests; its data lives in memory only. | 23 | // browser carries out between two requests; its data lives in memory only. | ||
| 26 | const ( | 24 | const ( | ||
| 27 | - | // PasskeyCeremonyTTL is how long a ceremony may take. | |||
| 28 | - | PasskeyCeremonyTTL = 2 * time.Minute | |||
| 29 | // MaxPendingPasskeyRegistrations bounds the registrations that wait to be | 25 | // MaxPendingPasskeyRegistrations bounds the registrations that wait to be | ||
| 30 | // finished. Only the owner starts one. | 26 | // finished. Only the owner starts one. | ||
| 31 | MaxPendingPasskeyRegistrations = 16 | 27 | MaxPendingPasskeyRegistrations = 16 | ||
| @@ -33,8 +29,8 @@ const ( | |||||
| 33 | // Anyone may start one, so a login is never refused: on overflow one is | 29 | // Anyone may start one, so a login is never refused: on overflow one is | ||
| 34 | // dropped instead, and anonymous callers cannot lock the owner out. | 30 | // dropped instead, and anonymous callers cannot lock the owner out. | ||
| 35 | MaxPendingPasskeyLogins = 256 | 31 | MaxPendingPasskeyLogins = 256 | ||
| 36 | - | // MaxPendingPasskeyLoginsPerClient bounds the logins one client (see | 32 | + | // MaxPendingPasskeyLoginsPerClient bounds the logins one client (one client |
| 37 | - | // clientKey) may have waiting. A client over it loses its own oldest. | 33 | + | // key) may have waiting. A client over it loses its own oldest. |
| 38 | MaxPendingPasskeyLoginsPerClient = 8 | 34 | MaxPendingPasskeyLoginsPerClient = 8 | ||
| 39 | 35 | ||||
| 40 | // ceremonyIDBytes is the size of a ceremony identifier: 256 bits. | 36 | // ceremonyIDBytes is the size of a ceremony identifier: 256 bits. | ||
| @@ -53,8 +49,9 @@ type ceremony struct { | |||||
| 53 | kind ceremonyKind | 49 | kind ceremonyKind | ||
| 54 | data []byte // opaque, from the verifier | 50 | data []byte // opaque, from the verifier | ||
| 55 | expires time.Time | 51 | expires time.Time | ||
| 56 | - | seq uint64 // creation order | 52 | + | seq uint64 // creation order |
| 57 | - | client string // who began a login (clientKey); "" for a registration | 53 | + | client string // who began a login (its client key); "" for a registration |
| 54 | + | name domain.PasskeyName // what a registration will call the passkey | |||
| 58 | } | 55 | } | ||
| 59 | 56 | ||||
| 60 | // Passkeys are the use cases of the owner's passkeys: registering, signing in, | 57 | // Passkeys are the use cases of the owner's passkeys: registering, signing in, | ||
| @@ -82,38 +79,17 @@ func NewPasskeys(store ports.PasskeyStore, verifier ports.PasskeyVerifier, sessi | |||||
| 82 | return &Passkeys{store: store, verifier: verifier, sessions: sessions, clock: clock, ceremonies: map[[sha256.Size]byte]ceremony{}} | 79 | return &Passkeys{store: store, verifier: verifier, sessions: sessions, clock: clock, ceremonies: map[[sha256.Size]byte]ceremony{}} | ||
| 83 | } | 80 | } | ||
| 84 | 81 | ||||
| 85 | - | // clientKey is what a login ceremony is attributed to: the address of the | |||
| 86 | - | // client as the web resolved it (trusted proxies applied), an IPv6 address | |||
| 87 | - | // reduced to its /64 network, since one host owns a whole /64. Anything that is | |||
| 88 | - | // not an address is kept as it is. | |||
| 89 | - | func clientKey(addr string) string { | |||
| 90 | - | a, err := netip.ParseAddr(addr) | |||
| 91 | - | if err != nil { | |||
| 92 | - | return addr | |||
| 93 | - | } | |||
| 94 | - | a = a.Unmap() | |||
| 95 | - | if a.Is6() { | |||
| 96 | - | p, err := a.Prefix(64) | |||
| 97 | - | if err != nil { | |||
| 98 | - | return addr | |||
| 99 | - | } | |||
| 100 | - | return p.String() | |||
| 101 | - | } | |||
| 102 | - | return a.String() | |||
| 103 | - | } | |||
| 104 | - | ||||
| 105 | // pendingLogins is what begin knows of the logins that wait, per client. | 82 | // pendingLogins is what begin knows of the logins that wait, per client. | ||
| 106 | type pendingLogins struct { | 83 | type pendingLogins struct { | ||
| 107 | - | count int | 84 | + | count int |
| 108 | - | oldest [sha256.Size]byte | 85 | + | oldest [sha256.Size]byte |
| 109 | - | oldestSeq uint64 | 86 | + | oldestSeq uint64 // 0 until a login is added: sequence numbers start at 1 |
| 110 | - | hasOldestSeq bool | |||
| 111 | } | 87 | } | ||
| 112 | 88 | ||||
| 113 | func (p *pendingLogins) add(h [sha256.Size]byte, seq uint64) { | 89 | func (p *pendingLogins) add(h [sha256.Size]byte, seq uint64) { | ||
| 114 | p.count++ | 90 | p.count++ | ||
| 115 | - | if !p.hasOldestSeq || seq < p.oldestSeq { | 91 | + | if p.oldestSeq == 0 || seq < p.oldestSeq { |
| 116 | - | p.oldest, p.oldestSeq, p.hasOldestSeq = h, seq, true | 92 | + | p.oldest, p.oldestSeq = h, seq |
| 117 | } | 93 | } | ||
| 118 | } | 94 | } | ||
| 119 | 95 | ||||
| @@ -123,7 +99,7 @@ func (p *pendingLogins) add(h [sha256.Size]byte, seq uint64) { | |||||
| 123 | // per-client cap it drops that client's own oldest; over the global cap it drops | 99 | // per-client cap it drops that client's own oldest; over the global cap it drops | ||
| 124 | // the oldest of the client with the most logins waiting. A client that floods | 100 | // the oldest of the client with the most logins waiting. A client that floods | ||
| 125 | // therefore only ever loses its own ceremonies, never another client's. | 101 | // therefore only ever loses its own ceremonies, never another client's. | ||
| 126 | - | func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, error) { | 102 | + | func (s *Passkeys) begin(c ceremony) (string, error) { |
| 127 | var raw [ceremonyIDBytes]byte | 103 | var raw [ceremonyIDBytes]byte | ||
| 128 | if _, err := rand.Read(raw[:]); err != nil { | 104 | if _, err := rand.Read(raw[:]); err != nil { | ||
| 129 | return "", fmt.Errorf("generate passkey ceremony: %w", err) | 105 | return "", fmt.Errorf("generate passkey ceremony: %w", err) | ||
| @@ -154,19 +130,19 @@ func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, | |||||
| 154 | pl.add(h, c.seq) | 130 | pl.add(h, c.seq) | ||
| 155 | } | 131 | } | ||
| 156 | } | 132 | } | ||
| 157 | - | switch kind { | 133 | + | switch c.kind { |
| 158 | case ceremonyRegister: | 134 | case ceremonyRegister: | ||
| 159 | if registrations >= MaxPendingPasskeyRegistrations { | 135 | if registrations >= MaxPendingPasskeyRegistrations { | ||
| 160 | return "", fmt.Errorf("%w: %d passkey registrations are already pending, finish one or wait two minutes", domain.ErrResourceExhausted, MaxPendingPasskeyRegistrations) | 136 | return "", fmt.Errorf("%w: %d passkey registrations are already pending, finish one or wait two minutes", domain.ErrResourceExhausted, MaxPendingPasskeyRegistrations) | ||
| 161 | } | 137 | } | ||
| 162 | case ceremonyLogin: | 138 | case ceremonyLogin: | ||
| 163 | - | if pl := byClient[client]; pl != nil && pl.count >= MaxPendingPasskeyLoginsPerClient { | 139 | + | if pl := byClient[c.client]; pl != nil && pl.count >= MaxPendingPasskeyLoginsPerClient { |
| 164 | delete(s.ceremonies, pl.oldest) | 140 | delete(s.ceremonies, pl.oldest) | ||
| 165 | logins-- | 141 | logins-- | ||
| 166 | // The client is back under its cap; the global step below needs | 142 | // The client is back under its cap; the global step below needs | ||
| 167 | // its new count and its new oldest. | 143 | // its new count and its new oldest. | ||
| 168 | pl.count-- | 144 | pl.count-- | ||
| 169 | - | s.refreshOldest(pl, client) | 145 | + | s.refreshOldest(pl, c.client) |
| 170 | } | 146 | } | ||
| 171 | if logins >= MaxPendingPasskeyLogins { | 147 | if logins >= MaxPendingPasskeyLogins { | ||
| 172 | // The client with the most waiting loses its oldest; among equals, | 148 | // The client with the most waiting loses its oldest; among equals, | ||
| @@ -186,50 +162,52 @@ func (s *Passkeys) begin(kind ceremonyKind, client string, data []byte) (string, | |||||
| 186 | } | 162 | } | ||
| 187 | } | 163 | } | ||
| 188 | s.seq++ | 164 | s.seq++ | ||
| 189 | - | s.ceremonies[sha256.Sum256([]byte(id))] = ceremony{kind: kind, data: data, expires: now.Add(PasskeyCeremonyTTL), seq: s.seq, client: client} | 165 | + | c.expires, c.seq = now.Add(domain.PasskeyCeremonyTTL), s.seq |
| 166 | + | s.ceremonies[sha256.Sum256([]byte(id))] = c | |||
| 190 | return id, nil | 167 | return id, nil | ||
| 191 | } | 168 | } | ||
| 192 | 169 | ||||
| 193 | // refreshOldest recomputes the oldest login of a client after one was dropped. | 170 | // refreshOldest recomputes the oldest login of a client after one was dropped. | ||
| 194 | // The caller holds s.mu. | 171 | // The caller holds s.mu. | ||
| 195 | func (s *Passkeys) refreshOldest(pl *pendingLogins, client string) { | 172 | func (s *Passkeys) refreshOldest(pl *pendingLogins, client string) { | ||
| 196 | - | pl.hasOldestSeq = false | 173 | + | pl.oldestSeq = 0 |
| 197 | for h, c := range s.ceremonies { | 174 | for h, c := range s.ceremonies { | ||
| 198 | if c.kind != ceremonyLogin || c.client != client { | 175 | if c.kind != ceremonyLogin || c.client != client { | ||
| 199 | continue | 176 | continue | ||
| 200 | } | 177 | } | ||
| 201 | - | if !pl.hasOldestSeq || c.seq < pl.oldestSeq { | 178 | + | if pl.oldestSeq == 0 || c.seq < pl.oldestSeq { |
| 202 | - | pl.oldest, pl.oldestSeq, pl.hasOldestSeq = h, c.seq, true | 179 | + | pl.oldest, pl.oldestSeq = h, c.seq |
| 203 | } | 180 | } | ||
| 204 | } | 181 | } | ||
| 205 | } | 182 | } | ||
| 206 | 183 | ||||
| 207 | - | // take removes the ceremony and returns its data. It is single use, also when | 184 | + | // take removes the ceremony and returns it. It is single use, also when the |
| 208 | - | // the ceremony expired or is of another kind. ok is false for an unknown, | 185 | + | // ceremony expired or is of another kind. ok is false for an unknown, expired or |
| 209 | - | // expired or mismatched ceremony. | 186 | + | // mismatched ceremony. |
| 210 | - | func (s *Passkeys) take(id string, kind ceremonyKind) (data []byte, ok bool) { | 187 | + | func (s *Passkeys) take(id string, kind ceremonyKind) (c ceremony, ok bool) { |
| 211 | h := sha256.Sum256([]byte(id)) | 188 | h := sha256.Sum256([]byte(id)) | ||
| 212 | s.mu.Lock() | 189 | s.mu.Lock() | ||
| 213 | c, found := s.ceremonies[h] | 190 | c, found := s.ceremonies[h] | ||
| 214 | delete(s.ceremonies, h) | 191 | delete(s.ceremonies, h) | ||
| 215 | s.mu.Unlock() | 192 | s.mu.Unlock() | ||
| 216 | if !found || c.kind != kind || !s.clock.Now().Before(c.expires) { | 193 | if !found || c.kind != kind || !s.clock.Now().Before(c.expires) { | ||
| 217 | - | return nil, false | 194 | + | return ceremony{}, false |
| 218 | } | 195 | } | ||
| 219 | - | return c.data, true | 196 | + | return c, true |
| 220 | } | 197 | } | ||
| 221 | 198 | ||||
| 222 | // BeginRegistration starts adding a passkey. Only the owner may. name is | 199 | // BeginRegistration starts adding a passkey. Only the owner may. name is | ||
| 223 | - | // checked now, so that a name the browser would be asked about and then refused | 200 | + | // checked now (an empty name means the default), so that a name the owner would |
| 224 | - | // costs no ceremony (an empty name means the default); finish checks it again. | 201 | + | // be prompted for and then see refused costs no ceremony; the ceremony keeps the |
| 225 | - | // It gives domain.ErrResourceExhausted when MaxPasskeys passkeys exist or too | 202 | + | // checked name and FinishRegistration stores it. It gives domain.ErrResourceExhausted when MaxPasskeys passkeys exist or too |
| 226 | // many registrations wait. It returns the ceremony identifier and the options | 203 | // many registrations wait. It returns the ceremony identifier and the options | ||
| 227 | // for the browser. | 204 | // for the browser. | ||
| 228 | func (s *Passkeys) BeginRegistration(ctx context.Context, p domain.Principal, name string) (string, []byte, error) { | 205 | func (s *Passkeys) BeginRegistration(ctx context.Context, p domain.Principal, name string) (string, []byte, error) { | ||
| 229 | if err := requireOwner(p); err != nil { | 206 | if err := requireOwner(p); err != nil { | ||
| 230 | return "", nil, err | 207 | return "", nil, err | ||
| 231 | } | 208 | } | ||
| 232 | - | if _, err := parsePasskeyName(name); err != nil { | 209 | + | parsed, err := parsePasskeyName(name) |
| 210 | + | if err != nil { | |||
| 233 | return "", nil, err | 211 | return "", nil, err | ||
| 234 | } | 212 | } | ||
| 235 | existing, err := s.store.List(ctx) | 213 | existing, err := s.store.List(ctx) | ||
| @@ -247,50 +225,45 @@ func (s *Passkeys) BeginRegistration(ctx context.Context, p domain.Principal, na | |||||
| 247 | if err != nil { | 225 | if err != nil { | ||
| 248 | return "", nil, err | 226 | return "", nil, err | ||
| 249 | } | 227 | } | ||
| 250 | - | id, err := s.begin(ceremonyRegister, "", data) | 228 | + | id, err := s.begin(ceremony{kind: ceremonyRegister, data: data, name: parsed}) |
| 251 | if err != nil { | 229 | if err != nil { | ||
| 252 | return "", nil, err | 230 | return "", nil, err | ||
| 253 | } | 231 | } | ||
| 254 | return id, options, nil | 232 | return id, options, nil | ||
| 255 | } | 233 | } | ||
| 256 | 234 | ||||
| 257 | - | // FinishRegistration checks the browser's response and stores the passkey | 235 | + | // FinishRegistration checks the browser's response and stores the passkey under |
| 258 | - | // under name; an empty name means domain.DefaultPasskeyName. A ceremony that is | 236 | + | // the name BeginRegistration checked. A ceremony that is unknown, expired, used |
| 259 | - | // unknown, expired, used or of the wrong kind, and a response the verifier | 237 | + | // or of the wrong kind, and a response the verifier refuses, give |
| 260 | - | // refuses, give domain.ErrUnauthenticated; a bad name matches | 238 | + | // domain.ErrUnauthenticated; a credential already registered gives |
| 261 | - | // domain.ErrInvalidText; a credential already registered gives | |||
| 262 | // domain.ErrAlreadyExists; too many passkeys, domain.ErrResourceExhausted. | 239 | // domain.ErrAlreadyExists; too many passkeys, domain.ErrResourceExhausted. | ||
| 263 | - | func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, ceremonyID, name string, response []byte) (domain.Passkey, error) { | 240 | + | func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, ceremonyID string, response []byte) error { |
| 264 | if err := requireOwner(p); err != nil { | 241 | if err := requireOwner(p); err != nil { | ||
| 265 | - | return domain.Passkey{}, err | 242 | + | return err |
| 266 | - | } | |||
| 267 | - | parsed, err := parsePasskeyName(name) // before take: a bad name does not burn the ceremony | |||
| 268 | - | if err != nil { | |||
| 269 | - | return domain.Passkey{}, err | |||
| 270 | } | 243 | } | ||
| 271 | log := zerowrap.FromCtx(ctx) | 244 | log := zerowrap.FromCtx(ctx) | ||
| 272 | - | data, ok := s.take(ceremonyID, ceremonyRegister) | 245 | + | c, ok := s.take(ceremonyID, ceremonyRegister) |
| 273 | if !ok { | 246 | if !ok { | ||
| 274 | err := fmt.Errorf("%w: passkey registration expired or unknown, start again", domain.ErrUnauthenticated) | 247 | err := fmt.Errorf("%w: passkey registration expired or unknown, start again", domain.ErrUnauthenticated) | ||
| 275 | log.Info().Err(err).Msg("passkey registration refused") | 248 | log.Info().Err(err).Msg("passkey registration refused") | ||
| 276 | - | return domain.Passkey{}, err | 249 | + | return err |
| 277 | } | 250 | } | ||
| 278 | handle, err := s.store.UserHandle(ctx) | 251 | handle, err := s.store.UserHandle(ctx) | ||
| 279 | if err != nil { | 252 | if err != nil { | ||
| 280 | - | return domain.Passkey{}, fmt.Errorf("load passkey user handle: %w", err) | 253 | + | return fmt.Errorf("load passkey user handle: %w", err) |
| 281 | } | 254 | } | ||
| 282 | - | pk, err := s.verifier.FinishRegistration(handle, data, response) | 255 | + | pk, err := s.verifier.FinishRegistration(handle, c.data, response) |
| 283 | if err != nil { | 256 | if err != nil { | ||
| 284 | if errors.Is(err, domain.ErrUnauthenticated) { | 257 | if errors.Is(err, domain.ErrUnauthenticated) { | ||
| 285 | // The reason is for the log (it holds no key); the caller gets a 400. | 258 | // The reason is for the log (it holds no key); the caller gets a 400. | ||
| 286 | log.Info().Err(err).Msg("passkey registration refused") | 259 | log.Info().Err(err).Msg("passkey registration refused") | ||
| 287 | } | 260 | } | ||
| 288 | - | return domain.Passkey{}, err | 261 | + | return err |
| 289 | } | 262 | } | ||
| 290 | if n := len(pk.ID); n == 0 || n > domain.MaxPasskeyIDLen { | 263 | if n := len(pk.ID); n == 0 || n > domain.MaxPasskeyIDLen { | ||
| 291 | - | return domain.Passkey{}, fmt.Errorf("%w: credential id has %d bytes", domain.ErrUnauthenticated, n) | 264 | + | return fmt.Errorf("%w: credential id has %d bytes", domain.ErrUnauthenticated, n) |
| 292 | } | 265 | } | ||
| 293 | - | pk.Name = parsed.String() | 266 | + | pk.Name = c.name.String() |
| 294 | pk.CreatedAt = s.clock.Now().UTC() | 267 | pk.CreatedAt = s.clock.Now().UTC() | ||
| 295 | pk.LastUsedAt = time.Time{} | 268 | pk.LastUsedAt = time.Time{} | ||
| 296 | 269 | ||||
| @@ -298,28 +271,28 @@ func (s *Passkeys) FinishRegistration(ctx context.Context, p domain.Principal, c | |||||
| 298 | defer s.writeMu.Unlock() | 271 | defer s.writeMu.Unlock() | ||
| 299 | existing, err := s.store.List(ctx) | 272 | existing, err := s.store.List(ctx) | ||
| 300 | if err != nil { | 273 | if err != nil { | ||
| 301 | - | return domain.Passkey{}, fmt.Errorf("list passkeys: %w", err) | 274 | + | return fmt.Errorf("list passkeys: %w", err) |
| 302 | } | 275 | } | ||
| 303 | if len(existing) >= domain.MaxPasskeys { | 276 | if len(existing) >= domain.MaxPasskeys { | ||
| 304 | - | return domain.Passkey{}, fmt.Errorf("%w: %d passkeys are registered, remove one first", domain.ErrResourceExhausted, domain.MaxPasskeys) | 277 | + | return fmt.Errorf("%w: %d passkeys are registered, remove one first", domain.ErrResourceExhausted, domain.MaxPasskeys) |
| 305 | } | 278 | } | ||
| 306 | if err := s.store.Create(ctx, pk); err != nil { | 279 | if err := s.store.Create(ctx, pk); err != nil { | ||
| 307 | - | return domain.Passkey{}, err | 280 | + | return err |
| 308 | } | 281 | } | ||
| 309 | log.Info().Str("passkey", passkeyRef(pk.ID)).Msg("passkey added") | 282 | log.Info().Str("passkey", passkeyRef(pk.ID)).Msg("passkey added") | ||
| 310 | - | return pk, nil | 283 | + | return nil |
| 311 | } | 284 | } | ||
| 312 | 285 | ||||
| 313 | // BeginLogin starts a passkey sign-in. Anonymous callers may. client is the | 286 | // BeginLogin starts a passkey sign-in. Anonymous callers may. client is the | ||
| 314 | - | // address the web resolved for the caller (the one the failure limiter uses); | 287 | + | // client key of the caller (clientlimit.Key of the address the failure limiter |
| 315 | - | // it bounds how many sign-ins one client keeps waiting. It returns the | 288 | + | // uses); it bounds how many sign-ins one client keeps waiting. It returns the |
| 316 | // ceremony identifier and the options for the browser. | 289 | // ceremony identifier and the options for the browser. | ||
| 317 | func (s *Passkeys) BeginLogin(_ context.Context, client string) (string, []byte, error) { | 290 | func (s *Passkeys) BeginLogin(_ context.Context, client string) (string, []byte, error) { | ||
| 318 | options, data, err := s.verifier.BeginLogin() | 291 | options, data, err := s.verifier.BeginLogin() | ||
| 319 | if err != nil { | 292 | if err != nil { | ||
| 320 | return "", nil, err | 293 | return "", nil, err | ||
| 321 | } | 294 | } | ||
| 322 | - | id, err := s.begin(ceremonyLogin, clientKey(client), data) | 295 | + | id, err := s.begin(ceremony{kind: ceremonyLogin, data: data, client: client}) |
| 323 | if err != nil { | 296 | if err != nil { | ||
| 324 | return "", nil, err | 297 | return "", nil, err | ||
| 325 | } | 298 | } | ||
| @@ -333,7 +306,7 @@ func (s *Passkeys) BeginLogin(_ context.Context, client string) (string, []byte, | |||||
| 333 | // store is returned as it is, so that the caller fails closed. | 306 | // store is returned as it is, so that the caller fails closed. | ||
| 334 | func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response []byte) (token string, expiresAt time.Time, err error) { | 307 | func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response []byte) (token string, expiresAt time.Time, err error) { | ||
| 335 | log := zerowrap.FromCtx(ctx) | 308 | log := zerowrap.FromCtx(ctx) | ||
| 336 | - | data, ok := s.take(ceremonyID, ceremonyLogin) | 309 | + | c, ok := s.take(ceremonyID, ceremonyLogin) |
| 337 | if !ok { | 310 | if !ok { | ||
| 338 | log.Info().Msg("passkey sign-in refused: the ceremony expired or is unknown") | 311 | log.Info().Msg("passkey sign-in refused: the ceremony expired or is unknown") | ||
| 339 | return "", time.Time{}, domain.ErrUnauthenticated | 312 | return "", time.Time{}, domain.ErrUnauthenticated | ||
| @@ -342,19 +315,8 @@ func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response | |||||
| 342 | if err != nil { | 315 | if err != nil { | ||
| 343 | return "", time.Time{}, fmt.Errorf("load passkey user handle: %w", err) | 316 | return "", time.Time{}, fmt.Errorf("load passkey user handle: %w", err) | ||
| 344 | } | 317 | } | ||
| 345 | - | var looked []byte // the credential the response named, for the log | 318 | + | pk, err := s.verifier.FinishLogin(handle, c.data, response, func(id []byte) (domain.Passkey, error) { return s.store.Get(ctx, id) }) |
| 346 | - | pk, err := s.verifier.FinishLogin(handle, data, response, func(id []byte) (domain.Passkey, error) { | |||
| 347 | - | looked = bytes.Clone(id) | |||
| 348 | - | return s.store.Get(ctx, id) | |||
| 349 | - | }) | |||
| 350 | if err != nil { | 319 | if err != nil { | ||
| 351 | - | if errors.Is(err, domain.ErrPasskeyCloned) { | |||
| 352 | - | ev := log.Warn().Err(err) | |||
| 353 | - | if looked != nil { | |||
| 354 | - | ev = ev.Str("passkey", passkeyRef(looked)) | |||
| 355 | - | } | |||
| 356 | - | ev.Msg("passkey sign-in refused: the counter did not advance, the authenticator may be cloned") | |||
| 357 | - | } | |||
| 358 | if errors.Is(err, domain.ErrUnauthenticated) { | 320 | if errors.Is(err, domain.ErrUnauthenticated) { | ||
| 359 | // The reason is for the log (it holds no key); the caller learns nothing. | 321 | // The reason is for the log (it holds no key); the caller learns nothing. | ||
| 360 | log.Info().Err(err).Msg("passkey sign-in refused") | 322 | log.Info().Err(err).Msg("passkey sign-in refused") | ||
| @@ -376,9 +338,10 @@ func (s *Passkeys) FinishLogin(ctx context.Context, ceremonyID string, response | |||||
| 376 | } | 338 | } | ||
| 377 | return "", time.Time{}, fmt.Errorf("read passkey: %w", err) | 339 | return "", time.Time{}, fmt.Errorf("read passkey: %w", err) | ||
| 378 | } | 340 | } | ||
| 379 | - | // The verifier compared the counter with the one it was shown; another | 341 | + | // The one sign-count check. The verifier returns the counter the response |
| 380 | - | // login may have moved it since. A counter that does not advance is the | 342 | + | // reports (the stored one when it does not advance), and another login may |
| 381 | - | // clone signal (synced passkeys report 0 and are exempt). | 343 | + | // have moved the stored one since it looked. A counter that does not advance |
| 344 | + | // is the clone signal; synced passkeys report 0 and are exempt. | |||
| 382 | if cur.SignCount != 0 && pk.SignCount <= cur.SignCount { | 345 | if cur.SignCount != 0 && pk.SignCount <= cur.SignCount { | ||
| 383 | log.Warn().Str("passkey", passkeyRef(pk.ID)).Msg("passkey sign-in refused: the counter did not advance, the authenticator may be cloned") | 346 | log.Warn().Str("passkey", passkeyRef(pk.ID)).Msg("passkey sign-in refused: the counter did not advance, the authenticator may be cloned") | ||
| 384 | return "", time.Time{}, domain.ErrUnauthenticated | 347 | return "", time.Time{}, domain.ErrUnauthenticated | ||
M internal/usecase/passkeys_test.go +220 -158
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "context" | |
| 8 | 8 | "crypto/sha256" | |
| 9 | 9 | "encoding/hex" | |
| 10 | + | "encoding/json" | |
| 10 | 11 | "errors" | |
| 11 | 12 | "fmt" | |
| 12 | 13 | "slices" | |
| @@ -195,10 +196,10 @@ func TestPasskeysAreOwnerOnly(t *testing.T) { | |||
| 195 | 196 | // A ceremony begun by the owner cannot be finished by anyone else. | |
| 196 | 197 | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 197 | 198 | require.NoError(t, err) | |
| 198 | - | _, err = f.passkeys.FinishRegistration(ctx, tc.p, id, "n", []byte("{}")) | |
| 199 | + | err = f.passkeys.FinishRegistration(ctx, tc.p, id, []byte("{}")) | |
| 199 | 200 | check("finish registration", err) | |
| 200 | 201 | if tc.err != nil { | |
| 201 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "n", []byte("{}")) | |
| 202 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte("{}")) | |
| 202 | 203 | assert.NoError(t, err, "a refused caller does not burn the ceremony") | |
| 203 | 204 | } | |
| 204 | 205 | ||
| @@ -226,83 +227,113 @@ func TestRegistration(t *testing.T) { | |||
| 226 | 227 | f := newPasskeyFixture(t) | |
| 227 | 228 | f.acceptRegistration("cred-1") | |
| 228 | 229 | ||
| 229 | - | id, options, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 230 | + | id, options, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "Pixel 9") | |
| 230 | 231 | require.NoError(t, err) | |
| 231 | 232 | assert.JSONEq(t, `{"publicKey":{}}`, string(options)) | |
| 232 | 233 | ||
| 233 | - | got, err := f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "Pixel 9", []byte(`{}`)) | |
| 234 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`))) | |
| 235 | + | list, err := f.passkeys.List(ctx, domain.Owner()) | |
| 234 | 236 | require.NoError(t, err) | |
| 235 | - | assert.Equal(t, "Pixel 9", got.Name) | |
| 237 | + | require.Len(t, list, 1) | |
| 238 | + | got := list[0] | |
| 239 | + | assert.Equal(t, "Pixel 9", got.Name, "the name given at begin") | |
| 236 | 240 | assert.Equal(t, *f.at.Load(), got.CreatedAt) | |
| 237 | 241 | assert.True(t, got.LastUsedAt.IsZero()) | |
| 238 | 242 | assert.Equal(t, []byte("cred-1"), got.ID) | |
| 239 | - | list, err := f.passkeys.List(ctx, domain.Owner()) | |
| 240 | - | require.NoError(t, err) | |
| 241 | - | require.Len(t, list, 1) | |
| 242 | - | assert.Equal(t, got, list[0]) | |
| 243 | 243 | ||
| 244 | 244 | // The ceremony is single use. | |
| 245 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "again", []byte(`{}`)) | |
| 245 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) | |
| 246 | 246 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 247 | 247 | } | |
| 248 | 248 | ||
| 249 | - | func TestRegistrationDefaultNameAndBadName(t *testing.T) { | |
| 249 | + | // An empty name is the default one, and each ceremony keeps the name it began | |
| 250 | + | // with, whatever else is pending. | |
| 251 | + | func TestRegistrationNames(t *testing.T) { | |
| 250 | 252 | t.Parallel() | |
| 251 | 253 | ctx := context.Background() | |
| 252 | 254 | f := newPasskeyFixture(t) | |
| 253 | - | f.acceptRegistration("cred-1") | |
| 254 | - | ||
| 255 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 255 | + | // The credential id is the ceremony data, so that two ceremonies finish as | |
| 256 | + | // two different passkeys. | |
| 257 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything).RunAndReturn( | |
| 258 | + | func(_, ceremony, _ []byte) (domain.Passkey, error) { | |
| 259 | + | return domain.Passkey{ID: bytes.Clone(ceremony), PublicKey: []byte("pub")}, nil | |
| 260 | + | }) | |
| 261 | + | unnamed, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 256 | 262 | require.NoError(t, err) | |
| 257 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "evil \u202e name", []byte(`{}`)) | |
| 258 | - | require.ErrorIs(t, err, domain.ErrInvalidText) | |
| 259 | - | assert.Zero(t, f.count()) | |
| 260 | - | ||
| 261 | - | // The verifier was not asked, and the ceremony is still there for a retry. | |
| 262 | - | got, err := f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "", []byte(`{}`)) | |
| 263 | + | named, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "Second") | |
| 263 | 264 | require.NoError(t, err) | |
| 264 | - | assert.Equal(t, domain.DefaultPasskeyName, got.Name) | |
| 265 | - | } | |
| 265 | + | // Finished in the other order. | |
| 266 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), named, []byte(`{}`))) | |
| 267 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), unnamed, []byte(`{}`))) | |
| 266 | 268 | ||
| 267 | - | func TestRegistrationDuplicate(t *testing.T) { | |
| 268 | - | t.Parallel() | |
| 269 | - | ctx := context.Background() | |
| 270 | - | f := newPasskeyFixture(t) | |
| 271 | - | f.add("cred-1") | |
| 272 | - | f.acceptRegistration("cred-1") | |
| 273 | - | ||
| 274 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 275 | - | require.NoError(t, err) | |
| 276 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "dup", []byte(`{}`)) | |
| 277 | - | assert.ErrorIs(t, err, domain.ErrAlreadyExists) | |
| 278 | - | assert.Equal(t, 1, f.count()) | |
| 279 | - | } | |
| 280 | - | ||
| 281 | - | func TestRegistrationRefusedByVerifier(t *testing.T) { | |
| 282 | - | t.Parallel() | |
| 283 | - | ctx := context.Background() | |
| 284 | - | f := newPasskeyFixture(t) | |
| 285 | - | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 286 | - | Return(domain.Passkey{}, fmt.Errorf("%w: bad origin", domain.ErrUnauthenticated)) | |
| 287 | - | ||
| 288 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 269 | + | names := map[string]string{} | |
| 270 | + | list, err := f.passkeys.List(ctx, domain.Owner()) | |
| 289 | 271 | require.NoError(t, err) | |
| 290 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | |
| 291 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 292 | - | assert.Zero(t, f.count()) | |
| 272 | + | for _, p := range list { | |
| 273 | + | names[string(p.ID)] = p.Name | |
| 274 | + | } | |
| 275 | + | assert.ElementsMatch(t, []string{"Second", domain.DefaultPasskeyName}, []string{names["register-2"], names["register-1"]}) | |
| 276 | + | assert.Equal(t, "Second", names["register-2"]) | |
| 277 | + | assert.Equal(t, domain.DefaultPasskeyName, names["register-1"]) | |
| 293 | 278 | } | |
| 294 | 279 | ||
| 295 | - | func TestRegistrationRefusesAnIdTooLong(t *testing.T) { | |
| 280 | + | // What FinishRegistration refuses, and what is left in the store each time. | |
| 281 | + | func TestRegistrationRefusals(t *testing.T) { | |
| 296 | 282 | t.Parallel() | |
| 297 | - | ctx := context.Background() | |
| 298 | - | f := newPasskeyFixture(t) | |
| 299 | - | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 300 | - | Return(domain.Passkey{ID: make([]byte, domain.MaxPasskeyIDLen+1), PublicKey: []byte("k")}, nil) | |
| 301 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 302 | - | require.NoError(t, err) | |
| 303 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | |
| 304 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 305 | - | assert.Zero(t, f.count()) | |
| 283 | + | tests := []struct { | |
| 284 | + | name string | |
| 285 | + | existing []string // passkeys already stored | |
| 286 | + | verifier func(f *passkeyFixture) | |
| 287 | + | wantErr error | |
| 288 | + | wantKept int // passkeys in the store afterwards | |
| 289 | + | }{ | |
| 290 | + | { | |
| 291 | + | name: "duplicate credential", | |
| 292 | + | existing: []string{"cred-1"}, | |
| 293 | + | verifier: func(f *passkeyFixture) { f.acceptRegistration("cred-1") }, | |
| 294 | + | wantErr: domain.ErrAlreadyExists, wantKept: 1, | |
| 295 | + | }, | |
| 296 | + | { | |
| 297 | + | name: "refused by the verifier", | |
| 298 | + | verifier: func(f *passkeyFixture) { | |
| 299 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 300 | + | Return(domain.Passkey{}, fmt.Errorf("%w: bad origin", domain.ErrUnauthenticated)) | |
| 301 | + | }, | |
| 302 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |
| 303 | + | }, | |
| 304 | + | { | |
| 305 | + | name: "credential id too long", | |
| 306 | + | verifier: func(f *passkeyFixture) { | |
| 307 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 308 | + | Return(domain.Passkey{ID: make([]byte, domain.MaxPasskeyIDLen+1), PublicKey: []byte("k")}, nil) | |
| 309 | + | }, | |
| 310 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |
| 311 | + | }, | |
| 312 | + | { | |
| 313 | + | name: "empty credential id", | |
| 314 | + | verifier: func(f *passkeyFixture) { | |
| 315 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 316 | + | Return(domain.Passkey{PublicKey: []byte("k")}, nil) | |
| 317 | + | }, | |
| 318 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |
| 319 | + | }, | |
| 320 | + | } | |
| 321 | + | for _, tc := range tests { | |
| 322 | + | t.Run(tc.name, func(t *testing.T) { | |
| 323 | + | t.Parallel() | |
| 324 | + | ctx := context.Background() | |
| 325 | + | f := newPasskeyFixture(t) | |
| 326 | + | for _, id := range tc.existing { | |
| 327 | + | f.add(id) | |
| 328 | + | } | |
| 329 | + | tc.verifier(f) | |
| 330 | + | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "x") | |
| 331 | + | require.NoError(t, err) | |
| 332 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) | |
| 333 | + | assert.ErrorIs(t, err, tc.wantErr) | |
| 334 | + | assert.Equal(t, tc.wantKept, f.count()) | |
| 335 | + | }) | |
| 336 | + | } | |
| 306 | 337 | } | |
| 307 | 338 | ||
| 308 | 339 | // The verifier is told about the passkeys to exclude. | |
| @@ -336,7 +367,7 @@ func TestMaxPasskeys(t *testing.T) { | |||
| 336 | 367 | // Full: no new ceremony, and the one in flight is refused at the end. | |
| 337 | 368 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 338 | 369 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | |
| 339 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | |
| 370 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) | |
| 340 | 371 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | |
| 341 | 372 | assert.Equal(t, domain.MaxPasskeys, f.count()) | |
| 342 | 373 | ||
| @@ -359,8 +390,8 @@ func TestCeremonyExpiry(t *testing.T) { | |||
| 359 | 390 | require.NoError(t, err) | |
| 360 | 391 | login, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 361 | 392 | require.NoError(t, err) | |
| 362 | - | f.advance(usecase.PasskeyCeremonyTTL - time.Second) | |
| 363 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, "x", []byte(`{}`)) | |
| 393 | + | f.advance(domain.PasskeyCeremonyTTL - time.Second) | |
| 394 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, []byte(`{}`)) | |
| 364 | 395 | assert.NoError(t, err) | |
| 365 | 396 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | |
| 366 | 397 | assert.NoError(t, err) | |
| @@ -370,9 +401,8 @@ func TestCeremonyExpiry(t *testing.T) { | |||
| 370 | 401 | require.NoError(t, err) | |
| 371 | 402 | login, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 372 | 403 | require.NoError(t, err) | |
| 373 | - | assert.Equal(t, 2*time.Minute, usecase.PasskeyCeremonyTTL) | |
| 374 | - | f.advance(usecase.PasskeyCeremonyTTL) | |
| 375 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, "x", []byte(`{}`)) | |
| 404 | + | f.advance(domain.PasskeyCeremonyTTL) | |
| 405 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, []byte(`{}`)) | |
| 376 | 406 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 377 | 407 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | |
| 378 | 408 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| @@ -389,7 +419,7 @@ func TestExpiredRegistrationsDoNotCount(t *testing.T) { | |||
| 389 | 419 | } | |
| 390 | 420 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 391 | 421 | require.ErrorIs(t, err, domain.ErrResourceExhausted) | |
| 392 | - | f.advance(usecase.PasskeyCeremonyTTL) | |
| 422 | + | f.advance(domain.PasskeyCeremonyTTL) | |
| 393 | 423 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 394 | 424 | assert.NoError(t, err) | |
| 395 | 425 | } | |
| @@ -413,7 +443,7 @@ func TestPendingRegistrationsAreBounded(t *testing.T) { | |||
| 413 | 443 | // registration frees one. | |
| 414 | 444 | _, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 415 | 445 | require.NoError(t, err) | |
| 416 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), ids[0], "x", []byte(`{}`)) | |
| 446 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), ids[0], []byte(`{}`)) | |
| 417 | 447 | require.NoError(t, err) | |
| 418 | 448 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 419 | 449 | assert.NoError(t, err) | |
| @@ -506,36 +536,6 @@ func TestPendingLoginsGlobalOverflow(t *testing.T) { | |||
| 506 | 536 | assert.True(t, f.finishes(fresh), "the new login works") | |
| 507 | 537 | } | |
| 508 | 538 | ||
| 509 | - | // IPv6 clients share a cap per /64; an IPv4-mapped address is the IPv4 one. | |
| 510 | - | func TestPendingLoginsClientKey(t *testing.T) { | |
| 511 | - | t.Parallel() | |
| 512 | - | f := newPasskeyFixture(t) | |
| 513 | - | f.add("cred-1") | |
| 514 | - | f.acceptLogin("cred-1", 1) | |
| 515 | - | perClient := usecase.MaxPendingPasskeyLoginsPerClient | |
| 516 | - | ||
| 517 | - | // Different addresses of one /64 count together. | |
| 518 | - | ids := f.loginIDs(t, "2001:db8:1:2::1", perClient) | |
| 519 | - | next := f.loginIDs(t, "2001:db8:1:2:ffff:ffff:ffff:ffff", 1)[0] | |
| 520 | - | assert.False(t, f.finishes(ids[0]), "the same /64: the 9th evicted the oldest") | |
| 521 | - | assert.True(t, f.finishes(next)) | |
| 522 | - | ||
| 523 | - | // Another /64 has its own cap. | |
| 524 | - | other := f.loginIDs(t, "2001:db8:1:3::1", perClient) | |
| 525 | - | for _, id := range other { | |
| 526 | - | assert.True(t, f.finishes(id), "a different /64 is another client") | |
| 527 | - | } | |
| 528 | - | ||
| 529 | - | // ::ffff:192.0.2.1 and 192.0.2.1 are one client. | |
| 530 | - | v4 := f.loginIDs(t, "192.0.2.1", perClient) | |
| 531 | - | f.loginIDs(t, "::ffff:192.0.2.1", 1) | |
| 532 | - | assert.False(t, f.finishes(v4[0])) | |
| 533 | - | // Two IPv4 addresses of one /24 are two clients. | |
| 534 | - | a := f.loginIDs(t, "192.0.2.50", perClient) | |
| 535 | - | f.loginIDs(t, "192.0.2.51", perClient) | |
| 536 | - | assert.True(t, f.finishes(a[0])) | |
| 537 | - | } | |
| 538 | - | ||
| 539 | 539 | // Logins never push out a registration, and registrations never push out a login. | |
| 540 | 540 | func TestCeremonyKindsAreSeparate(t *testing.T) { | |
| 541 | 541 | t.Parallel() | |
| @@ -554,7 +554,7 @@ func TestCeremonyKindsAreSeparate(t *testing.T) { | |||
| 554 | 554 | // attempt burns it. | |
| 555 | 555 | _, _, err = f.passkeys.FinishLogin(ctx, reg, []byte(`{}`)) | |
| 556 | 556 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 557 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), login, "x", []byte(`{}`)) | |
| 557 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), login, []byte(`{}`)) | |
| 558 | 558 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 559 | 559 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | |
| 560 | 560 | assert.ErrorIs(t, err, domain.ErrUnauthenticated, "burned by the mismatch") | |
| @@ -569,7 +569,7 @@ func TestUnknownCeremony(t *testing.T) { | |||
| 569 | 569 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 570 | 570 | _, _, err = f.passkeys.FinishLogin(ctx, "nope", []byte(`{}`)) | |
| 571 | 571 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 572 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "nope", "x", []byte(`{}`)) | |
| 572 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "nope", []byte(`{}`)) | |
| 573 | 573 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 574 | 574 | } | |
| 575 | 575 | ||
| @@ -630,8 +630,23 @@ func (l *syncLog) String() string { | |||
| 630 | 630 | return l.b.String() | |
| 631 | 631 | } | |
| 632 | 632 | ||
| 633 | - | // GO-001: the reason of a refusal is logged (info), never answered, and holds | |
| 634 | - | // no key material. | |
| 633 | + | // logEntries parses the JSON lines of the buffer. | |
| 634 | + | func logEntries(t *testing.T, logs *syncLog) []map[string]any { | |
| 635 | + | t.Helper() | |
| 636 | + | var out []map[string]any | |
| 637 | + | for _, line := range strings.Split(strings.TrimSpace(logs.String()), "\n") { | |
| 638 | + | if line == "" { | |
| 639 | + | continue | |
| 640 | + | } | |
| 641 | + | var m map[string]any | |
| 642 | + | require.NoError(t, json.Unmarshal([]byte(line), &m), line) | |
| 643 | + | out = append(out, m) | |
| 644 | + | } | |
| 645 | + | return out | |
| 646 | + | } | |
| 647 | + | ||
| 648 | + | // The reason of a refusal is logged at info, never answered, and holds no | |
| 649 | + | // secret: neither the response bytes nor a key. | |
| 635 | 650 | func TestRefusalsAreLogged(t *testing.T) { | |
| 636 | 651 | t.Parallel() | |
| 637 | 652 | ||
| @@ -646,20 +661,24 @@ func TestRefusalsAreLogged(t *testing.T) { | |||
| 646 | 661 | require.NoError(t, err) | |
| 647 | 662 | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{"secret":"response-bytes"}`)) | |
| 648 | 663 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 649 | - | out := logs.String() | |
| 650 | - | assert.Contains(t, out, `"message":"passkey sign-in refused"`) | |
| 651 | - | assert.Contains(t, out, `"level":"info"`) | |
| 652 | - | assert.Contains(t, out, "origin mismatch") | |
| 653 | - | assert.NotContains(t, out, "response-bytes") | |
| 664 | + | assert.NotContains(t, err.Error(), "origin mismatch", "the caller learns nothing") | |
| 665 | + | entries := logEntries(t, logs) | |
| 666 | + | require.Len(t, entries, 1) | |
| 667 | + | assert.Equal(t, "info", entries[0]["level"]) | |
| 668 | + | assert.Contains(t, entries[0]["error"], "origin mismatch") | |
| 669 | + | assert.NotContains(t, logs.String(), "response-bytes") | |
| 654 | 670 | }) | |
| 655 | 671 | ||
| 656 | 672 | t.Run("sign-in with an unknown ceremony", func(t *testing.T) { | |
| 657 | 673 | t.Parallel() | |
| 658 | 674 | ctx, logs := logCtx(t) | |
| 659 | 675 | f := newPasskeyFixture(t) | |
| 660 | - | _, _, err := f.passkeys.FinishLogin(ctx, "nope", []byte(`{}`)) | |
| 676 | + | _, _, err := f.passkeys.FinishLogin(ctx, "nope", []byte(`{"secret":"response-bytes"}`)) | |
| 661 | 677 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 662 | - | assert.Contains(t, logs.String(), "passkey sign-in refused") | |
| 678 | + | entries := logEntries(t, logs) | |
| 679 | + | require.Len(t, entries, 1) | |
| 680 | + | assert.Equal(t, "info", entries[0]["level"]) | |
| 681 | + | assert.NotContains(t, logs.String(), "response-bytes") | |
| 663 | 682 | }) | |
| 664 | 683 | ||
| 665 | 684 | t.Run("registration", func(t *testing.T) { | |
| @@ -668,48 +687,62 @@ func TestRefusalsAreLogged(t *testing.T) { | |||
| 668 | 687 | f := newPasskeyFixture(t) | |
| 669 | 688 | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |
| 670 | 689 | Return(domain.Passkey{}, fmt.Errorf("%w: passkey registration: user verification missing", domain.ErrUnauthenticated)) | |
| 671 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 690 | + | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "x") | |
| 672 | 691 | require.NoError(t, err) | |
| 673 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{"secret":"response-bytes"}`)) | |
| 692 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{"secret":"response-bytes"}`)) | |
| 674 | 693 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 675 | - | out := logs.String() | |
| 676 | - | assert.Contains(t, out, `"message":"passkey registration refused"`) | |
| 677 | - | assert.Contains(t, out, "user verification missing") | |
| 678 | - | assert.NotContains(t, out, "response-bytes") | |
| 679 | - | ||
| 680 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "unknown", "x", []byte(`{}`)) | |
| 694 | + | entries := logEntries(t, logs) | |
| 695 | + | require.Len(t, entries, 1) | |
| 696 | + | assert.Equal(t, "info", entries[0]["level"]) | |
| 697 | + | assert.Contains(t, entries[0]["error"], "user verification missing") | |
| 698 | + | assert.NotContains(t, logs.String(), "response-bytes") | |
| 699 | + | ||
| 700 | + | // An unknown ceremony is logged too. | |
| 701 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "unknown", []byte(`{}`)) | |
| 681 | 702 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 682 | - | assert.Equal(t, 2, strings.Count(logs.String(), "passkey registration refused")) | |
| 703 | + | entries = logEntries(t, logs) | |
| 704 | + | require.Len(t, entries, 2) | |
| 705 | + | assert.Equal(t, "info", entries[1]["level"]) | |
| 683 | 706 | }) | |
| 684 | 707 | } | |
| 685 | 708 | ||
| 686 | - | // GO-002: the clone warning says which passkey, by its short reference. | |
| 687 | - | func TestCloneWarningNamesThePasskey(t *testing.T) { | |
| 709 | + | // A counter that does not advance on a passkey that has one is refused, and the | |
| 710 | + | // warning names the passkey by its short reference. The library hands back the | |
| 711 | + | // stored counter when the received one is not higher, which is what the verifier | |
| 712 | + | // returns here. | |
| 713 | + | func TestCloneIsRefusedAndNamed(t *testing.T) { | |
| 688 | 714 | t.Parallel() | |
| 689 | 715 | ctx, logs := logCtx(t) | |
| 690 | 716 | f := newPasskeyFixture(t) | |
| 691 | - | f.add("cred-1") | |
| 717 | + | p := f.add("cred-1") | |
| 718 | + | p.SignCount = 5 | |
| 719 | + | f.mu.Lock() | |
| 720 | + | f.stored["cred-1"] = p | |
| 721 | + | f.mu.Unlock() | |
| 692 | 722 | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | |
| 693 | 723 | func(_, _, _ []byte, lookup func([]byte) (domain.Passkey, error)) (domain.Passkey, error) { | |
| 694 | - | _, err := lookup([]byte("cred-1")) | |
| 695 | - | require.NoError(t, err) | |
| 696 | - | return domain.Passkey{}, domain.ErrPasskeyCloned | |
| 724 | + | return lookup([]byte("cred-1")) // SignCount stays 5 | |
| 697 | 725 | }) | |
| 698 | 726 | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 699 | 727 | require.NoError(t, err) | |
| 700 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |
| 728 | + | token, _, err := f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |
| 701 | 729 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 730 | + | assert.Empty(t, token, "no session") | |
| 731 | + | assert.Equal(t, 1, f.count()) | |
| 732 | + | f.mu.Lock() | |
| 733 | + | assert.True(t, f.stored["cred-1"].LastUsedAt.IsZero(), "nothing was recorded") | |
| 734 | + | f.mu.Unlock() | |
| 702 | 735 | ||
| 703 | 736 | sum := sha256.Sum256([]byte("cred-1")) | |
| 704 | 737 | ref := hex.EncodeToString(sum[:])[:8] | |
| 705 | - | out := logs.String() | |
| 706 | - | assert.Contains(t, out, `"level":"warn"`) | |
| 707 | - | assert.Contains(t, out, `"passkey":"`+ref+`"`) | |
| 708 | - | assert.Contains(t, out, "may be cloned") | |
| 709 | - | assert.NotContains(t, out, "cred-1", "the credential id itself is never logged") | |
| 738 | + | entries := logEntries(t, logs) | |
| 739 | + | require.Len(t, entries, 1, "one log line for the refusal") | |
| 740 | + | assert.Equal(t, "warn", entries[0]["level"]) | |
| 741 | + | assert.Equal(t, ref, entries[0]["passkey"]) | |
| 742 | + | assert.NotContains(t, logs.String(), "cred-1", "the credential id itself is never logged") | |
| 710 | 743 | } | |
| 711 | 744 | ||
| 712 | - | // GO-003: two logins verified against the same stored counter cannot both | |
| 745 | + | // Two logins verified against the same stored counter cannot both | |
| 713 | 746 | // succeed. The verifier saw counter 5; by the time the second one writes, the | |
| 714 | 747 | // first has stored 6. | |
| 715 | 748 | func TestLoginRefusesACounterThatDidNotAdvance(t *testing.T) { | |
| @@ -773,21 +806,6 @@ func TestLoginRefusesACounterThatDidNotAdvance(t *testing.T) { | |||
| 773 | 806 | } | |
| 774 | 807 | }) | |
| 775 | 808 | ||
| 776 | - | t.Run("equal counters are refused when the stored one is not 0", func(t *testing.T) { | |
| 777 | - | t.Parallel() | |
| 778 | - | f := newPasskeyFixture(t) | |
| 779 | - | p := f.add("cred-1") | |
| 780 | - | p.SignCount = 5 | |
| 781 | - | f.mu.Lock() | |
| 782 | - | f.stored["cred-1"] = p | |
| 783 | - | f.mu.Unlock() | |
| 784 | - | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything). | |
| 785 | - | Return(domain.Passkey{ID: []byte("cred-1"), SignCount: 5}, nil) | |
| 786 | - | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 787 | - | require.NoError(t, err) | |
| 788 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |
| 789 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 790 | - | }) | |
| 791 | 809 | } | |
| 792 | 810 | ||
| 793 | 811 | func TestLoginRefusals(t *testing.T) { | |
| @@ -817,21 +835,6 @@ func TestLoginRefusals(t *testing.T) { | |||
| 817 | 835 | assert.NotContains(t, err.Error(), "bad signature", "the reason is for logs, not for the caller") | |
| 818 | 836 | }) | |
| 819 | 837 | ||
| 820 | - | t.Run("cloned authenticator", func(t *testing.T) { | |
| 821 | - | t.Parallel() | |
| 822 | - | f := newPasskeyFixture(t) | |
| 823 | - | p := f.add("cred-1") | |
| 824 | - | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything). | |
| 825 | - | Return(domain.Passkey{}, domain.ErrPasskeyCloned) | |
| 826 | - | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |
| 827 | - | require.NoError(t, err) | |
| 828 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |
| 829 | - | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |
| 830 | - | got, err := f.passkeys.List(ctx, domain.Owner()) | |
| 831 | - | require.NoError(t, err) | |
| 832 | - | assert.Equal(t, []domain.Passkey{p}, got, "nothing recorded") | |
| 833 | - | }) | |
| 834 | - | ||
| 835 | 838 | t.Run("store failure fails closed and is not an authentication error", func(t *testing.T) { | |
| 836 | 839 | t.Parallel() | |
| 837 | 840 | f := newPasskeyFixture(t) | |
| @@ -1000,3 +1003,62 @@ func TestBeginRegistrationChecksTheName(t *testing.T) { | |||
| 1000 | 1003 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |
| 1001 | 1004 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | |
| 1002 | 1005 | } | |
| 1006 | + | ||
| 1007 | + | // A client at its cap begins a login while the store is full. Its own eviction | |
| 1008 | + | // makes the room the new login needs, so exactly one ceremony is dropped, the | |
| 1009 | + | // client's own oldest: the global rule must not take a second one, from anyone. | |
| 1010 | + | func TestFullClientOnAFullStoreLosesOnlyItsOwnOldest(t *testing.T) { | |
| 1011 | + | t.Parallel() | |
| 1012 | + | f := newPasskeyFixture(t) | |
| 1013 | + | f.add("cred-1") | |
| 1014 | + | f.acceptLogin("cred-1", 1) | |
| 1015 | + | perClient := usecase.MaxPendingPasskeyLoginsPerClient | |
| 1016 | + | ||
| 1017 | + | // Seven clients with one login each, the flooder with a full cap, then full | |
| 1018 | + | // clients and a last partial one until the store holds exactly its cap. | |
| 1019 | + | small := make([]string, 0, 7) | |
| 1020 | + | for i := range 7 { | |
| 1021 | + | small = append(small, f.loginIDs(t, fmt.Sprintf("10.9.0.%d", i), 1)[0]) | |
| 1022 | + | } | |
| 1023 | + | flooder := f.loginIDs(t, "192.0.2.1", perClient) | |
| 1024 | + | total := len(small) + len(flooder) | |
| 1025 | + | var others []string | |
| 1026 | + | for n := 0; total+perClient <= usecase.MaxPendingPasskeyLogins; n++ { | |
| 1027 | + | others = append(others, f.loginIDs(t, fmt.Sprintf("10.0.%d.1", n), perClient)...) | |
| 1028 | + | total += perClient | |
| 1029 | + | } | |
| 1030 | + | if rest := usecase.MaxPendingPasskeyLogins - total; rest > 0 { | |
| 1031 | + | others = append(others, f.loginIDs(t, "10.8.0.1", rest)...) | |
| 1032 | + | total += rest | |
| 1033 | + | } | |
| 1034 | + | require.Equal(t, usecase.MaxPendingPasskeyLogins, total, "the store is full") | |
| 1035 | + | ||
| 1036 | + | newest := f.loginIDs(t, "192.0.2.1", 1)[0] | |
| 1037 | + | ||
| 1038 | + | assert.False(t, f.finishes(flooder[0]), "the flooder lost its own oldest") | |
| 1039 | + | for _, id := range append(append(flooder[1:], small...), others...) { | |
| 1040 | + | assert.True(t, f.finishes(id), "nobody else lost a ceremony") | |
| 1041 | + | } | |
| 1042 | + | assert.True(t, f.finishes(newest), "the new login works") | |
| 1043 | + | } | |
| 1044 | + | ||
| 1045 | + | // A failure of the store while removing a passkey is returned as it is, and | |
| 1046 | + | // nobody is signed out for it. | |
| 1047 | + | func TestRemoveReturnsAStoreError(t *testing.T) { | |
| 1048 | + | t.Parallel() | |
| 1049 | + | ctx := context.Background() | |
| 1050 | + | f := newPasskeyFixture(t) | |
| 1051 | + | boom := errors.New("disk on fire") | |
| 1052 | + | store := mocks.NewPasskeyStore(t) | |
| 1053 | + | store.EXPECT().Delete(mock.Anything, []byte("cred-1")).Return(boom) | |
| 1054 | + | // The session store is a strict mock: a BumpEpoch would fail the test. | |
| 1055 | + | pk := usecase.NewPasskeys(store, mocks.NewPasskeyVerifier(t), f.sessions, f.clock) | |
| 1056 | + | tok, _, err := f.sessions.Login(ctx, f.nonce(t)) | |
| 1057 | + | require.NoError(t, err) | |
| 1058 | + | ||
| 1059 | + | err = pk.Remove(ctx, domain.Owner(), []byte("cred-1")) | |
| 1060 | + | require.ErrorIs(t, err, boom) | |
| 1061 | + | assert.NotErrorIs(t, err, domain.ErrNotFound) | |
| 1062 | + | _, err = f.sessions.Verify(ctx, tok) | |
| 1063 | + | assert.NoError(t, err, "the epoch did not move: the sessions are still valid") | |
| 1064 | + | } | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "context" | 7 | "context" | ||
| 8 | "crypto/sha256" | 8 | "crypto/sha256" | ||
| 9 | "encoding/hex" | 9 | "encoding/hex" | ||
| 10 | + | "encoding/json" | |||
| 10 | "errors" | 11 | "errors" | ||
| 11 | "fmt" | 12 | "fmt" | ||
| 12 | "slices" | 13 | "slices" | ||
| @@ -195,10 +196,10 @@ func TestPasskeysAreOwnerOnly(t *testing.T) { | |||||
| 195 | // A ceremony begun by the owner cannot be finished by anyone else. | 196 | // A ceremony begun by the owner cannot be finished by anyone else. | ||
| 196 | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 197 | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 197 | require.NoError(t, err) | 198 | require.NoError(t, err) | ||
| 198 | - | _, err = f.passkeys.FinishRegistration(ctx, tc.p, id, "n", []byte("{}")) | 199 | + | err = f.passkeys.FinishRegistration(ctx, tc.p, id, []byte("{}")) |
| 199 | check("finish registration", err) | 200 | check("finish registration", err) | ||
| 200 | if tc.err != nil { | 201 | if tc.err != nil { | ||
| 201 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "n", []byte("{}")) | 202 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte("{}")) |
| 202 | assert.NoError(t, err, "a refused caller does not burn the ceremony") | 203 | assert.NoError(t, err, "a refused caller does not burn the ceremony") | ||
| 203 | } | 204 | } | ||
| 204 | 205 | ||||
| @@ -226,83 +227,113 @@ func TestRegistration(t *testing.T) { | |||||
| 226 | f := newPasskeyFixture(t) | 227 | f := newPasskeyFixture(t) | ||
| 227 | f.acceptRegistration("cred-1") | 228 | f.acceptRegistration("cred-1") | ||
| 228 | 229 | ||||
| 229 | - | id, options, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 230 | + | id, options, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "Pixel 9") |
| 230 | require.NoError(t, err) | 231 | require.NoError(t, err) | ||
| 231 | assert.JSONEq(t, `{"publicKey":{}}`, string(options)) | 232 | assert.JSONEq(t, `{"publicKey":{}}`, string(options)) | ||
| 232 | 233 | ||||
| 233 | - | got, err := f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "Pixel 9", []byte(`{}`)) | 234 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`))) |
| 235 | + | list, err := f.passkeys.List(ctx, domain.Owner()) | |||
| 234 | require.NoError(t, err) | 236 | require.NoError(t, err) | ||
| 235 | - | assert.Equal(t, "Pixel 9", got.Name) | 237 | + | require.Len(t, list, 1) |
| 238 | + | got := list[0] | |||
| 239 | + | assert.Equal(t, "Pixel 9", got.Name, "the name given at begin") | |||
| 236 | assert.Equal(t, *f.at.Load(), got.CreatedAt) | 240 | assert.Equal(t, *f.at.Load(), got.CreatedAt) | ||
| 237 | assert.True(t, got.LastUsedAt.IsZero()) | 241 | assert.True(t, got.LastUsedAt.IsZero()) | ||
| 238 | assert.Equal(t, []byte("cred-1"), got.ID) | 242 | assert.Equal(t, []byte("cred-1"), got.ID) | ||
| 239 | - | list, err := f.passkeys.List(ctx, domain.Owner()) | |||
| 240 | - | require.NoError(t, err) | |||
| 241 | - | require.Len(t, list, 1) | |||
| 242 | - | assert.Equal(t, got, list[0]) | |||
| 243 | 243 | ||||
| 244 | // The ceremony is single use. | 244 | // The ceremony is single use. | ||
| 245 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "again", []byte(`{}`)) | 245 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) |
| 246 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 246 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 247 | } | 247 | } | ||
| 248 | 248 | ||||
| 249 | - | func TestRegistrationDefaultNameAndBadName(t *testing.T) { | 249 | + | // An empty name is the default one, and each ceremony keeps the name it began |
| 250 | + | // with, whatever else is pending. | |||
| 251 | + | func TestRegistrationNames(t *testing.T) { | |||
| 250 | t.Parallel() | 252 | t.Parallel() | ||
| 251 | ctx := context.Background() | 253 | ctx := context.Background() | ||
| 252 | f := newPasskeyFixture(t) | 254 | f := newPasskeyFixture(t) | ||
| 253 | - | f.acceptRegistration("cred-1") | 255 | + | // The credential id is the ceremony data, so that two ceremonies finish as |
| 254 | - | 256 | + | // two different passkeys. | |
| 255 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 257 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything).RunAndReturn( |
| 258 | + | func(_, ceremony, _ []byte) (domain.Passkey, error) { | |||
| 259 | + | return domain.Passkey{ID: bytes.Clone(ceremony), PublicKey: []byte("pub")}, nil | |||
| 260 | + | }) | |||
| 261 | + | unnamed, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |||
| 256 | require.NoError(t, err) | 262 | require.NoError(t, err) | ||
| 257 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "evil \u202e name", []byte(`{}`)) | 263 | + | named, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "Second") |
| 258 | - | require.ErrorIs(t, err, domain.ErrInvalidText) | |||
| 259 | - | assert.Zero(t, f.count()) | |||
| 260 | - | ||||
| 261 | - | // The verifier was not asked, and the ceremony is still there for a retry. | |||
| 262 | - | got, err := f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "", []byte(`{}`)) | |||
| 263 | require.NoError(t, err) | 264 | require.NoError(t, err) | ||
| 264 | - | assert.Equal(t, domain.DefaultPasskeyName, got.Name) | 265 | + | // Finished in the other order. |
| 265 | - | } | 266 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), named, []byte(`{}`))) |
| 267 | + | require.NoError(t, f.passkeys.FinishRegistration(ctx, domain.Owner(), unnamed, []byte(`{}`))) | |||
| 266 | 268 | ||||
| 267 | - | func TestRegistrationDuplicate(t *testing.T) { | 269 | + | names := map[string]string{} |
| 268 | - | t.Parallel() | 270 | + | list, err := f.passkeys.List(ctx, domain.Owner()) |
| 269 | - | ctx := context.Background() | |||
| 270 | - | f := newPasskeyFixture(t) | |||
| 271 | - | f.add("cred-1") | |||
| 272 | - | f.acceptRegistration("cred-1") | |||
| 273 | - | ||||
| 274 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |||
| 275 | - | require.NoError(t, err) | |||
| 276 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "dup", []byte(`{}`)) | |||
| 277 | - | assert.ErrorIs(t, err, domain.ErrAlreadyExists) | |||
| 278 | - | assert.Equal(t, 1, f.count()) | |||
| 279 | - | } | |||
| 280 | - | ||||
| 281 | - | func TestRegistrationRefusedByVerifier(t *testing.T) { | |||
| 282 | - | t.Parallel() | |||
| 283 | - | ctx := context.Background() | |||
| 284 | - | f := newPasskeyFixture(t) | |||
| 285 | - | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |||
| 286 | - | Return(domain.Passkey{}, fmt.Errorf("%w: bad origin", domain.ErrUnauthenticated)) | |||
| 287 | - | ||||
| 288 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | |||
| 289 | require.NoError(t, err) | 271 | require.NoError(t, err) | ||
| 290 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | 272 | + | for _, p := range list { |
| 291 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 273 | + | names[string(p.ID)] = p.Name |
| 292 | - | assert.Zero(t, f.count()) | 274 | + | } |
| 275 | + | assert.ElementsMatch(t, []string{"Second", domain.DefaultPasskeyName}, []string{names["register-2"], names["register-1"]}) | |||
| 276 | + | assert.Equal(t, "Second", names["register-2"]) | |||
| 277 | + | assert.Equal(t, domain.DefaultPasskeyName, names["register-1"]) | |||
| 293 | } | 278 | } | ||
| 294 | 279 | ||||
| 295 | - | func TestRegistrationRefusesAnIdTooLong(t *testing.T) { | 280 | + | // What FinishRegistration refuses, and what is left in the store each time. |
| 281 | + | func TestRegistrationRefusals(t *testing.T) { | |||
| 296 | t.Parallel() | 282 | t.Parallel() | ||
| 297 | - | ctx := context.Background() | 283 | + | tests := []struct { |
| 298 | - | f := newPasskeyFixture(t) | 284 | + | name string |
| 299 | - | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | 285 | + | existing []string // passkeys already stored |
| 300 | - | Return(domain.Passkey{ID: make([]byte, domain.MaxPasskeyIDLen+1), PublicKey: []byte("k")}, nil) | 286 | + | verifier func(f *passkeyFixture) |
| 301 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 287 | + | wantErr error |
| 302 | - | require.NoError(t, err) | 288 | + | wantKept int // passkeys in the store afterwards |
| 303 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | 289 | + | }{ |
| 304 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 290 | + | { |
| 305 | - | assert.Zero(t, f.count()) | 291 | + | name: "duplicate credential", |
| 292 | + | existing: []string{"cred-1"}, | |||
| 293 | + | verifier: func(f *passkeyFixture) { f.acceptRegistration("cred-1") }, | |||
| 294 | + | wantErr: domain.ErrAlreadyExists, wantKept: 1, | |||
| 295 | + | }, | |||
| 296 | + | { | |||
| 297 | + | name: "refused by the verifier", | |||
| 298 | + | verifier: func(f *passkeyFixture) { | |||
| 299 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |||
| 300 | + | Return(domain.Passkey{}, fmt.Errorf("%w: bad origin", domain.ErrUnauthenticated)) | |||
| 301 | + | }, | |||
| 302 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |||
| 303 | + | }, | |||
| 304 | + | { | |||
| 305 | + | name: "credential id too long", | |||
| 306 | + | verifier: func(f *passkeyFixture) { | |||
| 307 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |||
| 308 | + | Return(domain.Passkey{ID: make([]byte, domain.MaxPasskeyIDLen+1), PublicKey: []byte("k")}, nil) | |||
| 309 | + | }, | |||
| 310 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |||
| 311 | + | }, | |||
| 312 | + | { | |||
| 313 | + | name: "empty credential id", | |||
| 314 | + | verifier: func(f *passkeyFixture) { | |||
| 315 | + | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | |||
| 316 | + | Return(domain.Passkey{PublicKey: []byte("k")}, nil) | |||
| 317 | + | }, | |||
| 318 | + | wantErr: domain.ErrUnauthenticated, wantKept: 0, | |||
| 319 | + | }, | |||
| 320 | + | } | |||
| 321 | + | for _, tc := range tests { | |||
| 322 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 323 | + | t.Parallel() | |||
| 324 | + | ctx := context.Background() | |||
| 325 | + | f := newPasskeyFixture(t) | |||
| 326 | + | for _, id := range tc.existing { | |||
| 327 | + | f.add(id) | |||
| 328 | + | } | |||
| 329 | + | tc.verifier(f) | |||
| 330 | + | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "x") | |||
| 331 | + | require.NoError(t, err) | |||
| 332 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) | |||
| 333 | + | assert.ErrorIs(t, err, tc.wantErr) | |||
| 334 | + | assert.Equal(t, tc.wantKept, f.count()) | |||
| 335 | + | }) | |||
| 336 | + | } | |||
| 306 | } | 337 | } | ||
| 307 | 338 | ||||
| 308 | // The verifier is told about the passkeys to exclude. | 339 | // The verifier is told about the passkeys to exclude. | ||
| @@ -336,7 +367,7 @@ func TestMaxPasskeys(t *testing.T) { | |||||
| 336 | // Full: no new ceremony, and the one in flight is refused at the end. | 367 | // Full: no new ceremony, and the one in flight is refused at the end. | ||
| 337 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 368 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 338 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | 369 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | ||
| 339 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{}`)) | 370 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{}`)) |
| 340 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | 371 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | ||
| 341 | assert.Equal(t, domain.MaxPasskeys, f.count()) | 372 | assert.Equal(t, domain.MaxPasskeys, f.count()) | ||
| 342 | 373 | ||||
| @@ -359,8 +390,8 @@ func TestCeremonyExpiry(t *testing.T) { | |||||
| 359 | require.NoError(t, err) | 390 | require.NoError(t, err) | ||
| 360 | login, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | 391 | login, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | ||
| 361 | require.NoError(t, err) | 392 | require.NoError(t, err) | ||
| 362 | - | f.advance(usecase.PasskeyCeremonyTTL - time.Second) | 393 | + | f.advance(domain.PasskeyCeremonyTTL - time.Second) |
| 363 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, "x", []byte(`{}`)) | 394 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, []byte(`{}`)) |
| 364 | assert.NoError(t, err) | 395 | assert.NoError(t, err) | ||
| 365 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | 396 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | ||
| 366 | assert.NoError(t, err) | 397 | assert.NoError(t, err) | ||
| @@ -370,9 +401,8 @@ func TestCeremonyExpiry(t *testing.T) { | |||||
| 370 | require.NoError(t, err) | 401 | require.NoError(t, err) | ||
| 371 | login, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | 402 | login, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | ||
| 372 | require.NoError(t, err) | 403 | require.NoError(t, err) | ||
| 373 | - | assert.Equal(t, 2*time.Minute, usecase.PasskeyCeremonyTTL) | 404 | + | f.advance(domain.PasskeyCeremonyTTL) |
| 374 | - | f.advance(usecase.PasskeyCeremonyTTL) | 405 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, []byte(`{}`)) |
| 375 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), reg, "x", []byte(`{}`)) | |||
| 376 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 406 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 377 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | 407 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | ||
| 378 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 408 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| @@ -389,7 +419,7 @@ func TestExpiredRegistrationsDoNotCount(t *testing.T) { | |||||
| 389 | } | 419 | } | ||
| 390 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 420 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 391 | require.ErrorIs(t, err, domain.ErrResourceExhausted) | 421 | require.ErrorIs(t, err, domain.ErrResourceExhausted) | ||
| 392 | - | f.advance(usecase.PasskeyCeremonyTTL) | 422 | + | f.advance(domain.PasskeyCeremonyTTL) |
| 393 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 423 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 394 | assert.NoError(t, err) | 424 | assert.NoError(t, err) | ||
| 395 | } | 425 | } | ||
| @@ -413,7 +443,7 @@ func TestPendingRegistrationsAreBounded(t *testing.T) { | |||||
| 413 | // registration frees one. | 443 | // registration frees one. | ||
| 414 | _, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | 444 | _, _, err = f.passkeys.BeginLogin(ctx, "192.0.2.1") | ||
| 415 | require.NoError(t, err) | 445 | require.NoError(t, err) | ||
| 416 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), ids[0], "x", []byte(`{}`)) | 446 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), ids[0], []byte(`{}`)) |
| 417 | require.NoError(t, err) | 447 | require.NoError(t, err) | ||
| 418 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 448 | _, _, err = f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 419 | assert.NoError(t, err) | 449 | assert.NoError(t, err) | ||
| @@ -506,36 +536,6 @@ func TestPendingLoginsGlobalOverflow(t *testing.T) { | |||||
| 506 | assert.True(t, f.finishes(fresh), "the new login works") | 536 | assert.True(t, f.finishes(fresh), "the new login works") | ||
| 507 | } | 537 | } | ||
| 508 | 538 | ||||
| 509 | - | // IPv6 clients share a cap per /64; an IPv4-mapped address is the IPv4 one. | |||
| 510 | - | func TestPendingLoginsClientKey(t *testing.T) { | |||
| 511 | - | t.Parallel() | |||
| 512 | - | f := newPasskeyFixture(t) | |||
| 513 | - | f.add("cred-1") | |||
| 514 | - | f.acceptLogin("cred-1", 1) | |||
| 515 | - | perClient := usecase.MaxPendingPasskeyLoginsPerClient | |||
| 516 | - | ||||
| 517 | - | // Different addresses of one /64 count together. | |||
| 518 | - | ids := f.loginIDs(t, "2001:db8:1:2::1", perClient) | |||
| 519 | - | next := f.loginIDs(t, "2001:db8:1:2:ffff:ffff:ffff:ffff", 1)[0] | |||
| 520 | - | assert.False(t, f.finishes(ids[0]), "the same /64: the 9th evicted the oldest") | |||
| 521 | - | assert.True(t, f.finishes(next)) | |||
| 522 | - | ||||
| 523 | - | // Another /64 has its own cap. | |||
| 524 | - | other := f.loginIDs(t, "2001:db8:1:3::1", perClient) | |||
| 525 | - | for _, id := range other { | |||
| 526 | - | assert.True(t, f.finishes(id), "a different /64 is another client") | |||
| 527 | - | } | |||
| 528 | - | ||||
| 529 | - | // ::ffff:192.0.2.1 and 192.0.2.1 are one client. | |||
| 530 | - | v4 := f.loginIDs(t, "192.0.2.1", perClient) | |||
| 531 | - | f.loginIDs(t, "::ffff:192.0.2.1", 1) | |||
| 532 | - | assert.False(t, f.finishes(v4[0])) | |||
| 533 | - | // Two IPv4 addresses of one /24 are two clients. | |||
| 534 | - | a := f.loginIDs(t, "192.0.2.50", perClient) | |||
| 535 | - | f.loginIDs(t, "192.0.2.51", perClient) | |||
| 536 | - | assert.True(t, f.finishes(a[0])) | |||
| 537 | - | } | |||
| 538 | - | ||||
| 539 | // Logins never push out a registration, and registrations never push out a login. | 539 | // Logins never push out a registration, and registrations never push out a login. | ||
| 540 | func TestCeremonyKindsAreSeparate(t *testing.T) { | 540 | func TestCeremonyKindsAreSeparate(t *testing.T) { | ||
| 541 | t.Parallel() | 541 | t.Parallel() | ||
| @@ -554,7 +554,7 @@ func TestCeremonyKindsAreSeparate(t *testing.T) { | |||||
| 554 | // attempt burns it. | 554 | // attempt burns it. | ||
| 555 | _, _, err = f.passkeys.FinishLogin(ctx, reg, []byte(`{}`)) | 555 | _, _, err = f.passkeys.FinishLogin(ctx, reg, []byte(`{}`)) | ||
| 556 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 556 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 557 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), login, "x", []byte(`{}`)) | 557 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), login, []byte(`{}`)) |
| 558 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 558 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 559 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | 559 | _, _, err = f.passkeys.FinishLogin(ctx, login, []byte(`{}`)) | ||
| 560 | assert.ErrorIs(t, err, domain.ErrUnauthenticated, "burned by the mismatch") | 560 | assert.ErrorIs(t, err, domain.ErrUnauthenticated, "burned by the mismatch") | ||
| @@ -569,7 +569,7 @@ func TestUnknownCeremony(t *testing.T) { | |||||
| 569 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 569 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 570 | _, _, err = f.passkeys.FinishLogin(ctx, "nope", []byte(`{}`)) | 570 | _, _, err = f.passkeys.FinishLogin(ctx, "nope", []byte(`{}`)) | ||
| 571 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 571 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 572 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "nope", "x", []byte(`{}`)) | 572 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "nope", []byte(`{}`)) |
| 573 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | 573 | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 574 | } | 574 | } | ||
| 575 | 575 | ||||
| @@ -630,8 +630,23 @@ func (l *syncLog) String() string { | |||||
| 630 | return l.b.String() | 630 | return l.b.String() | ||
| 631 | } | 631 | } | ||
| 632 | 632 | ||||
| 633 | - | // GO-001: the reason of a refusal is logged (info), never answered, and holds | 633 | + | // logEntries parses the JSON lines of the buffer. |
| 634 | - | // no key material. | 634 | + | func logEntries(t *testing.T, logs *syncLog) []map[string]any { |
| 635 | + | t.Helper() | |||
| 636 | + | var out []map[string]any | |||
| 637 | + | for _, line := range strings.Split(strings.TrimSpace(logs.String()), "\n") { | |||
| 638 | + | if line == "" { | |||
| 639 | + | continue | |||
| 640 | + | } | |||
| 641 | + | var m map[string]any | |||
| 642 | + | require.NoError(t, json.Unmarshal([]byte(line), &m), line) | |||
| 643 | + | out = append(out, m) | |||
| 644 | + | } | |||
| 645 | + | return out | |||
| 646 | + | } | |||
| 647 | + | ||||
| 648 | + | // The reason of a refusal is logged at info, never answered, and holds no | |||
| 649 | + | // secret: neither the response bytes nor a key. | |||
| 635 | func TestRefusalsAreLogged(t *testing.T) { | 650 | func TestRefusalsAreLogged(t *testing.T) { | ||
| 636 | t.Parallel() | 651 | t.Parallel() | ||
| 637 | 652 | ||||
| @@ -646,20 +661,24 @@ func TestRefusalsAreLogged(t *testing.T) { | |||||
| 646 | require.NoError(t, err) | 661 | require.NoError(t, err) | ||
| 647 | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{"secret":"response-bytes"}`)) | 662 | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{"secret":"response-bytes"}`)) | ||
| 648 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | 663 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 649 | - | out := logs.String() | 664 | + | assert.NotContains(t, err.Error(), "origin mismatch", "the caller learns nothing") |
| 650 | - | assert.Contains(t, out, `"message":"passkey sign-in refused"`) | 665 | + | entries := logEntries(t, logs) |
| 651 | - | assert.Contains(t, out, `"level":"info"`) | 666 | + | require.Len(t, entries, 1) |
| 652 | - | assert.Contains(t, out, "origin mismatch") | 667 | + | assert.Equal(t, "info", entries[0]["level"]) |
| 653 | - | assert.NotContains(t, out, "response-bytes") | 668 | + | assert.Contains(t, entries[0]["error"], "origin mismatch") |
| 669 | + | assert.NotContains(t, logs.String(), "response-bytes") | |||
| 654 | }) | 670 | }) | ||
| 655 | 671 | ||||
| 656 | t.Run("sign-in with an unknown ceremony", func(t *testing.T) { | 672 | t.Run("sign-in with an unknown ceremony", func(t *testing.T) { | ||
| 657 | t.Parallel() | 673 | t.Parallel() | ||
| 658 | ctx, logs := logCtx(t) | 674 | ctx, logs := logCtx(t) | ||
| 659 | f := newPasskeyFixture(t) | 675 | f := newPasskeyFixture(t) | ||
| 660 | - | _, _, err := f.passkeys.FinishLogin(ctx, "nope", []byte(`{}`)) | 676 | + | _, _, err := f.passkeys.FinishLogin(ctx, "nope", []byte(`{"secret":"response-bytes"}`)) |
| 661 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | 677 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 662 | - | assert.Contains(t, logs.String(), "passkey sign-in refused") | 678 | + | entries := logEntries(t, logs) |
| 679 | + | require.Len(t, entries, 1) | |||
| 680 | + | assert.Equal(t, "info", entries[0]["level"]) | |||
| 681 | + | assert.NotContains(t, logs.String(), "response-bytes") | |||
| 663 | }) | 682 | }) | ||
| 664 | 683 | ||||
| 665 | t.Run("registration", func(t *testing.T) { | 684 | t.Run("registration", func(t *testing.T) { | ||
| @@ -668,48 +687,62 @@ func TestRefusalsAreLogged(t *testing.T) { | |||||
| 668 | f := newPasskeyFixture(t) | 687 | f := newPasskeyFixture(t) | ||
| 669 | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | 688 | f.verifier.EXPECT().FinishRegistration(testHandle, mock.Anything, mock.Anything). | ||
| 670 | Return(domain.Passkey{}, fmt.Errorf("%w: passkey registration: user verification missing", domain.ErrUnauthenticated)) | 689 | Return(domain.Passkey{}, fmt.Errorf("%w: passkey registration: user verification missing", domain.ErrUnauthenticated)) | ||
| 671 | - | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 690 | + | id, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "x") |
| 672 | require.NoError(t, err) | 691 | require.NoError(t, err) | ||
| 673 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, "x", []byte(`{"secret":"response-bytes"}`)) | 692 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), id, []byte(`{"secret":"response-bytes"}`)) |
| 674 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | 693 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 675 | - | out := logs.String() | 694 | + | entries := logEntries(t, logs) |
| 676 | - | assert.Contains(t, out, `"message":"passkey registration refused"`) | 695 | + | require.Len(t, entries, 1) |
| 677 | - | assert.Contains(t, out, "user verification missing") | 696 | + | assert.Equal(t, "info", entries[0]["level"]) |
| 678 | - | assert.NotContains(t, out, "response-bytes") | 697 | + | assert.Contains(t, entries[0]["error"], "user verification missing") |
| 679 | - | 698 | + | assert.NotContains(t, logs.String(), "response-bytes") | |
| 680 | - | _, err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "unknown", "x", []byte(`{}`)) | 699 | + | |
| 700 | + | // An unknown ceremony is logged too. | |||
| 701 | + | err = f.passkeys.FinishRegistration(ctx, domain.Owner(), "unknown", []byte(`{}`)) | |||
| 681 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | 702 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 682 | - | assert.Equal(t, 2, strings.Count(logs.String(), "passkey registration refused")) | 703 | + | entries = logEntries(t, logs) |
| 704 | + | require.Len(t, entries, 2) | |||
| 705 | + | assert.Equal(t, "info", entries[1]["level"]) | |||
| 683 | }) | 706 | }) | ||
| 684 | } | 707 | } | ||
| 685 | 708 | ||||
| 686 | - | // GO-002: the clone warning says which passkey, by its short reference. | 709 | + | // A counter that does not advance on a passkey that has one is refused, and the |
| 687 | - | func TestCloneWarningNamesThePasskey(t *testing.T) { | 710 | + | // warning names the passkey by its short reference. The library hands back the |
| 711 | + | // stored counter when the received one is not higher, which is what the verifier | |||
| 712 | + | // returns here. | |||
| 713 | + | func TestCloneIsRefusedAndNamed(t *testing.T) { | |||
| 688 | t.Parallel() | 714 | t.Parallel() | ||
| 689 | ctx, logs := logCtx(t) | 715 | ctx, logs := logCtx(t) | ||
| 690 | f := newPasskeyFixture(t) | 716 | f := newPasskeyFixture(t) | ||
| 691 | - | f.add("cred-1") | 717 | + | p := f.add("cred-1") |
| 718 | + | p.SignCount = 5 | |||
| 719 | + | f.mu.Lock() | |||
| 720 | + | f.stored["cred-1"] = p | |||
| 721 | + | f.mu.Unlock() | |||
| 692 | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | 722 | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything).RunAndReturn( | ||
| 693 | func(_, _, _ []byte, lookup func([]byte) (domain.Passkey, error)) (domain.Passkey, error) { | 723 | func(_, _, _ []byte, lookup func([]byte) (domain.Passkey, error)) (domain.Passkey, error) { | ||
| 694 | - | _, err := lookup([]byte("cred-1")) | 724 | + | return lookup([]byte("cred-1")) // SignCount stays 5 |
| 695 | - | require.NoError(t, err) | |||
| 696 | - | return domain.Passkey{}, domain.ErrPasskeyCloned | |||
| 697 | }) | 725 | }) | ||
| 698 | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | 726 | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | ||
| 699 | require.NoError(t, err) | 727 | require.NoError(t, err) | ||
| 700 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | 728 | + | token, _, err := f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) |
| 701 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | 729 | require.ErrorIs(t, err, domain.ErrUnauthenticated) | ||
| 730 | + | assert.Empty(t, token, "no session") | |||
| 731 | + | assert.Equal(t, 1, f.count()) | |||
| 732 | + | f.mu.Lock() | |||
| 733 | + | assert.True(t, f.stored["cred-1"].LastUsedAt.IsZero(), "nothing was recorded") | |||
| 734 | + | f.mu.Unlock() | |||
| 702 | 735 | ||||
| 703 | sum := sha256.Sum256([]byte("cred-1")) | 736 | sum := sha256.Sum256([]byte("cred-1")) | ||
| 704 | ref := hex.EncodeToString(sum[:])[:8] | 737 | ref := hex.EncodeToString(sum[:])[:8] | ||
| 705 | - | out := logs.String() | 738 | + | entries := logEntries(t, logs) |
| 706 | - | assert.Contains(t, out, `"level":"warn"`) | 739 | + | require.Len(t, entries, 1, "one log line for the refusal") |
| 707 | - | assert.Contains(t, out, `"passkey":"`+ref+`"`) | 740 | + | assert.Equal(t, "warn", entries[0]["level"]) |
| 708 | - | assert.Contains(t, out, "may be cloned") | 741 | + | assert.Equal(t, ref, entries[0]["passkey"]) |
| 709 | - | assert.NotContains(t, out, "cred-1", "the credential id itself is never logged") | 742 | + | assert.NotContains(t, logs.String(), "cred-1", "the credential id itself is never logged") |
| 710 | } | 743 | } | ||
| 711 | 744 | ||||
| 712 | - | // GO-003: two logins verified against the same stored counter cannot both | 745 | + | // Two logins verified against the same stored counter cannot both |
| 713 | // succeed. The verifier saw counter 5; by the time the second one writes, the | 746 | // succeed. The verifier saw counter 5; by the time the second one writes, the | ||
| 714 | // first has stored 6. | 747 | // first has stored 6. | ||
| 715 | func TestLoginRefusesACounterThatDidNotAdvance(t *testing.T) { | 748 | func TestLoginRefusesACounterThatDidNotAdvance(t *testing.T) { | ||
| @@ -773,21 +806,6 @@ func TestLoginRefusesACounterThatDidNotAdvance(t *testing.T) { | |||||
| 773 | } | 806 | } | ||
| 774 | }) | 807 | }) | ||
| 775 | 808 | ||||
| 776 | - | t.Run("equal counters are refused when the stored one is not 0", func(t *testing.T) { | |||
| 777 | - | t.Parallel() | |||
| 778 | - | f := newPasskeyFixture(t) | |||
| 779 | - | p := f.add("cred-1") | |||
| 780 | - | p.SignCount = 5 | |||
| 781 | - | f.mu.Lock() | |||
| 782 | - | f.stored["cred-1"] = p | |||
| 783 | - | f.mu.Unlock() | |||
| 784 | - | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything). | |||
| 785 | - | Return(domain.Passkey{ID: []byte("cred-1"), SignCount: 5}, nil) | |||
| 786 | - | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |||
| 787 | - | require.NoError(t, err) | |||
| 788 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |||
| 789 | - | assert.ErrorIs(t, err, domain.ErrUnauthenticated) | |||
| 790 | - | }) | |||
| 791 | } | 809 | } | ||
| 792 | 810 | ||||
| 793 | func TestLoginRefusals(t *testing.T) { | 811 | func TestLoginRefusals(t *testing.T) { | ||
| @@ -817,21 +835,6 @@ func TestLoginRefusals(t *testing.T) { | |||||
| 817 | assert.NotContains(t, err.Error(), "bad signature", "the reason is for logs, not for the caller") | 835 | assert.NotContains(t, err.Error(), "bad signature", "the reason is for logs, not for the caller") | ||
| 818 | }) | 836 | }) | ||
| 819 | 837 | ||||
| 820 | - | t.Run("cloned authenticator", func(t *testing.T) { | |||
| 821 | - | t.Parallel() | |||
| 822 | - | f := newPasskeyFixture(t) | |||
| 823 | - | p := f.add("cred-1") | |||
| 824 | - | f.verifier.EXPECT().FinishLogin(testHandle, mock.Anything, mock.Anything, mock.Anything). | |||
| 825 | - | Return(domain.Passkey{}, domain.ErrPasskeyCloned) | |||
| 826 | - | id, _, err := f.passkeys.BeginLogin(ctx, "192.0.2.1") | |||
| 827 | - | require.NoError(t, err) | |||
| 828 | - | _, _, err = f.passkeys.FinishLogin(ctx, id, []byte(`{}`)) | |||
| 829 | - | require.ErrorIs(t, err, domain.ErrUnauthenticated) | |||
| 830 | - | got, err := f.passkeys.List(ctx, domain.Owner()) | |||
| 831 | - | require.NoError(t, err) | |||
| 832 | - | assert.Equal(t, []domain.Passkey{p}, got, "nothing recorded") | |||
| 833 | - | }) | |||
| 834 | - | ||||
| 835 | t.Run("store failure fails closed and is not an authentication error", func(t *testing.T) { | 838 | t.Run("store failure fails closed and is not an authentication error", func(t *testing.T) { | ||
| 836 | t.Parallel() | 839 | t.Parallel() | ||
| 837 | f := newPasskeyFixture(t) | 840 | f := newPasskeyFixture(t) | ||
| @@ -1000,3 +1003,62 @@ func TestBeginRegistrationChecksTheName(t *testing.T) { | |||||
| 1000 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | 1003 | _, _, err := f.passkeys.BeginRegistration(ctx, domain.Owner(), "") | ||
| 1001 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | 1004 | assert.ErrorIs(t, err, domain.ErrResourceExhausted) | ||
| 1002 | } | 1005 | } | ||
| 1006 | + | ||||
| 1007 | + | // A client at its cap begins a login while the store is full. Its own eviction | |||
| 1008 | + | // makes the room the new login needs, so exactly one ceremony is dropped, the | |||
| 1009 | + | // client's own oldest: the global rule must not take a second one, from anyone. | |||
| 1010 | + | func TestFullClientOnAFullStoreLosesOnlyItsOwnOldest(t *testing.T) { | |||
| 1011 | + | t.Parallel() | |||
| 1012 | + | f := newPasskeyFixture(t) | |||
| 1013 | + | f.add("cred-1") | |||
| 1014 | + | f.acceptLogin("cred-1", 1) | |||
| 1015 | + | perClient := usecase.MaxPendingPasskeyLoginsPerClient | |||
| 1016 | + | ||||
| 1017 | + | // Seven clients with one login each, the flooder with a full cap, then full | |||
| 1018 | + | // clients and a last partial one until the store holds exactly its cap. | |||
| 1019 | + | small := make([]string, 0, 7) | |||
| 1020 | + | for i := range 7 { | |||
| 1021 | + | small = append(small, f.loginIDs(t, fmt.Sprintf("10.9.0.%d", i), 1)[0]) | |||
| 1022 | + | } | |||
| 1023 | + | flooder := f.loginIDs(t, "192.0.2.1", perClient) | |||
| 1024 | + | total := len(small) + len(flooder) | |||
| 1025 | + | var others []string | |||
| 1026 | + | for n := 0; total+perClient <= usecase.MaxPendingPasskeyLogins; n++ { | |||
| 1027 | + | others = append(others, f.loginIDs(t, fmt.Sprintf("10.0.%d.1", n), perClient)...) | |||
| 1028 | + | total += perClient | |||
| 1029 | + | } | |||
| 1030 | + | if rest := usecase.MaxPendingPasskeyLogins - total; rest > 0 { | |||
| 1031 | + | others = append(others, f.loginIDs(t, "10.8.0.1", rest)...) | |||
| 1032 | + | total += rest | |||
| 1033 | + | } | |||
| 1034 | + | require.Equal(t, usecase.MaxPendingPasskeyLogins, total, "the store is full") | |||
| 1035 | + | ||||
| 1036 | + | newest := f.loginIDs(t, "192.0.2.1", 1)[0] | |||
| 1037 | + | ||||
| 1038 | + | assert.False(t, f.finishes(flooder[0]), "the flooder lost its own oldest") | |||
| 1039 | + | for _, id := range append(append(flooder[1:], small...), others...) { | |||
| 1040 | + | assert.True(t, f.finishes(id), "nobody else lost a ceremony") | |||
| 1041 | + | } | |||
| 1042 | + | assert.True(t, f.finishes(newest), "the new login works") | |||
| 1043 | + | } | |||
| 1044 | + | ||||
| 1045 | + | // A failure of the store while removing a passkey is returned as it is, and | |||
| 1046 | + | // nobody is signed out for it. | |||
| 1047 | + | func TestRemoveReturnsAStoreError(t *testing.T) { | |||
| 1048 | + | t.Parallel() | |||
| 1049 | + | ctx := context.Background() | |||
| 1050 | + | f := newPasskeyFixture(t) | |||
| 1051 | + | boom := errors.New("disk on fire") | |||
| 1052 | + | store := mocks.NewPasskeyStore(t) | |||
| 1053 | + | store.EXPECT().Delete(mock.Anything, []byte("cred-1")).Return(boom) | |||
| 1054 | + | // The session store is a strict mock: a BumpEpoch would fail the test. | |||
| 1055 | + | pk := usecase.NewPasskeys(store, mocks.NewPasskeyVerifier(t), f.sessions, f.clock) | |||
| 1056 | + | tok, _, err := f.sessions.Login(ctx, f.nonce(t)) | |||
| 1057 | + | require.NoError(t, err) | |||
| 1058 | + | ||||
| 1059 | + | err = pk.Remove(ctx, domain.Owner(), []byte("cred-1")) | |||
| 1060 | + | require.ErrorIs(t, err, boom) | |||
| 1061 | + | assert.NotErrorIs(t, err, domain.ErrNotFound) | |||
| 1062 | + | _, err = f.sessions.Verify(ctx, tok) | |||
| 1063 | + | assert.NoError(t, err, "the epoch did not move: the sessions are still valid") | |||
| 1064 | + | } | |||