Fix hmac secret (#2768)

* fix hmac secret

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

* add oidc hmac secret to resource tree

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

* fix gen

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: huabing zhao <zhaohuabing@gmail.com>

internal/crypto/certgen.go

@@ -77,6 +77,7 @@ type Certificates struct {
 	EnvoyPrivateKey           []byte
 	EnvoyRateLimitCertificate []byte
 	EnvoyRateLimitPrivateKey  []byte
+	OIDCHMACSecret            []byte
 }
 
 // certificateRequest defines a certificate request.
@@ -153,6 +154,11 @@ func GenerateCerts(cfg *config.Server) (*Certificates, error) {
 			return nil, err
 		}
 
+		oidcHMACSecret, err := generateHMACSecret()
+		if err != nil {
+			return nil, err
+		}
+
 		return &Certificates{
 			CACertificate:             caCertPEM,
 			EnvoyGatewayCertificate:   egCert,
@@ -161,6 +167,7 @@ func GenerateCerts(cfg *config.Server) (*Certificates, error) {
 			EnvoyPrivateKey:           envoyKey,
 			EnvoyRateLimitCertificate: envoyRateLimitCert,
 			EnvoyRateLimitPrivateKey:  envoyRateLimitKey,
+			OIDCHMACSecret:            oidcHMACSecret,
 		}, nil
 	default:
 		// Envoy Gateway, e.g. self-signed CA, is the only supported certificate provider.
@@ -285,3 +292,19 @@ func kubeServiceNames(service, namespace, dnsName string) []string {
 		fmt.Sprintf("%s.%s.svc.%s", service, namespace, dnsName),
 	}
 }
+
+func generateHMACSecret() ([]byte, error) {
+	// Set the desired length of the secret key in bytes
+	keyLength := 32
+
+	// Create a byte slice to hold the random bytes
+	key := make([]byte, keyLength)
+
+	// Read random bytes from the cryptographically secure random number generator
+	_, err := rand.Read(key)
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate hmack secret key: %w", err)
+	}
+
+	return key, nil
+}

internal/crypto/certgen_test.go

