diff --git a/operatorapi/login.go b/operatorapi/login.go index 16ac0c8db..264b7f921 100644 --- a/operatorapi/login.go +++ b/operatorapi/login.go @@ -111,7 +111,10 @@ func getLoginDetailsResponse(params authApi.LoginDetailParams) (*models.LoginDet return nil, restapi.ErrorWithContext(ctx, err) } // Validate user against IDP - identityProvider := &auth.IdentityProvider{Client: oauth2Client} + identityProvider := &auth.IdentityProvider{ + KeyFunc: oauth2.DefaultDerivedKey, + Client: oauth2Client, + } redirectURL = append(redirectURL, identityProvider.GenerateLoginURL()) } @@ -146,7 +149,10 @@ func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams) (*models.L return nil, restapi.ErrorWithContext(ctx, err) } // initialize new identity provider - identityProvider := auth.IdentityProvider{Client: oauth2Client} + identityProvider := auth.IdentityProvider{ + KeyFunc: oauth2.DefaultDerivedKey, + Client: oauth2Client, + } // Validate user against IDP _, err = verifyUserAgainstIDP(ctx, identityProvider, *lr.Code, *lr.State) if err != nil { diff --git a/pkg/auth/idp.go b/pkg/auth/idp.go index 4146deed3..cc47c407d 100644 --- a/pkg/auth/idp.go +++ b/pkg/auth/idp.go @@ -38,20 +38,21 @@ type IdentityProviderI interface { // Define the structure of a IdentityProvider with Client inside and define the functions that are used // during the authentication flow. type IdentityProvider struct { - Client *oauth2.Provider + KeyFunc oauth2.StateKeyFunc + Client *oauth2.Provider } // VerifyIdentity will verify the user identity against the idp using the authorization code flow func (c IdentityProvider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { - return c.Client.VerifyIdentity(ctx, code, state) + return c.Client.VerifyIdentity(ctx, code, state, c.KeyFunc) } // VerifyIdentityForOperator will verify the user identity against the idp using the authorization code flow func (c IdentityProvider) VerifyIdentityForOperator(ctx context.Context, code, state string) (*xoauth2.Token, error) { - return c.Client.VerifyIdentityForOperator(ctx, code, state) + return c.Client.VerifyIdentityForOperator(ctx, code, state, c.KeyFunc) } // GenerateLoginURL returns a new URL used by the user to login against the idp func (c IdentityProvider) GenerateLoginURL() string { - return c.Client.GenerateLoginURL() + return c.Client.GenerateLoginURL(c.KeyFunc) } diff --git a/pkg/auth/idp/oauth2/config.go b/pkg/auth/idp/oauth2/config.go index b8920a643..8dbe01aa7 100644 --- a/pkg/auth/idp/oauth2/config.go +++ b/pkg/auth/idp/oauth2/config.go @@ -19,10 +19,12 @@ package oauth2 import ( + "crypto/sha1" "strings" "github.com/minio/console/pkg/auth/utils" "github.com/minio/pkg/env" + "golang.org/x/crypto/pbkdf2" ) // ProviderConfig - OpenID IDP Configuration for console. @@ -37,6 +39,14 @@ type ProviderConfig struct { RedirectCallback string } +// GetStateKeyFunc - return the key function used to generate the authorization +// code flow state parameter. +func (pc ProviderConfig) GetStateKeyFunc() StateKeyFunc { + return func() []byte { + return pbkdf2.Key([]byte(pc.HMACPassphrase), []byte(pc.HMACSalt), 4096, 32, sha1.New) + } +} + type OpenIDPCfg map[string]ProviderConfig var DefaultIDPConfig = "_" diff --git a/pkg/auth/idp/oauth2/provider.go b/pkg/auth/idp/oauth2/provider.go index 1689e49c7..d41767a89 100644 --- a/pkg/auth/idp/oauth2/provider.go +++ b/pkg/auth/idp/oauth2/provider.go @@ -114,9 +114,9 @@ type Provider struct { provHTTPClient *http.Client } -// derivedKey is the key used to compute the HMAC for signing the oauth state parameter +// DefaultDerivedKey is the key used to compute the HMAC for signing the oauth state parameter // its derived using pbkdf on CONSOLE_IDP_HMAC_PASSPHRASE with CONSOLE_IDP_HMAC_SALT -var derivedKey = func() []byte { +var DefaultDerivedKey = func() []byte { return pbkdf2.Key([]byte(getPassphraseForIDPHmac()), []byte(getSaltForIDPHmac()), 4096, 32, sha1.New) } @@ -304,11 +304,15 @@ type User struct { Username string `json:"username"` } +// StateKeyFunc - is a function that returns a key used in OAuth Authorization +// flow state generation and verification. +type StateKeyFunc func() []byte + // VerifyIdentity will contact the configured IDP to the user identity based on the authorization code and state // if the user is valid, then it will contact MinIO to get valid sts credentials based on the identity provided by the IDP -func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) (*credentials.Credentials, error) { +func (client *Provider) VerifyIdentity(ctx context.Context, code, state string, keyFunc StateKeyFunc) (*credentials.Credentials, error) { // verify the provided state is valid (prevents CSRF attacks) - if err := validateOauth2State(state); err != nil { + if err := validateOauth2State(state, keyFunc); err != nil { return nil, err } getWebTokenExpiry := func() (*credentials.WebIdentityToken, error) { @@ -357,9 +361,9 @@ func (client *Provider) VerifyIdentity(ctx context.Context, code, state string) } // VerifyIdentityForOperator will contact the configured IDP and validate the user identity based on the authorization code and state -func (client *Provider) VerifyIdentityForOperator(ctx context.Context, code, state string) (*xoauth2.Token, error) { +func (client *Provider) VerifyIdentityForOperator(ctx context.Context, code, state string, keyFunc StateKeyFunc) (*xoauth2.Token, error) { // verify the provided state is valid (prevents CSRF attacks) - if err := validateOauth2State(state); err != nil { + if err := validateOauth2State(state, keyFunc); err != nil { return nil, err } customCtx := context.WithValue(ctx, oauth2.HTTPClient, client.provHTTPClient) @@ -376,7 +380,7 @@ func (client *Provider) VerifyIdentityForOperator(ctx context.Context, code, sta // validateOauth2State validates the provided state was originated using the same // instance (or one configured using the same secrets) of Console, this is basically used to prevent CSRF attacks // https://security.stackexchange.com/questions/20187/oauth2-cross-site-request-forgery-and-state-parameter -func validateOauth2State(state string) error { +func validateOauth2State(state string, keyFunc StateKeyFunc) error { // state contains a base64 encoded string that may ends with "==", the browser encodes that to "%3D%3D" // query unescape is need it before trying to decode the base64 string encodedMessage, err := url.QueryUnescape(state) @@ -396,7 +400,7 @@ func validateOauth2State(state string) error { // extract the state and hmac incomingState, incomingHmac := s[0], s[1] // validate that hmac(incomingState + pbkdf2(secret, salt)) == incomingHmac - if calculatedHmac := utils.ComputeHmac256(incomingState, derivedKey()); calculatedHmac != incomingHmac { + if calculatedHmac := utils.ComputeHmac256(incomingState, keyFunc()); calculatedHmac != incomingHmac { return fmt.Errorf("oauth2 state is invalid, expected %s, got %s", calculatedHmac, incomingHmac) } return nil @@ -429,16 +433,16 @@ func parseDiscoveryDoc(ustr string, httpClient *http.Client) (DiscoveryDoc, erro } // GetRandomStateWithHMAC computes message + hmac(message, pbkdf2(key, salt)) to be used as state during the oauth authorization -func GetRandomStateWithHMAC(length int) string { +func GetRandomStateWithHMAC(length int, keyFunc StateKeyFunc) string { state := utils.RandomCharString(length) - hmac := utils.ComputeHmac256(state, derivedKey()) + hmac := utils.ComputeHmac256(state, keyFunc()) return base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", state, hmac))) } // GenerateLoginURL returns a new login URL based on the configured IDP -func (client *Provider) GenerateLoginURL() string { +func (client *Provider) GenerateLoginURL(keyFunc StateKeyFunc) string { // generates random state and sign it using HMAC256 - state := GetRandomStateWithHMAC(25) + state := GetRandomStateWithHMAC(25, keyFunc) loginURL := client.oauth2Config.AuthCodeURL(state) return strings.TrimSpace(loginURL) } diff --git a/pkg/auth/idp/oauth2/provider_test.go b/pkg/auth/idp/oauth2/provider_test.go index a13405e62..db7f3d1c8 100644 --- a/pkg/auth/idp/oauth2/provider_test.go +++ b/pkg/auth/idp/oauth2/provider_test.go @@ -66,6 +66,6 @@ func TestGenerateLoginURL(t *testing.T) { // a non-empty string return state } - url := oauth2Provider.GenerateLoginURL() + url := oauth2Provider.GenerateLoginURL(DefaultDerivedKey) funcAssert.NotEqual("", url) } diff --git a/restapi/user_login.go b/restapi/user_login.go index dadf1e55a..7b0a9092f 100644 --- a/restapi/user_login.go +++ b/restapi/user_login.go @@ -162,7 +162,10 @@ func getLoginDetailsResponse(params authApi.LoginDetailParams, openIDProviders o return nil, ErrorWithContext(ctx, err, ErrOauth2Provider) } // Validate user against IDP - identityProvider := &auth.IdentityProvider{Client: oauth2Client} + identityProvider := &auth.IdentityProvider{ + KeyFunc: provider.GetStateKeyFunc(), + Client: oauth2Client, + } redirectURL = append(redirectURL, identityProvider.GenerateLoginURL()) if provider.DisplayName != "" { displayNames = append(displayNames, provider.DisplayName) @@ -201,7 +204,10 @@ func getLoginOauth2AuthResponse(params authApi.LoginOauth2AuthParams, openIDProv return nil, ErrorWithContext(ctx, err) } // initialize new identity provider - identityProvider := auth.IdentityProvider{Client: oauth2Client} + identityProvider := auth.IdentityProvider{ + KeyFunc: openIDProviders[idpName].GetStateKeyFunc(), + Client: oauth2Client, + } // Validate user against IDP userCredentials, err := verifyUserAgainstIDP(ctx, identityProvider, *lr.Code, *lr.State) if err != nil {