Skip to content

Commit

Permalink
Add HMAC signing and validation (#368)
Browse files Browse the repository at this point in the history
  • Loading branch information
p53 authored Nov 4, 2023
1 parent 71155bf commit 507e534
Show file tree
Hide file tree
Showing 9 changed files with 236 additions and 0 deletions.
4 changes: 4 additions & 0 deletions pkg/apperrors/apperrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ var (
ErrMarshallDiscoveryResp = errors.New("problem marshalling discovery response")
ErrDiscoveryResponseWrite = errors.New("problem during discovery response write")

ErrHmacHeaderEmpty = errors.New("request HMAC header empty")
ErrHmacMismatch = errors.New("received HMAC header and calculated HMAC does not match")

// config errors

ErrInvalidPostLoginRedirectPath = errors.New("post login redirect path invalid, should be only path not absolute url (no hostname, scheme)")
Expand Down Expand Up @@ -137,4 +140,5 @@ var (
ErrTooManyDefaultDenyOpts = errors.New(
"only one of enable-default-deny/enable-default-deny-strict can be true",
)
ErrHmacRequiresEncKey = errors.New("enable-hmac requires encryption key")
)
1 change: 1 addition & 0 deletions pkg/constant/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const (
ContextScopeName
HeaderXForwardedFor = "X-Forwarded-For"
HeaderXRealIP = "X-Real-IP"
HeaderXHMAC = "X-HMAC-SHA256"

DurationType = "time.Duration"

Expand Down
11 changes: 11 additions & 0 deletions pkg/keycloak/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ type Config struct {
StoreURL string `env:"STORE_URL" json:"store-url" usage:"url for the storage subsystem, e.g redis://user:secret@localhost:6379/0?protocol=3, only supported is redis usig redis uri spec" yaml:"store-url"`
// EncryptionKey is the encryption key used to encrypt the refresh token
EncryptionKey string `env:"ENCRYPTION_KEY" json:"encryption-key" usage:"encryption key used to encryption the session state" yaml:"encryption-key"`
// EnableHmac enables creating hmac for forwarded requests and verifications for incoming requests
EnableHmac bool `env:"Enable_HMAC" json:"enable-hmac" usage:"enable creating hmac for forwarded requests and verification on incoming requests"`

// NoProxy it passed through all middleware but not proxy to upstream, useful when using as auth backend for forward-auth (nginx, traefik)
NoProxy bool `env:"NO_PROXY" json:"no-proxy" usage:"do not proxy requests to upstream, useful for forward-auth usage (with nginx, traefik)" yaml:"no-proxy"`
Expand Down Expand Up @@ -638,6 +640,7 @@ func (r *Config) isForwardingProxySettingsValid() error {
r.isClientIDValid,
r.isDiscoveryURLValid,
r.isForwardingGrantValid,
r.isEnableHmacValid,
func() error {
if r.TLSCertificate != "" {
return apperrors.ErrInvalidForwardTLSCertOpt
Expand Down Expand Up @@ -674,6 +677,7 @@ func (r *Config) isReverseProxySettingsValid() error {
r.isMatchClaimValid,
r.isPKCEValid,
r.isPostLoginRedirectValid,
r.isEnableHmacValid,
}

for _, validationFunc := range validationRegistry {
Expand Down Expand Up @@ -977,3 +981,10 @@ func (r *Config) isPostLoginRedirectValid() error {
}
return nil
}

func (r *Config) isEnableHmacValid() error {
if r.EnableHmac && r.EncryptionKey == "" {
return apperrors.ErrHmacRequiresEncKey
}
return nil
}
42 changes: 42 additions & 0 deletions pkg/keycloak/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2335,3 +2335,45 @@ func TestIsPostLoginRedirectValid(t *testing.T) {
)
}
}

func TestIsEnableHmacValid(t *testing.T) {
testCases := []struct {
Name string
Config *Config
Valid bool
}{
{
Name: "ValidEnableHmac",
Config: &Config{
EncryptionKey: "sdkljfalisujeoir",
EnableHmac: true,
},
Valid: true,
},
{
Name: "MissinEncryptionKey",
Config: &Config{
EncryptionKey: "",
EnableHmac: true,
},
Valid: false,
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(
testCase.Name,
func(t *testing.T) {
err := testCase.Config.isEnableHmacValid()
if err != nil && testCase.Valid {
t.Fatalf("Expected test not to fail")
}

if err == nil && !testCase.Valid {
t.Fatalf("Expected test to fail")
}
},
)
}
}
8 changes: 8 additions & 0 deletions pkg/keycloak/proxy/forwarding.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,5 +127,13 @@ func (r *OauthProxy) forwardProxyHandler() func(*http.Request, *http.Response) {
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
req.Header.Set("X-Forwarded-Agent", constant.Prog)
}

if r.Config.EnableHmac {
reqHmac, err := utils.GenerateHmac(req, r.Config.EncryptionKey)
if err != nil {
r.Log.Error(err.Error())
}
req.Header.Set(constant.HeaderXHMAC, reqHmac)
}
}
}
38 changes: 38 additions & 0 deletions pkg/keycloak/proxy/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -894,3 +894,41 @@ func (r *OauthProxy) denyMiddleware(_ http.Handler) http.Handler {
r.accessForbidden(wrt, req)
})
}

// hmacMiddleware verifies hmac
func hmacMiddleware(logger *zap.Logger, encKey string) func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(wrt http.ResponseWriter, req *http.Request) {
scope, assertOk := req.Context().Value(constant.ContextScopeName).(*RequestScope)
if !assertOk {
logger.Error(apperrors.ErrAssertionFailed.Error())
return
}

if scope.AccessDenied {
next.ServeHTTP(wrt, req)
return
}

expectedMAC := req.Header.Get(constant.HeaderXHMAC)
if expectedMAC == "" {
logger.Debug(apperrors.ErrHmacHeaderEmpty.Error())
wrt.WriteHeader(http.StatusBadRequest)
return
}

reqHmac, err := utils.GenerateHmac(req, encKey)
if err != nil {
logger.Error(err.Error())
}

if reqHmac != expectedMAC {
logger.Debug(apperrors.ErrHmacMismatch.Error())
wrt.WriteHeader(http.StatusBadRequest)
return
}

next.ServeHTTP(wrt, req)
})
}
}
4 changes: 4 additions & 0 deletions pkg/keycloak/proxy/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ func (r *OauthProxy) CreateReverseProxy() error {
engine := chi.NewRouter()
r.useDefaultStack(engine)

if r.Config.EnableHmac {
engine.Use(hmacMiddleware(r.Log, r.Config.EncryptionKey))
}

// @step: configure CORS middleware
if len(r.Config.CorsOrigins) > 0 {
corsHandler := cors.New(cors.Options{
Expand Down
101 changes: 101 additions & 0 deletions pkg/testsuite/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,107 @@ func TestSkipOpenIDProviderTLSVerifyForwardingProxy(t *testing.T) {
proxy.RunTests(t, requests)
}

func TestEnableHmacForwardingProxy(t *testing.T) {
fakeUpstream := httptest.NewServer(&FakeUpstreamService{})
encKey := "sdkljfalisujeoir"
upstreamConfig := newFakeKeycloakConfig()
upstreamConfig.EnableHmac = true
upstreamConfig.EncryptionKey = encKey
upstreamConfig.NoRedirects = true
upstreamConfig.EnableDefaultDeny = true
upstreamConfig.ClientID = ValidUsername
upstreamConfig.ClientSecret = ValidPassword
upstreamConfig.PatRetryCount = 5
upstreamConfig.PatRetryInterval = 2 * time.Second
upstreamConfig.Upstream = fakeUpstream.URL
// in newFakeProxy we are creating fakeauth server so, we will
// have two different fakeauth servers for upstream and forwarding,
// so we need to skip issuer check, but responses will be same
// so it is ok for this testing
upstreamConfig.SkipAccessTokenIssuerCheck = true

upstreamProxy := newFakeProxy(
upstreamConfig,
&fakeAuthConfig{},
)

testCases := []struct {
Name string
ProxySettings func(conf *config.Config)
ExecutionSettings []fakeRequest
}{
{
Name: "TestWithEnableHmacOnBothSides",
ProxySettings: func(conf *config.Config) {
conf.EnableForwarding = true
conf.EnableHmac = true
conf.EncryptionKey = encKey
conf.ForwardingDomains = []string{}
conf.ForwardingUsername = ValidUsername
conf.ForwardingPassword = ValidPassword
conf.ForwardingGrantType = configcore.GrantTypeUserCreds
conf.PatRetryCount = 5
conf.PatRetryInterval = 2 * time.Second
conf.OpenIDProviderTimeout = 30 * time.Second
},
ExecutionSettings: []fakeRequest{
{
URL: upstreamProxy.getServiceURL() + "/test",
ProxyRequest: true,
ExpectedProxy: true,
ExpectedCode: http.StatusOK,
ExpectedContentContains: "gambol",
},
},
},
{
Name: "TestWithDisabledHmacOnForward",
ProxySettings: func(conf *config.Config) {
conf.EnableForwarding = true
conf.EnableHmac = false
conf.EncryptionKey = encKey
conf.ForwardingDomains = []string{}
conf.ForwardingUsername = ValidUsername
conf.ForwardingPassword = ValidPassword
conf.ForwardingGrantType = configcore.GrantTypeUserCreds
conf.PatRetryCount = 5
conf.PatRetryInterval = 2 * time.Second
conf.OpenIDProviderTimeout = 30 * time.Second
},
ExecutionSettings: []fakeRequest{
{
URL: upstreamProxy.getServiceURL() + "/test",
ProxyRequest: true,
ExpectedProxy: false,
ExpectedCode: http.StatusBadRequest,
ExpectedContent: func(body string, testNum int) {
assert.Equal(t, "", body)
},
},
},
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(
testCase.Name,
func(t *testing.T) {
forwardingConfig := newFakeKeycloakConfig()
forwardingConfig.Upstream = upstreamProxy.getServiceURL()

testCase.ProxySettings(forwardingConfig)
forwardingProxy := newFakeProxy(
forwardingConfig,
&fakeAuthConfig{},
)

forwardingProxy.RunTests(t, testCase.ExecutionSettings)
},
)
}
}

func TestForbiddenTemplate(t *testing.T) {
cfg := newFakeKeycloakConfig()
cfg.ForbiddenPage = "../../templates/forbidden.html.tmpl"
Expand Down
27 changes: 27 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ package utils

import (
"bytes"
"crypto/hmac"
cryptorand "crypto/rand"
"crypto/sha256"
sha "crypto/sha512"
"crypto/tls"
"encoding/base64"
"encoding/hex"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -433,3 +436,27 @@ func GetTokenInCookie(req *http.Request, name string) (string, error) {

return token.String(), nil
}

func GenerateHmac(req *http.Request, encKey string) (string, error) {
body, err := io.ReadAll(req.Body)
if err != nil {
return "", err
}

stringToSign := fmt.Sprintf(
"%s\n%s%s\n%s;%s;%s",
req.Method,
req.URL.Path,
req.URL.RawQuery,
req.Header.Get(constant.AuthorizationHeader),
req.Host,
sha256.Sum256(body),
)

mac := hmac.New(sha256.New, []byte(encKey))
mac.Write([]byte(stringToSign))
reqHmac := mac.Sum(nil)
hexHmac := hex.EncodeToString(reqHmac)

return hexHmac, nil
}

0 comments on commit 507e534

Please sign in to comment.