diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index eebe2fdc..cb43bfcb 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -171,7 +171,11 @@ belongs to the user agent that began the flow — the caller passes back the `SessionHandle` it bound to that browser (an HttpOnly cookie; `client/sessioncookie` sets and reads one), and a callback whose `state` doesn't match it is rejected before anything is -consumed, closing login CSRF ([RFC 9700 §4.7][bcp]) — then correlation +consumed, closing login CSRF ([RFC 9700 §4.7][bcp]); a native app whose +session store is its own on-device storage has that binding already, +and with `Config.CallbackBinding` set to `CallbackBindingDeviceLocalStore` +completes from the callback alone, `state` taken from the verified +signed response under Message Signing — then correlation state, issuer, JARM signature and claims, audience, expiry, response mode, authorization-code presence, error-response integrity, and replay. diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 403d787a..d52521d5 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -124,7 +124,10 @@ that listens on a port the operating system picks for each flow sets `Config.RedirectURI` to the port-less loopback URI and passes the port as `client.BeginAuthorizationRequest.RedirectPort`. A native client still authenticates like any other: give each app instance its own credentials, as attestation-based -client authentication does. +client authentication does. The rest of the native app's side — keys in +the platform key store, an on-device session store, completing after +the app is relaunched, token storage — is in `client`'s package doc, +under "Native apps". ## 4. Wire `Dependencies` and construct the server diff --git a/client/authorization_response.go b/client/authorization_response.go index c5340eae..c619496c 100644 --- a/client/authorization_response.go +++ b/client/authorization_response.go @@ -24,19 +24,12 @@ type AuthorizationCallback struct { // Session is the SessionHandle BeginAuthorization returned for this // attempt, recovered from wherever the caller bound it to the user // agent (see SessionHandle) — never from the callback itself. - // Required, unless Dependencies.Sessions declares - // storage.Capabilities.SingleUserAgent: a callback whose "state" - // doesn't match it is rejected before its session is consumed, so a - // callback URL delivered to a different browser can't complete - // someone else's flow. - // - // A native app whose session store is its own on-device storage — - // declaring SingleUserAgent — may leave it empty: the session is - // then the callback's own state, taken from the verified signed - // response under Message Signing. Nothing else writes that store, so - // a session found there was begun by this app. That is what lets an - // app the operating system stopped mid-authorization complete it - // after relaunching, with no handle kept in memory. + // Required — a callback whose "state" doesn't match it is rejected + // before its session is consumed, so a callback URL delivered to a + // different browser can't complete someone else's flow — unless + // Config.CallbackBinding is CallbackBindingDeviceLocalStore, for a + // native app whose session store is its own on-device storage: see + // that constant. Session SessionHandle } @@ -178,13 +171,13 @@ func (c *Client) consumeCallbackSession(ctx context.Context, cb AuthorizationCal // §4.7) before consuming anything: a mismatch leaves the session // intact for its rightful browser. session := cb.Session.value - if session == "" && c.sessionsSingleUserAgent() { - // The store holds only this user agent's sessions: one found by - // the callback's state is this user agent's. + if session == "" && c.cfg.CallbackBinding == CallbackBindingDeviceLocalStore { + // The store holds only this app's sessions: one found by the + // callback's state is this app's. session = state } if session == "" { - return sessionRecord{}, newError(ErrorInvalidRequest, "callback is not bound to a session: AuthorizationCallback.Session is required unless Dependencies.Sessions declares storage.Capabilities.SingleUserAgent", nil) + return sessionRecord{}, newError(ErrorInvalidRequest, "callback is not bound to a session: pass the SessionHandle kept with this user agent as AuthorizationCallback.Session (client/sessioncookie keeps it for a browser)", nil) } if subtle.ConstantTimeCompare([]byte(state), []byte(session)) != 1 { return sessionRecord{}, newError(ErrorInvalidRequest, "callback state does not match this user agent's session", nil) @@ -306,10 +299,3 @@ func paramString(params map[string]json.RawMessage, key string) (string, bool) { } return s, true } - -// sessionsSingleUserAgent reports whether Dependencies.Sessions declares -// storage.Capabilities.SingleUserAgent. -func (c *Client) sessionsSingleUserAgent() bool { - a, ok := c.deps.Sessions.(storage.StoreAssurance) - return ok && a.Capabilities().SingleUserAgent -} diff --git a/client/single_user_agent_test.go b/client/callback_binding_test.go similarity index 57% rename from client/single_user_agent_test.go rename to client/callback_binding_test.go index 56e90125..8fa187a5 100644 --- a/client/single_user_agent_test.go +++ b/client/callback_binding_test.go @@ -2,31 +2,22 @@ package client_test import ( "context" + "strings" "testing" "github.com/idfoundry/fapigo/client" - "github.com/idfoundry/fapigo/storage" - "github.com/idfoundry/fapigo/storage/memstore" ) -// appSessionStore is a native app's own session store: nothing but -// this app writes it, which it declares. -type appSessionStore struct{ storage.SessionStore } - -func (appSessionStore) Capabilities() storage.Capabilities { - return storage.Capabilities{SingleUserAgent: true} -} - -// TestCallbackWithoutSessionForASingleUserAgentStore covers an app the +// TestCallbackWithoutSessionForADeviceLocalStore covers an app the // operating system stopped mid-authorization: relaunched, it holds no -// SessionHandle, and completes from the callback alone, its store -// declaring it holds only this app's sessions. Under Message Signing the -// state comes from the verified signed response. -func TestCallbackWithoutSessionForASingleUserAgentStore(t *testing.T) { +// SessionHandle, and completes from the callback alone, configured with +// CallbackBindingDeviceLocalStore. Under Message Signing the state comes +// from the verified signed response. +func TestCallbackWithoutSessionForADeviceLocalStore(t *testing.T) { for name, messageSigned := range map[string]bool{"plain": false, "JARM": true} { t.Run(name, func(t *testing.T) { - c, as, _ := newTestClientWith(t, messageSigned, func(_ *client.Config, d *client.Dependencies) { - d.Sessions = appSessionStore{memstore.NewSessionStore()} + c, as, _ := newTestClientWith(t, messageSigned, func(cfg *client.Config, _ *client.Dependencies) { + cfg.CallbackBinding = client.CallbackBindingDeviceLocalStore }) ctx := context.Background() session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid", "accounts"}}) @@ -45,13 +36,13 @@ func TestCallbackWithoutSessionForASingleUserAgentStore(t *testing.T) { } } -// TestCallbackWithoutSessionStillRefused covers what the declaration -// doesn't change: a store that doesn't declare it still needs the -// handle, a callback whose state no session in the store has is still -// refused, and a handle given is still compared. +// TestCallbackWithoutSessionStillRefused covers what the binding +// doesn't change: the default still needs the handle, a callback whose +// state no session in the store has is still refused, and a handle +// given is still compared. func TestCallbackWithoutSessionStillRefused(t *testing.T) { ctx := context.Background() - t.Run("store not declaring it", func(t *testing.T) { + t.Run("the default binding", func(t *testing.T) { c, as, _ := newTestClient(t, false) session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}) if err != nil { @@ -63,8 +54,8 @@ func TestCallbackWithoutSessionStillRefused(t *testing.T) { } }) declaring := func(t *testing.T) (*client.Client, *fakeAS) { - c, as, _ := newTestClientWith(t, false, func(_ *client.Config, d *client.Dependencies) { - d.Sessions = appSessionStore{memstore.NewSessionStore()} + c, as, _ := newTestClientWith(t, false, func(cfg *client.Config, _ *client.Dependencies) { + cfg.CallbackBinding = client.CallbackBindingDeviceLocalStore }) return c, as } @@ -93,3 +84,26 @@ func TestCallbackWithoutSessionStillRefused(t *testing.T) { } }) } + +func TestNewRefusesAnUnknownCallbackBinding(t *testing.T) { + cfg, deps := validConfig(t), validDependencies(t) + cfg.CallbackBinding = 9 + if _, err := client.New(cfg, deps); err == nil { + t.Error("New accepted an unknown CallbackBinding") + } +} + +// TestMissingSessionErrorPointsAtTheHandle covers the refusal a web +// client that forgot its cookie meets: it says to pass the handle, and +// doesn't advertise turning the binding off. +func TestMissingSessionErrorPointsAtTheHandle(t *testing.T) { + c, as, _ := newTestClient(t, false) + session, err := c.BeginAuthorization(context.Background(), client.BeginAuthorizationRequest{Scope: []string{"openid"}}) + if err != nil { + t.Fatal(err) + } + _, err = c.CompleteAuthorization(context.Background(), client.AuthorizationCallback{RawQuery: as.callbackFor(t, session.Handle().String(), "c", "")}) + if err == nil || !strings.Contains(err.Error(), "sessioncookie") || strings.Contains(err.Error(), "DeviceLocal") { + t.Errorf("error = %v, want it to point at the handle and sessioncookie, not the device-local binding", err) + } +} diff --git a/client/client.go b/client/client.go index c82bc96e..b65124de 100644 --- a/client/client.go +++ b/client/client.go @@ -218,6 +218,9 @@ func validateAuthMethodAlgorithms(cfg Config) error { // Algorithms.DPoP is required only under SenderConstrainDPoP (the // default) — an mTLS-sender-constrained client never builds a DPoP // proof at all, so it never needs a DPoP signing algorithm. + if !cfg.CallbackBinding.IsValid() { + return fmt.Errorf("client: config: callback_binding %d is not a CallbackBinding", cfg.CallbackBinding) + } if cfg.SenderConstrain == storage.SenderConstrainDPoP && !cfg.Algorithms.DPoP.IsValid() { return fmt.Errorf("client: config: algorithms.dpop is required when sender_constrain is SenderConstrainDPoP") } diff --git a/client/config.go b/client/config.go index f65757bb..ab90be44 100644 --- a/client/config.go +++ b/client/config.go @@ -381,6 +381,12 @@ type Config struct { // names. RedirectURI string + // CallbackBinding is how an authorization callback is tied to the + // user agent that began it (RFC 9700 §4.7) — see CallbackBinding. + // The zero value, CallbackBindingSessionHandle, is right for every + // browser-based client. + CallbackBinding CallbackBinding + Endpoints Endpoints Profile Profile Algorithms Algorithms @@ -549,3 +555,42 @@ type FederationConfig struct { // key. Required when EntityID is set. Algorithm fapi.SignatureAlgorithm } + +// CallbackBinding is how HandleAuthorizationResponse ties a callback to +// the user agent that began its authorization, against login CSRF (RFC +// 9700 §4.7): the attacker starts a flow as themselves and delivers its +// callback to a victim, whose user agent would complete it. +type CallbackBinding uint8 + +const ( + // CallbackBindingSessionHandle requires AuthorizationCallback.Session: + // the SessionHandle the application kept with the user agent that + // began the flow (a cookie; client/sessioncookie). A callback whose + // state doesn't match it is refused. The zero value, and the only + // right choice wherever Dependencies.Sessions holds more than one + // user agent's sessions — any server-side store — since an attacker's + // own session is found there by its state too. + CallbackBindingSessionHandle CallbackBinding = iota + + // CallbackBindingDeviceLocalStore is for a native app whose + // Dependencies.Sessions is its own storage on the device, which + // nothing but this app writes. A session found there by a callback's + // state was begun by this app, so AuthorizationCallback.Session may + // be left empty: the session is the callback's own state, taken from + // the verified signed response under Message Signing. That lets an + // app the operating system stopped mid-authorization complete it + // after relaunching, with no handle kept. (An app may instead save + // Handle().String() on the device and pass it back, as any client + // does.) A Session that is given is still compared. Any authorization + // still pending in the store can be completed this way, not only the + // latest, so keep Limits.SessionLifetime short. Never set it for a + // store a server shares between users: it would bring login CSRF + // back. + CallbackBindingDeviceLocalStore +) + +// IsValid reports whether b is one of this package's CallbackBinding +// values. +func (b CallbackBinding) IsValid() bool { + return b == CallbackBindingSessionHandle || b == CallbackBindingDeviceLocalStore +} diff --git a/client/dependencies.go b/client/dependencies.go index b6222356..bea8cbc4 100644 --- a/client/dependencies.go +++ b/client/dependencies.go @@ -12,7 +12,11 @@ import ( // nil value for any field — there is no implicit fallback (no default // clock, no silently-installed in-memory session store). type Dependencies struct { - // Sessions persists in-progress authorization-flow state. + // Sessions persists in-progress authorization-flow state, until + // Limits.SessionLifetime. Under AssuranceProduction it must declare + // Durable and AtomicConsume (storage.StoreAssurance). A native app's + // own on-device store: see "Native apps" in the package doc, and + // Config.CallbackBinding. Sessions storage.SessionStore // Keys performs this client's own signing operations: client diff --git a/client/doc.go b/client/doc.go index 4098f8e6..016f7c4e 100644 --- a/client/doc.go +++ b/client/doc.go @@ -73,7 +73,9 @@ // its own String form (ParseSessionHandle) — the caller stores it with // the user agent that began the flow (package client/sessioncookie does // this), and HandleAuthorizationResponse -// rejects a callback that doesn't carry the matching one; +// rejects a callback that doesn't carry the matching one — unless +// Config.CallbackBinding is CallbackBindingDeviceLocalStore, for a +// native app whose session store is its own (see "Native apps" below); // HandleAuthorizationResponse returns a closed sum type // rather than one struct with optional fields, so a caller can't assume // every callback carries a code; every DPoP proof, request-object @@ -87,4 +89,43 @@ // error response, when there was one, available through // Error.ServerResponse — not a bare error the caller has to // string-match. +// +// # Native apps +// +// A mobile or desktop app — a wallet, say — uses this package as any +// client does, with these differences: +// +// - Registration. The authorization server registers the client as a +// native app (storage.ApplicationTypeNative), so it may use a +// private-use scheme redirect, written with a single slash +// (com.example.wallet:/callback), or loopback http to 127.0.0.1 or +// [::1]. For loopback on a port the operating system picks per flow, +// set Config.RedirectURI without a port and pass the port as +// BeginAuthorizationRequest.RedirectPort. The app still +// authenticates: give each installation its own credentials, as +// attestation-based client authentication does. +// - Keys. Hold the DPoP and attestation keys in the platform's key +// store (the iOS Keychain or Secure Enclave, the Android Keystore) +// behind crypto.Signer, through keys.NewKeyManagerFromSigners with +// keys.DeclareCustody(keys.KeyCustody{Durable: true}) — see +// KeyCustody.Durable. Keep each key until the tokens bound to it are +// discarded. The Secure Enclave signs ES256 only. +// - Sessions. Dependencies.Sessions is the app's own durable storage +// (a file, SQLite) in its data container, declaring Durable and +// AtomicConsume (storage.Capabilities). A mutex makes Consume +// atomic for one process; an extension or widget sharing the +// container needs a file lock or a transaction. +// - Relaunch. If the operating system stops the app while the user is +// at the authorization server, the callback reaches a fresh process. +// Build the Client again from the same Config, keys and session +// store, and call CompleteAuthorization with the callback's +// RawQuery. Set Config.CallbackBinding to +// CallbackBindingDeviceLocalStore so it needs no SessionHandle, or +// save Handle().String() on the device when the flow begins and +// pass it back. +// - Tokens. Keep them between launches sealed, with a TokenSetSealer. +// - Errors. Error() may include text the authorization server wrote. +// An app that hands errors to platform code — gomobile turns a Go +// error into an NSError with Error() as its message — should map an +// *Error to Code() and ServerResponse() in its own Go layer instead. package client diff --git a/client/errors.go b/client/errors.go index 028c98d8..713ba91d 100644 --- a/client/errors.go +++ b/client/errors.go @@ -21,10 +21,13 @@ const ( ErrorResponseTooLarge ErrorCode = "response_too_large" ) -// Error is the error type every public Client method returns. Code and -// PublicDescription are safe to surface to the embedding application -// (e.g. in a UI message); the underlying cause (available via Unwrap, -// and included in Error's own message) is for logs only. +// Error is the error type every public Client method returns. Code is +// safe to surface to the embedding application. PublicDescription is +// this package's own text, or, when the error came from a server's error +// response, that server's error_description (see ServerResponse): meant +// for an operator, not shown to a user as is. The underlying cause +// (available via Unwrap, and included in Error's own message) is for +// logs only. type Error struct { code ErrorCode description string @@ -50,7 +53,8 @@ func (e *Error) PublicDescription() string { return e.description } // It never includes a response body. It may include text the remote // server wrote: an authorization server error response's // error_description (PublicDescription), limited to RFC 6749's printable -// ASCII, and its error code — quoted, when it isn't RFC 6749 error text. +// ASCII, and its error code — quoted, and cut to 64 bytes, when it isn't +// RFC 6749 error text. // Where only this client's own words may be logged, log Code() and // ServerResponse()'s Code and HTTPStatus instead. func (e *Error) Error() string { diff --git a/client/server_error.go b/client/server_error.go index fe68dab2..45df8672 100644 --- a/client/server_error.go +++ b/client/server_error.go @@ -88,6 +88,12 @@ func parErrorFromResponse(status int, body []byte) *Error { resp := newServerErrorResponse(status, errResp.Code, errResp.Description, errResp.URI) code := errResp.Code if resp.Code == "" { + // Quoted, and cut short: only bounded text from the server + // reaches Error(). + const maxShown = 64 + if len(code) > maxShown { + code = code[:maxShown] + "…" + } code = strconv.Quote(code) } return newError(ErrorInvalidResponse, resp.Description, fmt.Errorf("authorization server error: %s", code)). diff --git a/client/server_error_test.go b/client/server_error_test.go index f4ca663c..3762c643 100644 --- a/client/server_error_test.go +++ b/client/server_error_test.go @@ -301,3 +301,17 @@ func TestCallbackDeniedEnforcesErrorCharacterSet(t *testing.T) { }) } } + +// TestPARErrorBoundsAMalformedCode covers an error code that isn't RFC +// 6749 error text and is long: Error() shows only its first 64 bytes. +func TestPARErrorBoundsAMalformedCode(t *testing.T) { + long := "bad\n" + strings.Repeat("x", 500) + c := newPARErrorTestClient(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(map[string]string{"error": long}) + }) + if msg := beginAuthorizationError(t, c).Error(); strings.Contains(msg, strings.Repeat("x", 100)) || !strings.Contains(msg, "xxx") { + t.Errorf("Error() = %q, want the malformed code cut to 64 bytes", msg) + } +} diff --git a/client/session.go b/client/session.go index 3388e622..61f3bbcd 100644 --- a/client/session.go +++ b/client/session.go @@ -41,9 +41,8 @@ func generateRandomToken(random io.Reader) (string, error) { // HandleAuthorizationResponse and CompleteAuthorization reject a // callback whose Session doesn't match its "state". // -// A native app whose session store is its own on-device storage, and -// declares storage.Capabilities.SingleUserAgent, has that binding -// already: see AuthorizationCallback.Session. +// A native app whose session store is its own on-device storage has that +// binding already: see CallbackBindingDeviceLocalStore. type SessionHandle struct { value string } diff --git a/keys/custody.go b/keys/custody.go index 918f4bd7..4ca99926 100644 --- a/keys/custody.go +++ b/keys/custody.go @@ -17,10 +17,10 @@ type KeyCustody struct { // For a client, it means a key outlives the process that made it // until the client is done with it: an authorization in progress can // be completed after a restart, and a DPoP-bound access or refresh - // token stays usable. A native app's keys created for one flow and - // deleted when it ends, but kept meanwhile in the platform's key - // store (the iOS Keychain or Secure Enclave, the Android Keystore), - // are Durable in this sense. + // token stays usable. A native app's keys created for one issuance, + // kept in the platform's key store (the iOS Keychain or Secure + // Enclave, the Android Keystore) and deleted only once the tokens + // bound to them are discarded, are Durable in this sense. Durable bool // CrossInstanceConsistent is true iff every instance of a diff --git a/storage/assurance.go b/storage/assurance.go index 87fea391..80ac8b49 100644 --- a/storage/assurance.go +++ b/storage/assurance.go @@ -58,17 +58,4 @@ type Capabilities struct { // EncryptedAtRest means persisted data is encrypted at rest. EncryptedAtRest bool - - // SingleUserAgent means a client.SessionStore holds only the - // authorizations begun by the one user agent it serves: a native - // app's own on-device storage, say, which nothing else writes. A - // session found by a callback's state was then begun by the user - // agent the callback reached, which is what binding the session - // handle to the user agent establishes (RFC 9700 §4.7), so the - // client may take the session from the callback itself - // (client.AuthorizationCallback.Session left empty). Never declare it - // for a store shared by many users' sessions — a web application's - // database, a server-side cache — where an attacker's own session is - // also found by its state, and the binding is what refuses it. - SingleUserAgent bool }