@@ -7,6 +7,7 @@ package crypto
 
 import (
 	"crypto/x509"
+	"encoding/base64"
 	"encoding/pem"
 	"fmt"
 	"testing"
@@ -153,3 +154,9 @@ func verifyCert(certPEM []byte, roots *x509.CertPool, dnsname string, currentTim
 
 	return nil
 }
+
+func TestGenerateHMACSecret(t *testing.T) {
+	bytes, _ := generateHMACSecret()
+	encodedSecret := base64.StdEncoding.EncodeToString(bytes)
+	fmt.Println("Base64 encoded secret:", encodedSecret)
+}

internal/gatewayapi/runner/runner.go

@@ -72,6 +72,7 @@ func (r *Runner) subscribeAndTranslate(ctx context.Context) {
 					GatewayClassName:        v1.ObjectName(resources.GatewayClass.Name),
 					GlobalRateLimitEnabled:  r.EnvoyGateway.RateLimit != nil,
 					EnvoyPatchPolicyEnabled: r.EnvoyGateway.ExtensionAPIs != nil && r.EnvoyGateway.ExtensionAPIs.EnableEnvoyPatchPolicy,
+					Namespace:               r.Namespace,
 				}
 
 				// If an extension is loaded, pass its supported groups/kinds to the translator

internal/gatewayapi/securitypolicy.go

@@ -35,6 +35,10 @@ const (
 	defaultRedirectURL  = "%REQ(x-forwarded-proto)%://%REQ(:authority)%/oauth2/callback"
 	defaultRedirectPath = "/oauth2/callback"
 	defaultLogoutPath   = "/logout"
+
+	// nolint: gosec
+	oidcHMACSecretName = "envoy-oidc-hmac"
+	oidcHMACSecretKey  = "hmac-secret"
 )
 
 func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.SecurityPolicy,
@@ -546,6 +550,20 @@ func (t *Translator) buildOIDC(
 	// Generate a unique cookie suffix for oauth filters
 	suffix := utils.Digest32(string(policy.UID))
 
+	// Get the HMAC secret
+	// HMAC secret is generated by the CertGen job and stored in a secret
+	// We need to rotate the HMAC secret in the future, probably the same
+	// way we rotate the certs generated by the CertGen job.
+	hmacSecret := resources.GetSecret(t.Namespace, oidcHMACSecretName)
+	if hmacSecret == nil {
+		return nil, fmt.Errorf("HMAC secret %s/%s not found", t.Namespace, oidcHMACSecretName)
+	}
+	hmacData, ok := hmacSecret.Data[oidcHMACSecretKey]
+	if !ok || len(hmacData) == 0 {
+		return nil, fmt.Errorf(
+			"HMAC secret not found in secret %s/%s", t.Namespace, oidcHMACSecretName)
+	}
+
 	return &ir.OIDC{
 		Provider:     *provider,
 		ClientID:     oidc.ClientID,
@@ -555,6 +573,7 @@ func (t *Translator) buildOIDC(
 		RedirectPath: redirectPath,
 		LogoutPath:   logoutPath,
 		CookieSuffix: suffix,
+		HMACSecret:   hmacData,
 	}, nil
 }
 

internal/gatewayapi/testdata/securitypolicy-with-oidc.in.yaml

@@ -20,6 +20,13 @@ secrets:
     name: client3-secret
   data:
     client-secret: Y2xpZW50MTpzZWNyZXQK
+- apiVersion: v1
+  kind: Secret
+  metadata:
+    namespace: envoy-gateway-system
+    name: envoy-oidc-hmac
+  data:
+    hmac-secret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY=
 gateways:
 - apiVersion: gateway.networking.k8s.io/v1
   kind: Gateway

internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml

@@ -240,6 +240,7 @@ xdsIR:
           clientID: client2.oauth.foo.com
           clientSecret: Y2xpZW50MTpzZWNyZXQK
           cookieSuffix: 5f93c2e4
+          hmacSecret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY=
           logoutPath: /foo/logout
           provider:
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
@@ -273,6 +274,7 @@ xdsIR:
           clientID: client1.apps.googleusercontent.com
           clientSecret: Y2xpZW50MTpzZWNyZXQK
           cookieSuffix: b0a1b740
+          hmacSecret: qrOYACHXoe7UEDI/raOjNSx+Z9ufXSc/22C3T6X/zPY=
           logoutPath: /bar/logout
           provider:
             authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth

internal/gatewayapi/translator.go

@@ -93,6 +93,9 @@ type Translator struct {
 	// introduced by an Extension so that the translator can
 	// store referenced resources in the IR for later use.
 	ExtensionGroupKinds []schema.GroupKind
+
+	// Namespace is the namespace that Envoy Gateway runs in.
+	Namespace string
 }
 
 type TranslateResult struct {

internal/gatewayapi/translator_test.go

@@ -77,6 +77,7 @@ func TestTranslate(t *testing.T) {
 				GatewayClassName:        "envoy-gateway-class",
 				GlobalRateLimitEnabled:  true,
 				EnvoyPatchPolicyEnabled: envoyPatchPolicyEnabled,
+				Namespace:               "envoy-gateway-system",
 			}
 
 			// Add common test fixtures

internal/ir/xds.go

@@ -179,6 +179,7 @@ func (x Xds) Printable() *Xds {
 			// Omit field
 			if route.OIDC != nil {
 				route.OIDC.ClientSecret = redacted
+				route.OIDC.HMACSecret = redacted
 			}
 			if route.BasicAuth != nil {
 				route.BasicAuth.Users = redacted
@@ -526,9 +527,11 @@ type OIDC struct {
 	// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
 	//
 	// This is an Opaque secret. The client secret should be stored in the key "client-secret".
-
 	ClientSecret []byte `json:"clientSecret,omitempty" yaml:"clientSecret,omitempty"`
 
+	// HMACSecret is the secret used to sign the HMAC of the OAuth2 filter cookies.
+	HMACSecret []byte `json:"hmacSecret,omitempty" yaml:"hmacSecret,omitempty"`
+
 	// The OIDC scopes to be used in the
 	// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
 	Scopes []string `json:"scopes,omitempty" yaml:"scopes,omitempty"`

internal/provider/kubernetes/controller.go

@@ -308,6 +308,9 @@ func (r *gatewayAPIReconciler) Reconcile(ctx context.Context, _ reconcile.Reques
 		// Add the referenced Secrets in SecurityPolicies to the resourceTree
 		r.processSecurityPolicySecretRefs(ctx, gwcResource, resourceMappings)
 
+		// Add the OIDC HMAC Secret to the resourceTree
+		r.processOIDCHMACSecret(ctx, gwcResource)
+
 		// Add all BackendTLSPolies
 		backendTLSPolicies := gwapiv1a2.BackendTLSPolicyList{}
 		if err := r.client.List(ctx, &backendTLSPolicies); err != nil {
@@ -434,6 +437,35 @@ func (r *gatewayAPIReconciler) processSecurityPolicySecretRefs(
 	}
 }
 
+// processOIDCHMACSecret adds the OIDC HMAC Secret to the resourceTree.
+// The OIDC HMAC Secret is created by the CertGen job and is used by SecurityPolicy
+// to configure OAuth2 filters.
+func (r *gatewayAPIReconciler) processOIDCHMACSecret(ctx context.Context, resourceTree *gatewayapi.Resources) {
+	var (
+		secret corev1.Secret
+		err    error
+	)
+
+	err = r.client.Get(ctx,
+		types.NamespacedName{Namespace: r.namespace, Name: oidcHMACSecretName},
+		&secret,
+	)
+
+	// we don't return an error here, because we want to continue reconciling
+	// despite that the OIDC HMAC secret can't be found.
+	// If the OIDC HMAC Secret is missing, the SecurityPolicy with OIDC will be
+	// marked as invalid in its status when translating to IR.
+	if err != nil {
+		r.log.Error(err,
+			"failed to process OIDC HMAC Secret",
+			"namespace", r.namespace, "name", oidcHMACSecretName)
+		return
+	}
+
+	resourceTree.Secrets = append(resourceTree.Secrets, &secret)
+	r.log.Info("processing OIDC HMAC Secret", "namespace", r.namespace, "name", oidcHMACSecretName)
+}
+
 // processSecretRef adds the referenced Secret to the resourceTree if it's valid.
 // - If it exists in the same namespace as the owner.
 // - If it exists in a different namespace, and there is a ReferenceGrant.

internal/provider/kubernetes/predicates.go

@@ -27,6 +27,9 @@ import (
 	"github.com/envoyproxy/gateway/internal/utils"
 )
 
+// nolint: gosec
+const oidcHMACSecretName = "envoy-oidc-hmac"
+
 // hasMatchingController returns true if the provided object is a GatewayClass
 // with a Spec.Controller string matching this Envoy Gateway's controller string,
 // or false otherwise.
@@ -153,6 +156,10 @@ func (r *gatewayAPIReconciler) validateSecretForReconcile(obj client.Object) boo
 		return true
 	}
 
+	if r.isOIDCHMACSecret(&nsName) {
+		return true
+	}
+
 	return false
 }
 
@@ -202,6 +209,14 @@ func (r *gatewayAPIReconciler) isCtpReferencingSecret(nsName *types.NamespacedNa
 	return len(ctpList.Items) > 0
 }
 
+func (r *gatewayAPIReconciler) isOIDCHMACSecret(nsName *types.NamespacedName) bool {
+	oidcHMACSecret := types.NamespacedName{
+		Namespace: r.namespace,
+		Name:      oidcHMACSecretName,
+	}
+	return *nsName == oidcHMACSecret
+}
+
 // validateServiceForReconcile tries finding the owning Gateway of the Service
 // if it exists, finds the Gateway's Deployment, and further updates the Gateway
 // status Ready condition. All Services are pushed for reconciliation.

internal/provider/kubernetes/secrets.go

@@ -26,7 +26,10 @@ var (
 
 // caCertificateKey is the key name for accessing TLS CA certificate bundles
 // in Kubernetes Secrets.
-const caCertificateKey = "ca.crt"
+const (
+	caCertificateKey = "ca.crt"
+	hmacSecretKey    = "hmac-secret"
+)
 
 func newSecret(secretType corev1.SecretType, name string, namespace string, data map[string][]byte) corev1.Secret {
 	return corev1.Secret{
@@ -76,6 +79,13 @@ func CertsToSecret(namespace string, certs *crypto.Certificates) []corev1.Secret
 				corev1.TLSCertKey:       certs.EnvoyRateLimitCertificate,
 				corev1.TLSPrivateKeyKey: certs.EnvoyRateLimitPrivateKey,
 			}),
+		newSecret(
+			corev1.SecretTypeOpaque,
+			"envoy-oidc-hmac",
+			namespace,
+			map[string][]byte{
+				hmacSecretKey: certs.OIDCHMACSecret,
+			}),
 	}
 }
 

internal/xds/translator/oidc.go

@@ -6,7 +6,6 @@
 package translator
 
 import (
-	"crypto/rand"
 	"errors"
 	"fmt"
 
@@ -279,11 +278,7 @@ func createOAuth2Secrets(tCtx *types.ResourceVersionTable, routes []*ir.HTTPRout
 			errs = errors.Join(errs, err)
 		}
 
-		hmacSecret, err := buildOAuth2HMACSecret(route)
-		if err != nil {
-			errs = errors.Join(errs, err)
-		}
-		if err := addXdsSecret(tCtx, hmacSecret); err != nil {
+		if err := addXdsSecret(tCtx, buildOAuth2HMACSecret(route)); err != nil {
 			errs = errors.Join(errs, err)
 		}
 	}
@@ -308,25 +303,21 @@ func buildOAuth2ClientSecret(route *ir.HTTPRoute) *tlsv3.Secret {
 	return clientSecret
 }
 
-func buildOAuth2HMACSecret(route *ir.HTTPRoute) (*tlsv3.Secret, error) {
-	hmac, err := generateHMACSecretKey()
-	if err != nil {
-		return nil, fmt.Errorf("failed to generate hmack secret key: %w", err)
-	}
+func buildOAuth2HMACSecret(route *ir.HTTPRoute) *tlsv3.Secret {
 	hmacSecret := &tlsv3.Secret{
 		Name: oauth2HMACSecretName(route),
 		Type: &tlsv3.Secret_GenericSecret{
 			GenericSecret: &tlsv3.GenericSecret{
 				Secret: &corev3.DataSource{
 					Specifier: &corev3.DataSource_InlineBytes{
-						InlineBytes: hmac,
+						InlineBytes: route.OIDC.HMACSecret,
 					},
 				},
 			},
 		},
 	}
 
-	return hmacSecret, nil
+	return hmacSecret
 }
 
 func oauth2ClientSecretName(route *ir.HTTPRoute) string {
@@ -337,22 +328,6 @@ func oauth2HMACSecretName(route *ir.HTTPRoute) string {
 	return fmt.Sprintf("%s/oauth2/hmac_secret", route.Name)
 }
 
-func generateHMACSecretKey() ([]byte, error) {
-	// Set the desired length of the secret key in bytes
-	keyLength := 32
-
-	// Create a byte slice to hold the random bytes
-	key := make([]byte, keyLength)
-
-	// Read random bytes from the cryptographically secure random number generator
-	_, err := rand.Read(key)
-	if err != nil {
-		return nil, err
-	}
-
-	return key, nil
-}
-
 // patchRoute patches the provided route with the oauth2 config if applicable.
 // Note: this method enables the corresponding oauth2 filter for the provided route.
 func (*oidc) patchRoute(route *routev3.Route, irRoute *ir.HTTPRoute) error {