From 365304a68423738785b0692ca803dca1b7814eda Mon Sep 17 00:00:00 2001 From: James Elliott Date: Wed, 2 Dec 2020 10:03:44 +1100 Subject: [PATCH] [FEATURE] Add Optional Check for Session Username on VerifyGet (#1427) * Adding the Session-Username header to the /api/verify endpoint when using cookie auth will check the value stored in the session store for the username and the header value are the same. * use strings.EqualFold to compare case insensitively * add docs * add unit tests * invalidate session if it is theoretically hijacked and log it as a warning (can only be determined if the header doesn't match the cookie) * add example PAM script * go mod tidy * go mod bump to 1.15 --- docs/features/single-factor.md | 48 +++++++++++++++ go.mod | 3 +- go.sum | 13 ---- internal/handlers/const.go | 4 ++ internal/handlers/handler_verify.go | 18 ++++++ internal/handlers/handler_verify_test.go | 78 +++++++++++++++++++++--- 6 files changed, 141 insertions(+), 23 deletions(-) diff --git a/docs/features/single-factor.md b/docs/features/single-factor.md index d0e14c36a..8d32ec3ca 100644 --- a/docs/features/single-factor.md +++ b/docs/features/single-factor.md @@ -27,3 +27,51 @@ Authelia reads credentials from the header `Proxy-Authorization` instead of the usual `Authorization` header. This is because in some circumstances both Authelia and the application could require authentication in order to provide specific authorizations at the level of the application. + + +## Session-Username header + +Authelia by default only verifies the cookie and the associated user with that cookie can +access a protected resource. The client browser does not know the username and does not send +this to Authelia, it's stored by Authelia for security reasons. + +The Session-Username header has been implemented as a means +to use Authelia with non-web services such as PAM. Basically how it works is if the +Session-Username header is sent in the request to the /api/verify endpoint it will +only respond with a sucess message if the cookie username and the header username +match. + +### Example + +These examples are for demonstration purposes only, the original use case and full instructions +are described [here](https://github.com/authelia/authelia/issues/1322#issuecomment-729519155). +You will need to adjust the FORWARDED_HOST and VERIFY_URL vars to achieve a functional result. + +#### PAM Rule + +`auth [success=1 default=ignore] pam_exec.so expose_authtok /usr/bin/pam-authelia ` + +#### PAM Script + +```bash +#!/bin/bash +# The password from stdin +PAM_PASSWORD=$(cat -) + +# url from which authelia session key was created +FORWARDED_HOST=auth.example.com + +# internal path to verify api +VERIFY_URL=http://127.0.0.1:80/api/verify + +AUTH_RESULT=$(curl -b "authelia_session=${PAM_PASSWORD}" -H "Session-Username: ${PAM_USER}" -H "X-Forwarded-Host: ${FORWARDED_HOST}" -H "X-Forwarded-Proto: https" -s -o /dev/null -I -w "%{http_code}" -L "${VERIFY_URL}") + +if [[ "$AUTH_RESULT" == 200 ]]; then + echo "Auth verify ok" + exit 0 +else + echo "Auth verify failed $AUTH_RESULT" + exit 1 +fi +``` + diff --git a/go.mod b/go.mod index 5f8d6a34d..3774e97db 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/authelia/authelia -go 1.14 +go 1.15 require ( aletheia.icu/broccoli/fs v0.0.0-20200420200651-c5ac961a357a @@ -32,5 +32,6 @@ require ( github.com/tebeka/selenium v0.9.9 github.com/tstranex/u2f v1.0.0 github.com/valyala/fasthttp v1.15.1 + golang.org/x/text v0.3.3 gopkg.in/yaml.v2 v2.3.0 ) diff --git a/go.sum b/go.sum index ebb107221..668793ed0 100644 --- a/go.sum +++ b/go.sum @@ -126,8 +126,6 @@ github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfb github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= github.com/golang/mock v1.3.1 h1:qGJ6qTW+x6xX/my+8YUVl4WNpX9B7+/l2tRsHGZ7f2s= github.com/golang/mock v1.3.1/go.mod h1:sBzyDLLjw3U8JLTeZvSv8jJB+tU5PVekmnlKIyFUx0Y= -github.com/golang/mock v1.4.3 h1:GV+pQPG/EUUbkh47niozDcADz6go/dUwhVzdUQHIVRw= -github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.4 h1:l75CXGRSwbaYNpl/Z2X1XIIAMSCquvXgpVZDhwEIJsc= github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -210,8 +208,6 @@ github.com/jackc/pgconn v0.0.0-20190831204454-2fabfa3c18b7/go.mod h1:ZJKsE/KZfsU github.com/jackc/pgconn v1.4.0/go.mod h1:Y2O3ZDF0q4mMacyWV3AstPJpeHXWGEetiFttmq5lahk= github.com/jackc/pgconn v1.5.0/go.mod h1:QeD3lBfpTFe8WUnPZWN5KY/mB8FGMIYRdd8P8Jr0fAI= github.com/jackc/pgconn v1.5.1-0.20200601181101-fa742c524853/go.mod h1:QeD3lBfpTFe8WUnPZWN5KY/mB8FGMIYRdd8P8Jr0fAI= -github.com/jackc/pgconn v1.6.3 h1:4Ks3RKvSvKPolXZsnLQTDAsokDhgID14Cv4ehECmzlY= -github.com/jackc/pgconn v1.6.3/go.mod h1:w2pne1C2tZgP+TvjqLpOigGzNqjBgQW9dUw/4Chex78= github.com/jackc/pgconn v1.6.4 h1:S7T6cx5o2OqmxdHaXLH1ZeD1SbI8jBznyYE9Ec0RCQ8= github.com/jackc/pgconn v1.6.4/go.mod h1:w2pne1C2tZgP+TvjqLpOigGzNqjBgQW9dUw/4Chex78= github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE= @@ -247,8 +243,6 @@ github.com/jackc/pgx/v4 v4.0.0-pre1.0.20190824185557-6972a5742186/go.mod h1:X+GQ github.com/jackc/pgx/v4 v4.5.0/go.mod h1:EpAKPLdnTorwmPUUsqrPxy5fphV18j9q3wrfRXgo+kA= github.com/jackc/pgx/v4 v4.6.1-0.20200510190926-94ba730bb1e9/go.mod h1:t3/cdRQl6fOLDxqtlyhe9UWgfIi9R8+8v8GKV5TRA/o= github.com/jackc/pgx/v4 v4.6.1-0.20200606145419-4e5062306904/go.mod h1:ZDaNWkt9sW1JMiNn0kdYBaLelIhw7Pg4qd+Vk6tw7Hg= -github.com/jackc/pgx/v4 v4.8.0 h1:xO3bPvwr0MJSoDfb4yeeWZIxSZ2VFBm5axPnaNEnGUQ= -github.com/jackc/pgx/v4 v4.8.0/go.mod h1:AjqYcDmEyst6GF8jJi/RF73Gla9d7/HLZzJEZj2uwpM= github.com/jackc/pgx/v4 v4.8.1 h1:SUbCLP2pXvf/Sr/25KsuI4aTxiFYIvpfk4l6aTSdyCw= github.com/jackc/pgx/v4 v4.8.1/go.mod h1:4HOLxrl8wToZJReD04/yB20GDwf4KBYETvlHciCnwW0= github.com/jackc/puddle v0.0.0-20190413234325-e4ced69a3a2b/go.mod h1:m4B5Dj62Y0fbyuIc15OsIqK0+JU8nkqQjsgx7dvjSWk= @@ -405,8 +399,6 @@ github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb6 github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE= -github.com/spf13/viper v1.7.0 h1:xVKxvI7ouOI5I+U9s2eeiUfMaWBVoXA3AWskkrqK0VM= -github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= github.com/spf13/viper v1.7.1 h1:pM5oEahlgWv/WnHXpgbKz7iLIxRf65tye2Ci+XFK5sk= github.com/spf13/viper v1.7.1/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= @@ -556,7 +548,6 @@ golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20u golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980 h1:OjiUf46hAmXblsZdnoSXsEUSKU8r1UEzcL5RVZ4gO9Y= golang.org/x/sys v0.0.0-20200602225109-6fdc65e7d980/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -669,7 +660,3 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= -rsc.io/quote/v3 v3.1.0 h1:9JKUTTIUgS6kzR9mK1YuGKv6Nl+DijDNIc0ghT58FaY= -rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= -rsc.io/sampler v1.3.0 h1:7uVkIFmeBqHfdjD+gZwtXXI+RODJ2Wc4O7MPEh/QiW4= -rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= diff --git a/internal/handlers/const.go b/internal/handlers/const.go index 370de9f65..03395bf0e 100644 --- a/internal/handlers/const.go +++ b/internal/handlers/const.go @@ -13,6 +13,10 @@ const authPrefix = "Basic " // AuthorizationHeader is the basic-auth HTTP header Authelia utilises. const AuthorizationHeader = "Proxy-Authorization" + +// SessionUsernameHeader is used as additional protection to validate a user for things like pam_exec. +const SessionUsernameHeader = "Session-Username" + const remoteUserHeader = "Remote-User" const remoteNameHeader = "Remote-Name" const remoteEmailHeader = "Remote-Email" diff --git a/internal/handlers/handler_verify.go b/internal/handlers/handler_verify.go index 407469b5b..65c064ce1 100644 --- a/internal/handlers/handler_verify.go +++ b/internal/handlers/handler_verify.go @@ -431,6 +431,24 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques } else { username, name, groups, emails, authLevel, err = verifySessionCookie(ctx, targetURL, &userSession, refreshProfile, refreshProfileInterval) + + sessionUsername := ctx.Request.Header.Peek(SessionUsernameHeader) + if sessionUsername != nil && !strings.EqualFold(string(sessionUsername), username) { + ctx.Logger.Warnf( + "Could not match user %s to their %s header with a value of %s when visiting %s, possible cookie hijack or attempt to bypass security detected destroying the session and sending 401 response", + username, SessionUsernameHeader, sessionUsername, targetURL.String()) + + err := ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx) + if err != nil { + ctx.Logger.Error( + fmt.Errorf( + "Unable to destroy user session after handler could not match them to their %s header: %s", + SessionUsernameHeader, err)) + } + + ctx.ReplyUnauthorized() + return + } } if err != nil { diff --git a/internal/handlers/handler_verify_test.go b/internal/handlers/handler_verify_test.go index 88e9a0d56..e570fb57d 100644 --- a/internal/handlers/handler_verify_test.go +++ b/internal/handlers/handler_verify_test.go @@ -421,7 +421,9 @@ func TestShouldNotCrashOnEmptyEmail(t *testing.T) { userSession.Username = testUsername userSession.Emails = nil userSession.AuthenticationLevel = authentication.OneFactor - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://bypass.example.com") @@ -477,7 +479,9 @@ func TestShouldVerifyAuthorizationsUsingSessionCookie(t *testing.T) { userSession.Username = testCase.Username userSession.Emails = testCase.Emails userSession.AuthenticationLevel = testCase.AuthenticationLevel - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", testCase.URL) @@ -514,7 +518,9 @@ func TestShouldDestroySessionWhenInactiveForTooLong(t *testing.T) { userSession.Username = testUsername userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = past.Unix() - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") @@ -545,7 +551,9 @@ func TestShouldDestroySessionWhenInactiveForTooLongUsingDurationNotation(t *test userSession.Username = testUsername userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = clock.Now().Add(-1 * time.Hour).Unix() - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") @@ -572,7 +580,9 @@ func TestShouldKeepSessionWhenUserCheckedRememberMeAndIsInactiveForTooLong(t *te userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = 0 userSession.KeepMeLoggedIn = true - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") @@ -603,7 +613,9 @@ func TestShouldKeepSessionWhenInactivityTimeoutHasNotBeenExceeded(t *testing.T) userSession.Emails = []string{"john.doe@example.com"} userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = past.Unix() - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") @@ -638,7 +650,9 @@ func TestShouldRedirectWhenSessionInactiveForTooLongAndRDParamProvided(t *testin userSession.Username = testUsername userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = past.Unix() - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.QueryArgs().Add("rd", "https://login.example.com") mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") @@ -669,7 +683,9 @@ func TestShouldUpdateInactivityTimestampEvenWhenHittingForbiddenResources(t *tes userSession.Username = testUsername userSession.AuthenticationLevel = authentication.TwoFactor userSession.LastActivity = past.Unix() - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://deny.example.com") @@ -690,7 +706,9 @@ func TestShouldURLEncodeRedirectionURLParameter(t *testing.T) { userSession := mock.Ctx.GetSession() userSession.Username = testUsername userSession.AuthenticationLevel = authentication.NotAuthenticated - mock.Ctx.SaveSession(userSession) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting. + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com") mock.Ctx.Request.SetHost("mydomain.com") @@ -1022,3 +1040,45 @@ func TestShouldGetAddedUserGroupsFromBackend(t *testing.T) { assert.Equal(t, "users", userSession.Groups[1]) assert.Equal(t, "grafana", userSession.Groups[2]) } + +func TestShouldCheckValidSessionUsernameHeaderAndReturn200(t *testing.T) { + mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + expectedStatusCode := 200 + + userSession := mock.Ctx.GetSession() + userSession.Username = testUsername + userSession.AuthenticationLevel = authentication.OneFactor + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) + + mock.Ctx.Request.Header.Set("X-Original-URL", "https://one-factor.example.com") + mock.Ctx.Request.Header.Set(SessionUsernameHeader, testUsername) + VerifyGet(verifyGetCfg)(mock.Ctx) + + assert.Equal(t, expectedStatusCode, mock.Ctx.Response.StatusCode()) + assert.Equal(t, "", string(mock.Ctx.Response.Body())) +} + +func TestShouldCheckInvalidSessionUsernameHeaderAndReturn401(t *testing.T) { + mock := mocks.NewMockAutheliaCtx(t) + defer mock.Close() + + expectedStatusCode := 401 + + userSession := mock.Ctx.GetSession() + userSession.Username = testUsername + userSession.AuthenticationLevel = authentication.OneFactor + + err := mock.Ctx.SaveSession(userSession) + require.NoError(t, err) + + mock.Ctx.Request.Header.Set("X-Original-URL", "https://one-factor.example.com") + mock.Ctx.Request.Header.Set(SessionUsernameHeader, "root") + VerifyGet(verifyGetCfg)(mock.Ctx) + + assert.Equal(t, expectedStatusCode, mock.Ctx.Response.StatusCode()) + assert.Equal(t, "Unauthorized", string(mock.Ctx.Response.Body())) +}