Configure oauth-proxy with random cookie secret

The oauth-proxy uses the cookie secret as a key to sign and validate
session cookies. This change generates a random 32 byte key and stores
it in the skupper-console-session secret to be used by the proxy for
this purpose.

Signed-off-by: Christian Kruse <[email protected]>

Improve error handling per review

Signed-off-by: Christian Kruse <[email protected]>

Author: c-kruse

api/types/types.go

@@ -215,6 +215,7 @@ const (
 	SiteCaSecret             string = "skupper-site-ca"
 	ConsoleServerSecret      string = "skupper-console-certs"
 	ConsoleUsersSecret       string = "skupper-console-users"
+	ConsoleSessionSecret     string = "skupper-console-session"
 	PrometheusServerSecret   string = "skupper-prometheus-certs"
 	OauthRouterConsoleSecret string = "skupper-router-console-certs"
 	ServiceCaSecret          string = "skupper-service-ca"

client/router_create.go

@@ -52,7 +52,7 @@ func OauthProxyContainer(serviceAccount string, servicePort string) *corev1.Cont
 			"--upstream=http://localhost:" + servicePort,
 			"--tls-cert=/etc/tls/proxy-certs/tls.crt",
 			"--tls-key=/etc/tls/proxy-certs/tls.key",
-			"--cookie-secret=SECRET",
+			"--cookie-secret-file=/etc/skupper-console-session/session_secret",
 		},
 		Ports: []corev1.ContainerPort{
 			{
@@ -335,6 +335,7 @@ func (cli *VanClient) GetVanControllerSpec(options types.SiteConfigSpec, van *ty
 			envVars = append(envVars, corev1.EnvVar{Name: "FLOW_HOST", Value: "localhost"})
 			mounts = append(mounts, []corev1.VolumeMount{})
 			kube.AppendSecretVolume(&volumes, &mounts[oauthProxy], types.ConsoleServerSecret, "/etc/tls/proxy-certs/")
+			kube.AppendSecretVolume(&volumes, &mounts[oauthProxy], types.ConsoleSessionSecret, "/etc/skupper-console-session/")
 		} else if options.AuthMode == string(types.ConsoleAuthModeInternal) {
 			envVars = append(envVars, corev1.EnvVar{Name: "FLOW_USERS", Value: "/etc/console-users"})
 			kube.AppendSharedSecretVolume(&volumes, []*[]corev1.VolumeMount{&mounts[serviceController], &mounts[flowCollector]}, "skupper-console-users", "/etc/console-users/")
@@ -710,7 +711,7 @@ func configureDeployment(spec *types.DeploymentSpec, options *types.Tuning) erro
 	}
 }
 
-func (cli *VanClient) GetRouterSpecFromOpts(options types.SiteConfigSpec, siteId string) *types.RouterSpec {
+func (cli *VanClient) GetRouterSpecFromOpts(options types.SiteConfigSpec, siteId string) (*types.RouterSpec, error) {
 	// skupper-router container index
 	// TODO: update after dataplance changes
 	const (
@@ -977,6 +978,15 @@ func (cli *VanClient) GetRouterSpecFromOpts(options types.SiteConfigSpec, siteId
 			Labels:      options.Labels,
 		})
 	}
+
+	if options.EnableFlowCollector && options.AuthMode == string(types.ConsoleAuthModeOpenshift) {
+		sessionCreds, err := kube.GenerateConsoleSessionCredentials(nil)
+		if err != nil {
+			return van, fmt.Errorf("failed to generate console session credentials: %s", err)
+		}
+		credentials = append(credentials, sessionCreds)
+	}
+
 	if options.AuthMode == string(types.ConsoleAuthModeInternal) && (options.EnableFlowCollector || options.EnableRestAPI) {
 		userData := map[string][]byte{}
 		if options.User != "" {
@@ -1162,7 +1172,7 @@ func (cli *VanClient) GetRouterSpecFromOpts(options types.SiteConfigSpec, siteId
 	}
 	van.Transport.Routes = routes
 
-	return van
+	return van, nil
 }
 
 func (cli *VanClient) GetRouterHostAliasesSpecFromTokens(ctx context.Context, namespace string) ([]corev1.HostAlias, error) {
@@ -1252,13 +1262,15 @@ func (cli *VanClient) RouterCreate(ctx context.Context, options types.SiteConfig
 	if siteId == "" {
 		siteId = utils.RandomId(10)
 	}
-	van := cli.GetRouterSpecFromOpts(options.Spec, siteId)
+	van, err := cli.GetRouterSpecFromOpts(options.Spec, siteId)
+	if err != nil {
+		return err
+	}
 	siteOwnerRef := asOwnerReference(options.Reference)
 	var ownerRefs []metav1.OwnerReference
 	if siteOwnerRef != nil {
 		ownerRefs = []metav1.OwnerReference{*siteOwnerRef}
 	}
-	var err error
 	hostAliases, err := cli.GetRouterHostAliasesSpecFromTokens(ctx, cli.GetNamespace())
 	if err != nil {
 		return err

client/router_create_test.go

@@ -195,6 +195,7 @@ func TestRouterCreateDefaults(t *testing.T) {
 				types.LocalClientSecret,
 				types.SiteServerSecret,
 				types.ConsoleServerSecret,
+				types.ConsoleSessionSecret,
 				types.ServiceCaSecret,
 				types.ServiceClientSecret},
 			svcsExpected:        []string{types.LocalTransportServiceName, types.TransportServiceName, types.ControllerServiceName, types.PrometheusServiceName},

client/router_update.go

@@ -104,6 +104,7 @@ func (cli *VanClient) RouterUpdateVersionInNamespace(ctx context.Context, hup bo
 	addPrometheusServer := false
 	moveClaims := false
 	updateRole := false
+	updateOauthProxy := false
 	inprogress, originalVersion, err := cli.isUpdating(namespace)
 	if err != nil {
 		return false, err
@@ -121,6 +122,7 @@ func (cli *VanClient) RouterUpdateVersionInNamespace(ctx context.Context, hup bo
 			moveClaims = !config.IsEdge()
 			updateRole = true //config-sync requires extra permission to write skupper-network-status configmap
 		}
+		updateOauthProxy = utils.LessRecentThanVersion(originalVersion, "1.7.2")
 	} else {
 		originalVersion = site.Version
 	}
@@ -148,6 +150,7 @@ func (cli *VanClient) RouterUpdateVersionInNamespace(ctx context.Context, hup bo
 				moveClaims = !config.IsEdge()
 				updateRole = true //config-sync requires extra permission to write skupper-network-status configmap
 			}
+			updateOauthProxy = utils.LessRecentThanVersion(originalVersion, "1.7.2")
 
 			err = cli.updateStarted(site.Version, namespace, configmap.ObjectMeta.OwnerReferences)
 			if err != nil {
@@ -666,6 +669,25 @@ func (cli *VanClient) RouterUpdateVersionInNamespace(ctx context.Context, hup bo
 		}
 	}
 
+	if updateOauthProxy {
+		siteConfig, _ := cli.SiteConfigInspectInNamespace(ctx, nil, namespace)
+		if siteConfig.Spec.EnableFlowCollector &&
+			siteConfig.Spec.EnableConsole &&
+			siteConfig.Spec.AuthMode == string(types.ConsoleAuthModeOpenshift) {
+			var owner *metav1.OwnerReference
+			if len(configmap.ObjectMeta.OwnerReferences) > 0 {
+				owner = &configmap.ObjectMeta.OwnerReferences[0]
+			}
+			changed, err := ensureOauthProxyConfig(cli, owner, controller)
+			if err != nil {
+				return false, err
+			}
+			if changed {
+				updateController = true
+			}
+		}
+	}
+
 	desiredControllerImage := images.GetServiceControllerImageName()
 	if controller.Spec.Template.Spec.Containers[0].Image != desiredControllerImage {
 		controller.Spec.Template.Spec.Containers[0].Image = desiredControllerImage
@@ -1473,3 +1495,78 @@ func createFlowCollectorSidecar(ctx context.Context, cli *VanClient, controller
 	controller.Spec.Template.Spec.Containers = append(controller.Spec.Template.Spec.Containers, flowContainer)
 	return nil
 }
+
+func ensureOauthProxyConfig(cli *VanClient, owner *metav1.OwnerReference, controller *appsv1.Deployment) (bool, error) {
+	var edited bool
+	// ensure the console session credentials are present
+	sessionCreds, err := kube.GenerateConsoleSessionCredentials(nil)
+	if err != nil {
+		return false, err
+	}
+	edited = true
+	_, err = kube.NewSecret(sessionCreds, owner, cli.Namespace, cli.KubeClient)
+	if err != nil {
+		if !errors.IsAlreadyExists(err) { // ignore already exists errors
+			return false, fmt.Errorf("error creating skupper-console-session secret: %s", err)
+		}
+		edited = false
+	}
+
+	// ensure oauth-proxy container spec is updated
+	idx := -1
+	for i, c := range controller.Spec.Template.Spec.Containers {
+		if c.Name == "oauth-proxy" {
+			idx = i
+			break
+		}
+	}
+	if idx < 0 {
+		return edited, fmt.Errorf("error updating oauth-proxy spec: container not found")
+	}
+	desiredCtr := OauthProxyContainer(types.ControllerServiceAccountName, fmt.Sprint(types.ConsoleOpenShiftServicePort))
+	actualCtr := controller.Spec.Template.Spec.Containers[idx]
+	if !reflect.DeepEqual(actualCtr.Args, desiredCtr.Args) {
+		edited = true
+		actualCtr.Args = desiredCtr.Args
+	}
+
+	// ensure console session secret is mounted as volume
+	volIdx := -1
+	for i, v := range controller.Spec.Template.Spec.Volumes {
+		if v.Name == types.ConsoleSessionSecret {
+			volIdx = i
+			break
+		}
+	}
+	if volIdx < 0 {
+		edited = true
+		controller.Spec.Template.Spec.Volumes = append(controller.Spec.Template.Spec.Volumes, corev1.Volume{
+			Name: types.ConsoleSessionSecret,
+			VolumeSource: corev1.VolumeSource{
+				Secret: &corev1.SecretVolumeSource{
+					SecretName: types.ConsoleSessionSecret,
+				},
+			},
+		})
+	}
+
+	mtIdx := -1
+	for i, m := range actualCtr.VolumeMounts {
+		if m.Name == types.ConsoleSessionSecret {
+			mtIdx = i
+			break
+		}
+	}
+	if mtIdx < 0 {
+		edited = true
+		actualCtr.VolumeMounts = append(actualCtr.VolumeMounts, corev1.VolumeMount{
+			Name:      types.ConsoleSessionSecret,
+			MountPath: "/etc/skupper-console-session/",
+		})
+	}
+
+	if edited {
+		controller.Spec.Template.Spec.Containers[idx] = actualCtr
+	}
+	return edited, nil
+}

pkg/kube/secrets.go

@@ -2,7 +2,10 @@ package kube
 
 import (
 	"context"
+	"crypto/rand"
+	"encoding/base64"
 	"fmt"
+	"io"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -161,3 +164,28 @@ func RegenerateCredentials(credential types.Credential, namespace string, ca *co
 	current.Data = regenerated.Data
 	return cli.CoreV1().Secrets(namespace).Update(context.TODO(), current, metav1.UpdateOptions{})
 }
+
+func GenerateConsoleSessionCredentials(source io.Reader) (types.Credential, error) {
+	var (
+		key     [32]byte // key is 32 random bytes
+		keyText [44]byte // keyText is the base64 encoded key - 44 characters
+		cred    types.Credential
+	)
+	if source == nil {
+		source = rand.Reader
+	}
+	if _, err := io.ReadFull(source, key[:]); err != nil {
+		return cred, fmt.Errorf("error generating console session key: %s", err)
+	}
+	base64.URLEncoding.Encode(keyText[:], key[:])
+	return types.Credential{
+		CA:          "",
+		Name:        types.ConsoleSessionSecret,
+		Subject:     "",
+		ConnectJson: false,
+		Data: map[string][]byte{
+			"session_secret": keyText[:],
+		},
+		Post: false,
+	}, nil
+}

pkg/kube/secrets_test.go

@@ -0,0 +1,71 @@
+package kube
+
+import (
+	"bytes"
+	"crypto/rand"
+	"fmt"
+	"io"
+	"testing"
+
+	"github.com/skupperproject/skupper/api/types"
+	"gotest.tools/assert"
+)
+
+func TestGenerateConsoleSessionCredentials(t *testing.T) {
+	zeros := bytes.NewReader(make([]byte, 128))
+	const zerosEncoded = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="
+
+	exampleText := "GenerateConsoleSessionCredentials"
+	exampleTextEncoded := "R2VuZXJhdGVDb25zb2xlU2Vzc2lvbkNyZWRlbnRpYWw="
+
+	testcases := []struct {
+		Input     io.Reader
+		CheckData func(map[string][]byte) error
+	}{
+		{
+			Input: zeros,
+			CheckData: func(data map[string][]byte) error {
+				if string(data["session_secret"]) != zerosEncoded {
+					return fmt.Errorf("session secret should have been %q but got %v", zerosEncoded, data)
+				}
+				return nil
+			},
+		},
+		{
+			Input: nil,
+			CheckData: func(data map[string][]byte) error {
+				if len(data["session_secret"]) != 44 {
+					return fmt.Errorf("session secret should have been 44 bytes long but got %v", data)
+				}
+				return nil
+			},
+		},
+		{
+			Input: rand.Reader,
+			CheckData: func(data map[string][]byte) error {
+				if len(data["session_secret"]) != 44 {
+					return fmt.Errorf("session secret should have been 44 bytes long but got %v", data)
+				}
+				return nil
+			},
+		},
+		{
+			Input: bytes.NewReader([]byte(exampleText)),
+			CheckData: func(data map[string][]byte) error {
+				if string(data["session_secret"]) != exampleTextEncoded {
+					return fmt.Errorf("session secret should have been %q but got %v", exampleTextEncoded, data)
+				}
+				return nil
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run("", func(t *testing.T) {
+			creds, err := GenerateConsoleSessionCredentials(tc.Input)
+			assert.Assert(t, err)
+			assert.Equal(t, creds.Name, types.ConsoleSessionSecret)
+			assert.Assert(t, tc.CheckData(creds.Data))
+		})
+	}
+}