diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cdfbc872..8e8038f33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ when the IDP uses a self-signed CA. Merged from https://github.com/vmware-archiv Adds the `customAssetsDir` config option to override the contents of /assets/ for use in custom templates. + +### Override hard-coded session salt + +Adds the `sessionSalt` config option to override the hard-coded salt. Min. length is 8 characters. +Fixes https://github.com/vmware-archive/gangway/issues/71 + ### todo ... diff --git a/cmd/gangway/handlers_test.go b/cmd/gangway/handlers_test.go index 44672b7e4..0fa99737f 100644 --- a/cmd/gangway/handlers_test.go +++ b/cmd/gangway/handlers_test.go @@ -36,7 +36,7 @@ import ( ) func testInit() { - gangwayUserSession = session.New("test") + gangwayUserSession = session.New("test", "0123456789") transportConfig = config.NewTransportConfig("") oauth2Cfg = &oauth2.Config{ @@ -434,7 +434,7 @@ func TestUnauthedCommandlineHandlerRedirect(t *testing.T) { t.Fatal(err) } - session.New("test") + session.New("test", "0123456789") rr := httptest.NewRecorder() handler := http.HandlerFunc(commandlineHandler) diff --git a/cmd/gangway/main.go b/cmd/gangway/main.go index ea6ebcd17..9352b1d3a 100644 --- a/cmd/gangway/main.go +++ b/cmd/gangway/main.go @@ -86,7 +86,7 @@ func main() { } transportConfig = config.NewTransportConfig(cfg.TrustedCAPath) - gangwayUserSession = session.New(cfg.SessionSecurityKey) + gangwayUserSession = session.New(cfg.SessionSecurityKey, cfg.SessionSalt) var assetFs http.FileSystem if cfg.CustomAssetsDir != "" { diff --git a/docs/README.md b/docs/README.md index 4da5f4821..5a6ad1d7a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,14 +16,17 @@ While this is called a secret, based on the way that OAuth2 works with command l This will be divulged to any client that is configured through gangway. As such, it is probably acceptable to keep that secret in the config file and not worry about managing it as a true secret. -We also have a secret string that is used to as a way to encrypt the cookies that are returned to the users. -If using the example YAML, create a secret to hold this value with the following command line: +We also have a secret string and salt that is used to as a way to encrypt the cookies that are returned +to the users. If using the example YAML, create a secret to hold these values with the following command line: ``` kubectl -n gangway create secret generic gangway-key \ - --from-literal=sessionkey=$(openssl rand -base64 32) + --from-literal=sessionkey=$(openssl rand -base64 32) \ + --from-literal=sessionsalt=$(openssl rand -base64 32) ``` +Setting a salt is optional; if the config variable is not set a default baked in salt will be used. + ## Path Prefix Gangway takes an optional path prefix if you want to host it at a url other than '/' (e.g. `https://example.com/gangway`). diff --git a/docs/yaml/03-deployment.yaml b/docs/yaml/03-deployment.yaml index 3c2ad7ae5..a2d0380a4 100644 --- a/docs/yaml/03-deployment.yaml +++ b/docs/yaml/03-deployment.yaml @@ -28,6 +28,11 @@ spec: secretKeyRef: name: gangway-key key: sessionkey + - name: GANGWAY_SESSION_SALT + valueFrom: + secretKeyRef: + name: gangway-key + key: sessionsalt ports: - name: http containerPort: 8080 diff --git a/internal/config/config.go b/internal/config/config.go index ae58e19cf..28d55c352 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,6 +23,8 @@ import ( "sigs.k8s.io/yaml" ) +const hardCodedDefaultSalt = "MkmfuPNHnZBBivy0L0aW" + // Config the configuration field for gangway type Config struct { Host string `yaml:"host"` @@ -48,6 +50,7 @@ type Config struct { HTTPPath string `yaml:"httpPath" envconfig:"http_path"` ShowClaims bool `yaml:"showClaims" envconfig:"show_claims"` SessionSecurityKey string `yaml:"sessionSecurityKey" envconfig:"SESSION_SECURITY_KEY"` + SessionSalt string `yaml:"sessionSalt" envconfig:"SESSION_SALT"` CustomHTMLTemplatesDir string `yaml:"customHTMLTemplatesDir" envconfig:"custom_html_templates_dir"` CustomAssetsDir string `yaml:"customAssetsDir" envconfig:"custom_assets_dir"` } @@ -68,6 +71,7 @@ func NewConfig(configFile string) (*Config, error) { ClusterCAPath: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt", HTTPPath: "", ShowClaims: false, + SessionSalt: hardCodedDefaultSalt, } if configFile != "" { @@ -111,6 +115,7 @@ func (cfg *Config) Validate() error { {cfg.RedirectURL == "", "no redirectURL specified"}, {cfg.SessionSecurityKey == "", "no SessionSecurityKey specified"}, {cfg.APIServerURL == "", "no apiServerURL specified"}, + {len(cfg.SessionSalt) < 8, "salt needs to be min. 8 characters"}, } for _, check := range checks { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c1bc4b9c0..520983073 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -27,6 +27,7 @@ func TestConfigNotFound(t *testing.T) { } func TestEnvionmentOverrides(t *testing.T) { + salt := "randombanana" os.Setenv("GANGWAY_AUTHORIZE_URL", "https://foo.bar/authorize") os.Setenv("GANGWAY_APISERVER_URL", "https://k8s-api.foo.baz") os.Setenv("GANGWAY_CLIENT_ID", "foo") @@ -39,6 +40,7 @@ func TestEnvionmentOverrides(t *testing.T) { os.Setenv("GANGWAY_AUDIENCE", "foo") os.Setenv("GANGWAY_SCOPES", "groups,sub") os.Setenv("GANGWAY_SHOW_CLAIMS", "false") + os.Setenv("GANGWAY_SESSION_SALT", salt) cfg, err := NewConfig("") if err != nil { t.Errorf("Failed to test config overrides with error: %s", err) @@ -59,6 +61,28 @@ func TestEnvionmentOverrides(t *testing.T) { if cfg.ShowClaims != false { t.Errorf("Failed to disable showing of claims. Expected %t but got %t", false, cfg.ShowClaims) } + if cfg.SessionSalt != salt { + t.Errorf("Failed to override session salt. Expected %s but got %s", salt, cfg.SessionSalt) + } +} + +func TestSessionSaltLength(t *testing.T) { + salt := "2short" + os.Setenv("GANGWAY_AUTHORIZE_URL", "https://foo.bar/authorize") + os.Setenv("GANGWAY_APISERVER_URL", "https://k8s-api.foo.baz") + os.Setenv("GANGWAY_CLIENT_ID", "foo") + os.Setenv("GANGWAY_CLIENT_SECRET", "bar") + os.Setenv("GANGWAY_REDIRECT_URL", "https://foo.baz/callback") + os.Setenv("GANGWAY_SESSION_SECURITY_KEY", "testing") + os.Setenv("GANGWAY_TOKEN_URL", "https://foo.bar/token") + os.Setenv("GANGWAY_SESSION_SALT", salt) + _, err := NewConfig("") + if err == nil { + t.Errorf("Expected error but got none") + } + if err.Error() != "invalid config: salt needs to be min. 8 characters" { + t.Errorf("Wrong error. Expected %v but got %v", "salt needs to be min. 8 characters", err) + } } func TestGetRootPathPrefix(t *testing.T) { diff --git a/internal/session/session.go b/internal/session/session.go index fdd8e68a3..f2f16b7ed 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -22,22 +22,20 @@ import ( "golang.org/x/crypto/pbkdf2" ) -const salt = "MkmfuPNHnZBBivy0L0aW" - // Session defines a Gangway session type Session struct { Session *CustomCookieStore } // New inits a Session with CookieStore -func New(sessionSecurityKey string) *Session { +func New(sessionSecurityKey string, sessionSalt string) *Session { return &Session{ - Session: NewCustomCookieStore(generateSessionKeys(sessionSecurityKey)), + Session: NewCustomCookieStore(generateSessionKeys(sessionSecurityKey, sessionSalt)), } } // generateSessionKeys creates a signed encryption key for the cookie store -func generateSessionKeys(sessionSecurityKey string) ([]byte, []byte) { +func generateSessionKeys(sessionSecurityKey string, salt string) ([]byte, []byte) { // Take the configured security key and generate 96 bytes of data. This is // used as the signing and encryption keys for the cookie store. For details // on the PBKDF2 function: https://en.wikipedia.org/wiki/PBKDF2 diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 566fc4e53..09ad75d5e 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -24,7 +24,7 @@ import ( ) func TestGenerateSessionKeys(t *testing.T) { - b1, b2 := generateSessionKeys("testing") + b1, b2 := generateSessionKeys("testing", "0123456789") if len(b1) != 64 || len(b2) != 32 { t.Errorf("Wrong byte length's returned") @@ -33,7 +33,7 @@ func TestGenerateSessionKeys(t *testing.T) { } func TestInitSessionStore(t *testing.T) { - s := New("testing") + s := New("testing", "0123456789") if s.Session == nil { t.Errorf("Session Store is nil. Did not get initialized") return @@ -42,7 +42,7 @@ func TestInitSessionStore(t *testing.T) { } func TestCleanupSession(t *testing.T) { - s := New("testing") + s := New("testing", "0123456789") session := &sessions.Session{} // create a test http server ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {