add api key secret validation to reject duplicated keys, remove repeated maps in config, remove reject code, add unit tests

Signed-off-by: Haywood Shannon <[email protected]>

Signed-off-by: Haywood Shannon <[email protected]>

Author: haywoodsh

examples/custom-resources/api-key/api-key-policy-2.yaml

@@ -9,7 +9,4 @@ spec:
       - "X-header-name"
       query:
       - "queryName"
-    rejectCodes: # optional
-      notSupplied: 408
-      noMatch: 410
     clientSecret: api-key-client-secret-2

examples/custom-resources/api-key/api-key-policy.yaml

@@ -11,7 +11,4 @@ spec:
       - "some-other-header"
       query:
       - "queryName"
-#    rejectCodes: # optional
-#      notSupplied: 401
-#      noMatch: 403
     clientSecret: api-key-client-secret-1

examples/custom-resources/api-key/api-key-secret-1.yaml

@@ -5,4 +5,4 @@ metadata:
 type: nginx.org/apikey
 data:
     client1: cGFzc3dvcmQ=
-    client2: N2ViNDMwOGItY2Q1Yi00NDEzLWI0NTUtYjMyZmQ4OTg2MmZk
+    client2: N2ViNDMwOGItY2Q1Yi00NDEzLWI0NTUtYjMyZmQ4OTg2MmZk

examples/custom-resources/api-key/cafe-virtual-server-2.yaml

@@ -24,3 +24,9 @@ spec:
       pass: coffee
     policies:
       - name: api-key-policy-2
+  - path: /coffee2
+    action:
+      pass: coffee
+    policies:
+    - name: api-key-policy
+

examples/custom-resources/api-key/coffee-virtual-server-route.yaml

@@ -14,3 +14,8 @@ spec:
       pass: coffee
     policies:
       - name: api-key-policy-2
+  - path: /coffee2
+    action:
+      pass: coffee
+    policies:
+    - name: api-key-policy

internal/configs/configurator.go

@@ -1991,9 +1991,7 @@ func (cnf *Configurator) AddOrUpdateSecret(secret *api_v1.Secret) string {
 		// OIDC ClientSecret is not required on the filesystem, it is written directly to the config file.
 		return ""
 	case secrets.SecretTypeAPIKey:
-		glog.Infof("adding apikey secret: %s", secret.Name)
-		// OIDC ClientSecret is not required on the filesystem, it is written directly to the config file.
-
+		// APIKey ClientSecret is not required on the filesystem, it is written directly to the config file.
 		return ""
 	default:
 		return cnf.addOrUpdateTLSSecret(secret)

internal/configs/version1/nginx.tmpl

@@ -1,5 +1,4 @@
 {{- /*gotype: github.com/nginxinc/kubernetes-ingress/internal/configs/version1.MainConfig*/ -}}
-
 worker_processes  {{.WorkerProcesses}};
 {{- if .WorkerRlimitNofile}}
 worker_rlimit_nofile {{.WorkerRlimitNofile}};{{end}}

internal/configs/version2/http.go

@@ -139,17 +139,9 @@ type OIDC struct {
 }
 
 type APIKey struct {
-	Header                []string
-	Query                 []string
-	RejectCodeNotSupplied int
-	RejectCodeNoMatch     int
-	Clients               []Client
-	MapName               string
-}
-
-type Client struct {
-	ClientID     string
-	EncryptedKey string
+	Header  []string
+	Query   []string
+	MapName string
 }
 
 // WAF defines WAF configuration.

internal/configs/version2/nginx-plus.virtualserver.tmpl

@@ -1,6 +1,4 @@
 {{- /*gotype: github.com/nginxinc/kubernetes-ingress/internal/configs/version2.VirtualServerConfig*/ -}}
-
-
 {{ range $u := .Upstreams }}
 upstream {{ $u.Name }} {
     zone {{ $u.Name }} {{ if ne $u.UpstreamZoneSize "0" }}{{ $u.UpstreamZoneSize }}{{ else }}512k{{ end }};

internal/configs/virtualserver.go

@@ -416,18 +416,18 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 		vsNamespace:    vsEx.VirtualServer.Namespace,
 		vsName:         vsEx.VirtualServer.Name,
 	}
-	policiesCfg, mapsFromPolicies := vsc.generatePolicies(ownerDetails, vsEx.VirtualServer.Spec.Policies, vsEx.Policies, specContext, policyOpts)
-
-	maps = append(maps, mapsFromPolicies...)
+	policiesCfg := vsc.generatePolicies(ownerDetails, vsEx.VirtualServer.Spec.Policies, vsEx.Policies, specContext, policyOpts)
 	if policiesCfg.JWKSAuthEnabled {
 		jwtAuthKey := policiesCfg.JWTAuth.Key
 		policiesCfg.JWTAuthList = make(map[string]*version2.JWTAuth)
 		policiesCfg.JWTAuthList[jwtAuthKey] = policiesCfg.JWTAuth
 	}
-	//
-	//if policiesCfg.APIKey {
-	//	apiKey := policiesCfg.APIKey.MapName
-	//}
+
+	if policiesCfg.APIKeyEnabled {
+		apiMapName := policiesCfg.APIKey.MapName
+		policiesCfg.APIKeyClientMap = make(map[string][]APIKeyClient)
+		policiesCfg.APIKeyClientMap[apiMapName] = policiesCfg.APIKeyClients
+	}
 
 	dosCfg := generateDosCfg(dosResources[""])
 
@@ -571,9 +571,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 			vsNamespace:    vsEx.VirtualServer.Namespace,
 			vsName:         vsEx.VirtualServer.Name,
 		}
-		routePoliciesCfg, mapsFromPolicies := vsc.generatePolicies(ownerDetails, r.Policies, vsEx.Policies, routeContext, policyOpts)
-		maps = append(maps, mapsFromPolicies...)
-
+		routePoliciesCfg := vsc.generatePolicies(ownerDetails, r.Policies, vsEx.Policies, routeContext, policyOpts)
 		if policiesCfg.OIDC {
 			routePoliciesCfg.OIDC = policiesCfg.OIDC
 		}
@@ -589,6 +587,16 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 				policiesCfg.JWTAuthList[jwtAuthKey] = routePoliciesCfg.JWTAuth
 			}
 		}
+		if routePoliciesCfg.APIKeyEnabled {
+			policiesCfg.APIKeyEnabled = routePoliciesCfg.APIKeyEnabled
+			apiMapName := routePoliciesCfg.APIKey.MapName
+			if policiesCfg.APIKeyClientMap == nil {
+				policiesCfg.APIKeyClientMap = make(map[string][]APIKeyClient)
+			}
+			if _, exists := policiesCfg.APIKeyClientMap[apiMapName]; !exists {
+				policiesCfg.APIKeyClientMap[apiMapName] = routePoliciesCfg.APIKeyClients
+			}
+		}
 		limitReqZones = append(limitReqZones, routePoliciesCfg.LimitReqZones...)
 
 		dosRouteCfg := generateDosCfg(dosResources[r.Path])
@@ -703,8 +711,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 				policyRefs = r.Policies
 				context = subRouteContext
 			}
-			routePoliciesCfg, mapsFromPolicies := vsc.generatePolicies(ownerDetails, policyRefs, vsEx.Policies, context, policyOpts)
-			maps = append(maps, mapsFromPolicies...)
+			routePoliciesCfg := vsc.generatePolicies(ownerDetails, policyRefs, vsEx.Policies, context, policyOpts)
 			if policiesCfg.OIDC {
 				routePoliciesCfg.OIDC = policiesCfg.OIDC
 			}
@@ -720,6 +727,17 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 					policiesCfg.JWTAuthList[jwtAuthKey] = routePoliciesCfg.JWTAuth
 				}
 			}
+			if routePoliciesCfg.APIKeyEnabled {
+				policiesCfg.APIKeyEnabled = routePoliciesCfg.APIKeyEnabled
+				apiMapName := routePoliciesCfg.APIKey.MapName
+				if policiesCfg.APIKeyClientMap == nil {
+					policiesCfg.APIKeyClientMap = make(map[string][]APIKeyClient)
+				}
+				if _, exists := policiesCfg.APIKeyClientMap[apiMapName]; !exists {
+					policiesCfg.APIKeyClientMap[apiMapName] = routePoliciesCfg.APIKeyClients
+				}
+			}
+
 			limitReqZones = append(limitReqZones, routePoliciesCfg.LimitReqZones...)
 
 			dosRouteCfg := generateDosCfg(dosResources[r.Path])
@@ -787,6 +805,10 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(
 		}
 	}
 
