Skip to content

Commit

Permalink
Configure oauth-proxy with random cookie secret
Browse files Browse the repository at this point in the history
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 <christian@c-kruse.com>

Improve error handling per review

Signed-off-by: Christian Kruse <christian@c-kruse.com>
  • Loading branch information
c-kruse committed Jul 1, 2024
1 parent 5503b3b commit d2cb378
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 5 deletions.
1 change: 1 addition & 0 deletions api/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
22 changes: 17 additions & 5 deletions client/router_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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/")
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions client/router_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
97 changes: 97 additions & 0 deletions client/router_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
28 changes: 28 additions & 0 deletions pkg/kube/secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package kube

import (
"context"
"crypto/rand"
"encoding/base64"
"fmt"
"io"
"strings"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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
}
71 changes: 71 additions & 0 deletions pkg/kube/secrets_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}

0 comments on commit d2cb378

Please sign in to comment.