diff --git a/lxd/api_1.0.go b/lxd/api_1.0.go index 8d278d7a2817..ec6593127cac 100644 --- a/lxd/api_1.0.go +++ b/lxd/api_1.0.go @@ -454,7 +454,7 @@ func api10Put(d *Daemon, r *http.Request) response.Response { logger.Debug("Handling config changed notification") changed := make(map[string]string) for key, value := range req.Config { - changed[key] = value.(string) + changed[key], _ = value.(string) } // Get the current (updated) config. @@ -474,7 +474,7 @@ func api10Put(d *Daemon, r *http.Request) response.Response { d.globalConfigMu.Unlock() // Run any update triggers. - err = doApi10UpdateTriggers(d, nil, changed, s.LocalConfig, config) + err = doAPI10UpdateTriggers(d, nil, changed, s.LocalConfig, config) if err != nil { return response.SmartError(err) } @@ -492,7 +492,7 @@ func api10Put(d *Daemon, r *http.Request) response.Response { return response.PreconditionFailed(err) } - return doApi10Update(d, r, req, false) + return doAPI10Update(d, r, req, false) } // swagger:operation PATCH /1.0 server server_patch @@ -561,10 +561,10 @@ func api10Patch(d *Daemon, r *http.Request) response.Response { return response.EmptySyncResponse } - return doApi10Update(d, r, req, true) + return doAPI10Update(d, r, req, true) } -func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) response.Response { +func doAPI10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) response.Response { s := d.State() // First deal with config specific to the local daemon @@ -601,14 +601,15 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re return fmt.Errorf("Cannot fetch node config from database: %w", err) } - newClusterHTTPSAddress, found := nodeValues["cluster.https_address"] - if !found && patch { + newClusterHTTPSAddress := "" + newClusterHTTPSAddressAny, found := nodeValues["cluster.https_address"] + if found { + newClusterHTTPSAddress, _ = newClusterHTTPSAddressAny.(string) + } else if patch { newClusterHTTPSAddress = curConfig["cluster.https_address"] - } else if !found { - newClusterHTTPSAddress = "" } - if curConfig["cluster.https_address"] != newClusterHTTPSAddress.(string) { + if curConfig["cluster.https_address"] != newClusterHTTPSAddress { return fmt.Errorf("Changing cluster.https_address is currently not supported") } } @@ -792,7 +793,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re d.globalConfigMu.Unlock() // Run any update triggers. - err = doApi10UpdateTriggers(d, nodeChanged, clusterChanged, newNodeConfig, newClusterConfig) + err = doAPI10UpdateTriggers(d, nodeChanged, clusterChanged, newNodeConfig, newClusterConfig) if err != nil { return response.SmartError(err) } @@ -804,7 +805,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re return response.EmptySyncResponse } -func doApi10UpdateTriggers(d *Daemon, nodeChanged, clusterChanged map[string]string, nodeConfig *node.Config, clusterConfig *clusterConfig.Config) error { +func doAPI10UpdateTriggers(d *Daemon, nodeChanged, clusterChanged map[string]string, nodeConfig *node.Config, clusterConfig *clusterConfig.Config) error { s := d.State() maasChanged := false @@ -1059,7 +1060,7 @@ func doApi10UpdateTriggers(d *Daemon, nodeChanged, clusterChanged map[string]str d.oidcVerifier = nil } else { var err error - d.oidcVerifier, err = oidc.NewVerifier(oidcIssuer, oidcClientID, oidcAudience) + d.oidcVerifier, err = oidc.NewVerifier(oidcIssuer, oidcClientID, oidcAudience, s.ServerCert, nil) if err != nil { return fmt.Errorf("Failed creating verifier: %w", err) } diff --git a/lxd/api_cluster.go b/lxd/api_cluster.go index 0b42337b23d7..487756736bb4 100644 --- a/lxd/api_cluster.go +++ b/lxd/api_cluster.go @@ -781,7 +781,7 @@ func clusterPutJoin(d *Daemon, r *http.Request, req api.ClusterPut) response.Res changes[k], _ = v.(string) } - err = doApi10UpdateTriggers(d, nil, changes, nodeConfig, currentClusterConfig) + err = doAPI10UpdateTriggers(d, nil, changes, nodeConfig, currentClusterConfig) if err != nil { return err } diff --git a/lxd/auth/oidc/oidc.go b/lxd/auth/oidc/oidc.go index f52fddee9143..7317159fb6ee 100644 --- a/lxd/auth/oidc/oidc.go +++ b/lxd/auth/oidc/oidc.go @@ -2,18 +2,22 @@ package oidc import ( "context" + "crypto/sha512" "errors" "fmt" + "io" "net/http" "strings" "time" "github.com/google/uuid" + "github.com/gorilla/securecookie" "github.com/zitadel/oidc/v2/pkg/client" "github.com/zitadel/oidc/v2/pkg/client/rp" httphelper "github.com/zitadel/oidc/v2/pkg/http" "github.com/zitadel/oidc/v2/pkg/oidc" "github.com/zitadel/oidc/v2/pkg/op" + "golang.org/x/crypto/hkdf" "github.com/canonical/lxd/lxd/response" "github.com/canonical/lxd/shared" @@ -25,6 +29,13 @@ const ( // cookieNameRefreshToken is the identifier used to set and retrieve the refresh token. cookieNameRefreshToken = "oidc_refresh" + + // cookieNameSessionID is used to identify the session. It does not need to be encrypted. + cookieNameSessionID = "session_id" +) + +const ( + defaultConfigExpiryInterval = 5 * time.Minute ) // Verifier holds all information needed to verify an access token offline. @@ -32,15 +43,20 @@ type Verifier struct { accessTokenVerifier op.AccessTokenVerifier relyingParty rp.RelyingParty - clientID string - issuer string - audience string - cookieKey []byte + clientID string + issuer string + audience string + clusterCert func() *shared.CertInfo // host is used for setting a valid callback URL when setting the relyingParty. // When creating the relyingParty, the OIDC library performs discovery (e.g. it calls the /well-known/oidc-configuration endpoint). // We don't want to perform this on every request, so we only do it when the request host changes. host string + + // configExpiry is the next time at which the relying party and access token verifier will be considered out of date + // and will be refreshed. This refreshes the cookie encryption keys that the relying party uses. + configExpiry time.Time + configExpiryInterval time.Duration } // AuthError represents an authentication error. @@ -67,11 +83,11 @@ func (o *Verifier) Auth(ctx context.Context, w http.ResponseWriter, r *http.Requ authorizationHeader := r.Header.Get("Authorization") - idToken, refreshToken, err := o.getCookies(r) + _, idToken, refreshToken, err := o.getCookies(r) if err != nil { // Cookies are present but we failed to decrypt them. They may have been tampered with, so delete them to force // the user to log in again. - _ = o.setCookies(w, "", "", true) + _ = o.setCookies(w, nil, uuid.UUID{}, "", "", true) return "", fmt.Errorf("Failed to retrieve login information: %w", err) } @@ -146,8 +162,14 @@ func (o *Verifier) authenticateIDToken(ctx context.Context, w http.ResponseWrite return "", AuthError{Err: fmt.Errorf("Failed to verify refreshed ID token: %w", err)} } + sessionID := uuid.New() + secureCookie, err := o.secureCookieFromSession(sessionID) + if err != nil { + return "", AuthError{Err: fmt.Errorf("Failed to create new session with refreshed token: %w", err)} + } + // Update the cookies. - err = o.setCookies(w, idToken, tokens.RefreshToken, false) + err = o.setCookies(w, secureCookie, sessionID, idToken, tokens.RefreshToken, false) if err != nil { return "", fmt.Errorf("Failed to update login cookies: %w", err) } @@ -169,7 +191,7 @@ func (o *Verifier) Login(w http.ResponseWriter, r *http.Request) { // Logout deletes the ID and refresh token cookies and redirects the user to the login page. func (o *Verifier) Logout(w http.ResponseWriter, r *http.Request) { - err := o.setCookies(w, "", "", true) + err := o.setCookies(w, nil, uuid.UUID{}, "", "", true) if err != nil { _ = response.ErrorResponse(http.StatusInternalServerError, fmt.Errorf("Failed to delete login information: %w", err).Error()).Render(w) return @@ -187,7 +209,14 @@ func (o *Verifier) Callback(w http.ResponseWriter, r *http.Request) { } handler := rp.CodeExchangeHandler(func(w http.ResponseWriter, r *http.Request, tokens *oidc.Tokens[*oidc.IDTokenClaims], state string, rp rp.RelyingParty) { - err := o.setCookies(w, tokens.IDToken, tokens.RefreshToken, false) + sessionID := uuid.New() + secureCookie, err := o.secureCookieFromSession(sessionID) + if err != nil { + _ = response.ErrorResponse(http.StatusInternalServerError, fmt.Errorf("Failed to start a new session: %w", err).Error()).Render(w) + return + } + + err = o.setCookies(w, secureCookie, sessionID, tokens.IDToken, tokens.RefreshToken, false) if err != nil { _ = response.ErrorResponse(http.StatusInternalServerError, fmt.Errorf("Failed to set login information: %w", err).Error()).Render(w) return @@ -211,12 +240,17 @@ func (o *Verifier) WriteHeaders(w http.ResponseWriter) error { } // IsRequest checks if the request is using OIDC authentication. We check for the presence of the Authorization header -// or one of the ID or refresh tokens. -func (o *Verifier) IsRequest(r *http.Request) bool { +// or one of the ID or refresh tokens and the session cookie. +func (*Verifier) IsRequest(r *http.Request) bool { if r.Header.Get("Authorization") != "" { return true } + _, err := r.Cookie(cookieNameSessionID) + if err != nil { + return false + } + idTokenCookie, err := r.Cookie(cookieNameIDToken) if err == nil && idTokenCookie != nil { return true @@ -234,13 +268,14 @@ func (o *Verifier) IsRequest(r *http.Request) bool { // if the given host is different from the Verifier host we reset the relyingParty to ensure the callback URL is set // correctly. func (o *Verifier) ensureConfig(host string) error { - if o.relyingParty == nil || host != o.host { + if o.relyingParty == nil || host != o.host || time.Now().After(o.configExpiry) { err := o.setRelyingParty(host) if err != nil { return err } o.host = host + o.configExpiry = time.Now().Add(o.configExpiryInterval) } if o.accessTokenVerifier == nil { @@ -255,7 +290,26 @@ func (o *Verifier) ensureConfig(host string) error { // setRelyingParty sets the relyingParty on the Verifier. The host argument is used to set a valid callback URL. func (o *Verifier) setRelyingParty(host string) error { - cookieHandler := httphelper.NewCookieHandler(o.cookieKey, o.cookieKey) + // The relying party sets cookies for the following values: + // - "state": Used to prevent CSRF attacks (https://datatracker.ietf.org/doc/html/rfc6749#section-10.12). + // - "pkce": Used to prevent authorization code interception attacks (https://datatracker.ietf.org/doc/html/rfc7636). + // Both should be stored securely. However, these cookies do not need to be decrypted by other cluster members, so + // it is ok to use the secure key generation that is built in to the securecookie library. This also reduces the + // exposure of our private key. + + // The hash key should be 64 bytes (https://github.com/gorilla/securecookie). + cookieHashKey := securecookie.GenerateRandomKey(64) + if cookieHashKey == nil { + return errors.New("Failed to generate a secure cookie hash key") + } + + // The block key should 32 bytes for AES-256 encryption. + cookieBlockKey := securecookie.GenerateRandomKey(32) + if cookieBlockKey == nil { + return errors.New("Failed to generate a secure cookie hash key") + } + + cookieHandler := httphelper.NewCookieHandler(cookieHashKey, cookieBlockKey) options := []rp.Option{ rp.WithCookieHandler(cookieHandler), rp.WithVerifierOpts(rp.WithIssuedAtOffset(5 * time.Second)), @@ -292,81 +346,183 @@ func (o *Verifier) setAccessTokenVerifier() error { return nil } -// getCookies gets the ID and refresh tokens from the request cookies. -func (*Verifier) getCookies(r *http.Request) (idToken string, refreshToken string, err error) { +// getCookies gets the sessionID, identity and refresh tokens from the request cookies and decrypts them. +func (o *Verifier) getCookies(r *http.Request) (sessionIDPtr *uuid.UUID, idToken string, refreshToken string, err error) { + sessionIDCookie, err := r.Cookie(cookieNameSessionID) + if err != nil && !errors.Is(err, http.ErrNoCookie) { + return nil, "", "", fmt.Errorf("Failed to get session ID cookie from request: %w", err) + } else if sessionIDCookie == nil { + return nil, "", "", nil + } + + sessionID, err := uuid.Parse(sessionIDCookie.Value) + if err != nil { + return nil, "", "", fmt.Errorf("Invalid session ID cookie: %w", err) + } + + secureCookie, err := o.secureCookieFromSession(sessionID) + if err != nil { + return nil, "", "", fmt.Errorf("Failed to decrypt cookies: %w", err) + } + idTokenCookie, err := r.Cookie(cookieNameIDToken) if err != nil && !errors.Is(err, http.ErrNoCookie) { - return "", "", fmt.Errorf("Failed to get ID token cookie from request: %w", err) + return nil, "", "", fmt.Errorf("Failed to get ID token cookie from request: %w", err) } if idTokenCookie != nil { - idToken = idTokenCookie.Value + err = secureCookie.Decode(cookieNameIDToken, idTokenCookie.Value, &idToken) + if err != nil { + return nil, "", "", fmt.Errorf("Failed to decrypt ID token cookie: %w", err) + } } refreshTokenCookie, err := r.Cookie(cookieNameRefreshToken) if err != nil && !errors.Is(err, http.ErrNoCookie) { - return "", "", fmt.Errorf("Failed to get refresh token cookie from request: %w", err) + return nil, "", "", fmt.Errorf("Failed to get refresh token cookie from request: %w", err) } if refreshTokenCookie != nil { - refreshToken = refreshTokenCookie.Value + err = secureCookie.Decode(cookieNameRefreshToken, refreshTokenCookie.Value, &refreshToken) + if err != nil { + return nil, "", "", fmt.Errorf("Failed to decrypt refresh token cookie: %w", err) + } } - return idToken, refreshToken, nil + return &sessionID, idToken, refreshToken, nil } -// setCookies sets the ID and refresh tokens in the HTTP response. Cookies are only set if they are +// setCookies encrypts the session, ID, and refresh tokens and sets them in the HTTP response. Cookies are only set if they are // non-empty. If delete is true, the values are set to empty strings and the cookie expiry is set to unix zero time. -func (*Verifier) setCookies(w http.ResponseWriter, idToken string, refreshToken string, delete bool) error { - if idToken != "" || delete { - idTokenCookie := http.Cookie{ - Name: cookieNameIDToken, - Path: "/", - Secure: true, - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - } - - if delete { - idTokenCookie.Value = "" - idTokenCookie.Expires = time.Unix(0, 0) - } else { - idTokenCookie.Value = idToken - } +func (*Verifier) setCookies(w http.ResponseWriter, secureCookie *securecookie.SecureCookie, sessionID uuid.UUID, idToken string, refreshToken string, delete bool) error { + idTokenCookie := http.Cookie{ + Name: cookieNameIDToken, + Path: "/", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } - http.SetCookie(w, &idTokenCookie) + refreshTokenCookie := http.Cookie{ + Name: cookieNameRefreshToken, + Path: "/", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, } - if refreshToken != "" || delete { - refreshTokenCookie := http.Cookie{ - Name: cookieNameRefreshToken, - Path: "/", - Secure: true, - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - } + sessionIDCookie := http.Cookie{ + Name: cookieNameSessionID, + Path: "/", + Secure: true, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + } - if delete { - refreshTokenCookie.Value = "" - refreshTokenCookie.Expires = time.Unix(0, 0) - } else { - refreshTokenCookie.Value = refreshToken - } + if delete { + idTokenCookie.Expires = time.Unix(0, 0) + refreshTokenCookie.Expires = time.Unix(0, 0) + sessionIDCookie.Expires = time.Unix(0, 0) + http.SetCookie(w, &idTokenCookie) http.SetCookie(w, &refreshTokenCookie) + http.SetCookie(w, &sessionIDCookie) + return nil + } + + encodedIDTokenCookie, err := secureCookie.Encode(cookieNameIDToken, idToken) + if err != nil { + return fmt.Errorf("Failed to encrypt ID token: %w", err) } + encodedRefreshToken, err := secureCookie.Encode(cookieNameRefreshToken, refreshToken) + if err != nil { + return fmt.Errorf("Failed to encrypt refresh token: %w", err) + } + + sessionIDCookie.Value = sessionID.String() + idTokenCookie.Value = encodedIDTokenCookie + refreshTokenCookie.Value = encodedRefreshToken + + http.SetCookie(w, &idTokenCookie) + http.SetCookie(w, &refreshTokenCookie) + http.SetCookie(w, &sessionIDCookie) return nil } -// NewVerifier returns a Verifier. -func NewVerifier(issuer string, clientid string, audience string) (*Verifier, error) { - cookieKey, err := uuid.New().MarshalBinary() +// secureCookieFromSession returns a *securecookie.SecureCookie that is secure, unique to each client, and possible to +// decrypt on all cluster members. +// +// To do this we use the cluster private key as an input seed to HKDF (https://datatracker.ietf.org/doc/html/rfc5869) and +// use the given sessionID uuid.UUID as a salt. The session ID can then be stored as a plaintext cookie so that we can +// regenerate the keys upon the next request. +// +// Warning: Changes to this function might cause all existing OIDC users to be logged out of LXD (but not logged out of +// the IdP). +func (o *Verifier) secureCookieFromSession(sessionID uuid.UUID) (*securecookie.SecureCookie, error) { + // Get the sessionID as a binary so that we can use it as a salt. + salt, err := sessionID.MarshalBinary() + if err != nil { + return nil, fmt.Errorf("Failed to marshal session ID as binary: %w", err) + } + + // Get the current cluster private key. + clusterPrivateKey := o.clusterCert().PrivateKey() + + // Extract a pseudo-random key from the cluster private key. + prk := hkdf.Extract(sha512.New, clusterPrivateKey, salt) + + // Get an io.Reader from which we can read a secure key. We will use this key as the hash key for the cookie. + // The hash key is used to verify the integrity of decrypted values using HMAC. The HKDF "info" is set to "INTEGRITY" + // to indicate the intended usage of the key and prevent decryption in other contexts + // (see https://datatracker.ietf.org/doc/html/rfc5869#section-3.2). + keyDerivationFunc := hkdf.Expand(sha512.New, prk, []byte("INTEGRITY")) + + // Read 64 bytes of the derived key. The securecookie library recommends 64 bytes for the hash key (https://github.com/gorilla/securecookie). + cookieHashKey := make([]byte, 64) + _, err = io.ReadFull(keyDerivationFunc, cookieHashKey) if err != nil { - return nil, fmt.Errorf("Failed to create UUID: %w", err) + return nil, fmt.Errorf("Failed creating secure cookie hash key: %w", err) } - verifier := &Verifier{issuer: issuer, clientID: clientid, audience: audience, cookieKey: cookieKey} + // Get an io.Reader from which we can read a secure key. We will use this key as the block key for the cookie. + // The block key is used by securecookie to perform AES encryption. The HKDF "info" is set to "ENCRYPTION" + // to indicate the intended usage of the key and prevent decryption in other contexts + // (see https://datatracker.ietf.org/doc/html/rfc5869#section-3.2). + keyDerivationFunc = hkdf.Expand(sha512.New, prk, []byte("ENCRYPTION")) + + // Read 32 bytes of the derived key. Given 32 bytes for the block key the securecookie library will use AES-256 for encryption. + cookieBlockKey := make([]byte, 32) + _, err = io.ReadFull(keyDerivationFunc, cookieBlockKey) + if err != nil { + return nil, fmt.Errorf("Failed creating secure cookie block key: %w", err) + } + + return securecookie.New(cookieHashKey, cookieBlockKey), nil +} + +// Opts contains optional configurable fields for the Verifier. +type Opts struct { + ConfigExpiryInterval time.Duration +} + +// NewVerifier returns a Verifier. +func NewVerifier(issuer string, clientID string, audience string, clusterCert func() *shared.CertInfo, options *Opts) (*Verifier, error) { + opts := &Opts{ + ConfigExpiryInterval: defaultConfigExpiryInterval, + } + + if options != nil { + opts.ConfigExpiryInterval = options.ConfigExpiryInterval + } + + verifier := &Verifier{ + issuer: issuer, + clientID: clientID, + audience: audience, + clusterCert: clusterCert, + configExpiryInterval: opts.ConfigExpiryInterval, + } return verifier, nil } diff --git a/lxd/daemon.go b/lxd/daemon.go index d530ddad317c..51bb802cfad2 100644 --- a/lxd/daemon.go +++ b/lxd/daemon.go @@ -297,7 +297,7 @@ func (d *Daemon) getTrustedCertificates() map[certificate.Type]map[string]x509.C // This does not perform authorization, only validates authentication. // Returns whether trusted or not, the username (or certificate fingerprint) of the trusted client, and the type of // client that has been authenticated (cluster, unix, candid or tls). -func (d *Daemon) Authenticate(w http.ResponseWriter, r *http.Request) (bool, string, string, error) { +func (d *Daemon) Authenticate(w http.ResponseWriter, r *http.Request) (trusted bool, username string, method string, err error) { trustedCerts := d.getTrustedCertificates() // Allow internal cluster traffic by checking against the trusted certfificates. @@ -1355,7 +1355,7 @@ func (d *Daemon) init() error { // Setup OIDC authentication. if oidcIssuer != "" && oidcClientID != "" { - d.oidcVerifier, err = oidc.NewVerifier(oidcIssuer, oidcClientID, oidcAudience) + d.oidcVerifier, err = oidc.NewVerifier(oidcIssuer, oidcClientID, oidcAudience, d.serverCert, nil) if err != nil { return err }