+	for mapName, apiKeyClients := range policiesCfg.APIKeyClientMap {
+		maps = append(maps, *generateAPIKeyClientMap(mapName, apiKeyClients))
+	}
+
 	httpSnippets := generateSnippets(vsc.enableSnippets, vsEx.VirtualServer.Spec.HTTPSnippets, []string{})
 	serverSnippets := generateSnippets(
 		vsc.enableSnippets,
@@ -870,8 +892,10 @@ type policiesCfg struct {
 	IngressMTLS     *version2.IngressMTLS
 	EgressMTLS      *version2.EgressMTLS
 	OIDC            bool
+	APIKeyEnabled   bool
 	APIKey          *version2.APIKey
-	APIKeyList      map[string]*version2.APIKey
+	APIKeyClients   []APIKeyClient
+	APIKeyClientMap map[string][]APIKeyClient
 	WAF             *version2.WAF
 	ErrorReturn     *version2.Return
 	BundleValidator bundleValidator
@@ -886,6 +910,11 @@ type internalBundleValidator struct {
 	bundlePath string
 }
 
+type APIKeyClient struct {
+	ClientID  string
+	HashedKey string
+}
+
 func (i internalBundleValidator) validate(bundle string) (string, error) {
 	bundle = path.Join(i.bundlePath, bundle)
 	_, err := os.Stat(bundle)
@@ -1308,19 +1337,17 @@ func (p *policiesCfg) addAPIKeyConfig(
 	polNamespace string,
 	secretRefs map[string]*secrets.SecretReference,
 	context string,
-) (*validationResults, *version2.Map) {
+) *validationResults {
 	res := newValidationResults()
-	var clients []version2.Client
 	if p.APIKey != nil {
 		res.addWarningf(
 			"Multiple APIKey policies in the same context is not valid. APIKey policy %s will be ignored",
 			polKey,
 		)
 		res.isError = true
-		return res, nil
+		return res
 	}
 
-	// if apiKey.ClientSecret != "" {
 	secretKey := fmt.Sprintf("%v/%v", polNamespace, apiKey.ClientSecret)
 	glog.Infof("secretKey: %v", secretKey)
 	secretRef := secretRefs[secretKey]
@@ -1333,57 +1360,65 @@ func (p *policiesCfg) addAPIKeyConfig(
 	if secretType != "" && secretType != secrets.SecretTypeAPIKey {
 		res.addWarningf("API Key policy %s references a secret %s of a wrong type '%s', must be '%s'", polKey, secretKey, secretType, secrets.SecretTypeAPIKey)
 		res.isError = true
-		return res, nil
+		return res
 	} else if secretRef.Error != nil {
 		res.addWarningf("API Key %s references an invalid secret %s: %v", polKey, secretKey, secretRef.Error)
 		res.isError = true
-		return res, nil
+		return res
+	}
+
+	p.APIKeyClients = generateAPIKeyClients(secretRef.Secret.Data)
+
+	mapName := fmt.Sprintf("apikey_auth_client_name_%s", strings.Split(strings.Replace(polKey, "-", "_", -1), "/")[1])
+	p.APIKey = &version2.APIKey{
+		Header:  apiKey.SuppliedIn.Header,
+		Query:   apiKey.SuppliedIn.Query,
+		MapName: mapName,
 	}
+	p.APIKeyEnabled = true
+	return res
+}
 
-	for clientID, clientSecret := range secretRef.Secret.Data {
+func generateAPIKeyClients(secretData map[string][]byte) []APIKeyClient {
+	var clients []APIKeyClient
+	for clientID, apiKey := range secretData {
 
 		h := sha256.New()
-		h.Write([]byte(clientSecret))
+		h.Write([]byte(apiKey))
 		sha256Hash := hex.EncodeToString(h.Sum(nil))
 		base64Str := base64.URLEncoding.EncodeToString(h.Sum(nil))
 
-		glog.Infof("clientSecret %s", clientSecret)
+		glog.Infof("apiKey %s", apiKey)
 		glog.Infof("sha %s", sha256Hash)
 		glog.Infof("base64Str %s", base64Str)
-		clients = append(clients, version2.Client{ClientID: clientID, EncryptedKey: sha256Hash}) //
+		clients = append(clients, APIKeyClient{ClientID: clientID, HashedKey: sha256Hash}) //
 	}
+	return clients
+}
 
-	default_parameter := version2.Parameter{
+func generateAPIKeyClientMap(mapName string, apiKeyClients []APIKeyClient) *version2.Map {
+	glog.Infof("mapName: %v, apiKeyClients: %v", mapName, apiKeyClients)
+
+	defaultParam := version2.Parameter{
 		Value:  "default",
 		Result: "\"\"",
 	}
 
-	params := []version2.Parameter{default_parameter}
-	for _, client := range clients {
+	params := []version2.Parameter{defaultParam}
+	for _, client := range apiKeyClients {
 		params = append(params, version2.Parameter{
-			Value:  fmt.Sprintf("\"%s\"", client.EncryptedKey),
+			Value:  fmt.Sprintf("\"%s\"", client.HashedKey),
 			Result: fmt.Sprintf("\"%s\"", client.ClientID),
 		})
 	}
 
 	sourceName := "$apikey_auth_token"
-	mapName := fmt.Sprintf("apikey_auth_client_name_%s", strings.Split(strings.Replace(polKey, "-", "_", -1), "/")[1])
 
-	shaToClient := &version2.Map{
+	return &version2.Map{
 		Source:     sourceName,
 		Variable:   fmt.Sprintf("$%s", mapName),
 		Parameters: params,
 	}
-
-	p.APIKey = &version2.APIKey{
-		Header:                apiKey.SuppliedIn.Header,
-		Query:                 apiKey.SuppliedIn.Query,
-		RejectCodeNotSupplied: generateIntFromPointer(apiKey.RejectCodes.NotSupplied, 401),
-		RejectCodeNoMatch:     generateIntFromPointer(apiKey.RejectCodes.NoMatch, 403),
-		Clients:               clients,
-		MapName:               mapName,
-	}
-	return res, shaToClient
 }
 
 func (p *policiesCfg) addWAFConfig(
@@ -1474,9 +1509,8 @@ func (vsc *virtualServerConfigurator) generatePolicies(
 	policies map[string]*conf_v1.Policy,
 	context string,
 	policyOpts policyOptions,
-) (policiesCfg, []version2.Map) {
+) policiesCfg {
 	config := newPoliciesConfig(vsc.bundleValidator)
-	maps := []version2.Map{}
 
 	for _, p := range policyRefs {
 		polNamespace := p.Namespace
@@ -1518,44 +1552,27 @@ func (vsc *virtualServerConfigurator) generatePolicies(
 			case pol.Spec.OIDC != nil:
 				res = config.addOIDCConfig(pol.Spec.OIDC, key, polNamespace, policyOpts.secretRefs, vsc.oidcPolCfg)
 			case pol.Spec.APIKey != nil:
-				res, shaToClientMap := config.addAPIKeyConfig(pol.Spec.APIKey, key, polNamespace, policyOpts.secretRefs, context)
-				// TODO: refactor
-				if res != nil && len(res.warnings) > 0 {
-					vsc.addWarnings(ownerDetails.owner, res.warnings)
-				}
-
-				if res != nil && res.isError {
-					return policiesCfg{
-						ErrorReturn: &version2.Return{Code: 500},
-					}, maps
-				}
-				if res != nil && !res.isError && shaToClientMap != nil {
-					maps = append(maps, *shaToClientMap)
-				}
-
+				res = config.addAPIKeyConfig(pol.Spec.APIKey, key, polNamespace, policyOpts.secretRefs, context)
 			case pol.Spec.WAF != nil:
 				res = config.addWAFConfig(pol.Spec.WAF, key, polNamespace, policyOpts.apResources)
 			default:
 				res = newValidationResults()
 			}
-			if res != nil && len(res.warnings) > 0 {
-				vsc.addWarnings(ownerDetails.owner, res.warnings)
-			}
-
-			if res != nil && res.isError {
+			vsc.addWarnings(ownerDetails.owner, res.warnings)
+			if res.isError {
 				return policiesCfg{
 					ErrorReturn: &version2.Return{Code: 500},
-				}, maps
+				}
 			}
 		} else {
 			vsc.addWarningf(ownerDetails.owner, "Policy %s is missing or invalid", key)
 			return policiesCfg{
 				ErrorReturn: &version2.Return{Code: 500},
-			}, maps
+			}
 		}
 	}
 
-	return *config, maps
+	return *config
 }
 
 func generateLimitReq(zoneName string, rateLimitPol *conf_v1.RateLimit) version2.LimitReq {