Skip to content

Commit

Permalink
Gateway privileged port mapping (#2707)
Browse files Browse the repository at this point in the history
* Adds port mapping to Gateway Class Config to avoid running container on privileged ports

Co-authored-by: Nathan Coleman <[email protected]>
  • Loading branch information
missylbytes and nathancoleman authored Aug 8, 2023
1 parent 671675d commit 71cdbc2
Show file tree
Hide file tree
Showing 16 changed files with 209 additions and 43 deletions.
3 changes: 3 additions & 0 deletions .changelog/2707.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:feature
api-gateway: adds ability to map privileged ports on Gateway listeners to unprivileged ports so that containers do not require additional privileges
```
9 changes: 9 additions & 0 deletions charts/consul/templates/crd-gatewayclassconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ spec:
description: The name of an existing SecurityContextConstraints
resource to bind to the managed role when running on OpenShift.
type: string
mapPrivilegedContainerPorts:
type: integer
format: int32
minimum: 0
maximum: 64512
description: mapPrivilegedContainerPorts is the value which Consul will add to privileged container port
values (ports < 1024) defined on a Gateway when the number is greater than 0. This cannot be more than
64512 as the highest privileged port is 1023, which would then map to 65535, which is the highest
valid port number.
type: object
type: object
served: true
Expand Down
1 change: 1 addition & 0 deletions charts/consul/templates/gateway-resources-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ spec:
{{- if .Values.global.openshift.enabled }}
- -openshift-scc-name={{ .Values.connectInject.apiGateway.managedGatewayClass.openshiftSCCName }}
{{- end }}
- -map-privileged-container-ports={{ .Values.connectInject.apiGateway.managedGatewayClass.mapPrivilegedContainerPorts }}
{{- end}}
resources:
requests:
Expand Down
6 changes: 6 additions & 0 deletions charts/consul/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2206,6 +2206,12 @@ connectInject:
# @type: string
openshiftSCCName: "restricted-v2"

# This value defines the amount we will add to privileged container ports on gateways that use this class.
# This is useful if you don't want to give your containers extra permissions to run privileged ports.
# Example: The gateway listener is defined on port 80, but the underlying value of the port on the container
# will be the 80 + the number defined below.
mapPrivilegedContainerPorts: 0

# Configuration for the ServiceAccount created for the api-gateway component
serviceAccount:
# This value defines additional annotations for the client service account. This should be formatted as a multi-line
Expand Down
13 changes: 7 additions & 6 deletions cli/helm/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -576,12 +576,13 @@ type CopyAnnotations struct {
}

type ManagedGatewayClass struct {
Enabled bool `yaml:"enabled"`
NodeSelector interface{} `yaml:"nodeSelector"`
ServiceType string `yaml:"serviceType"`
UseHostPorts bool `yaml:"useHostPorts"`
CopyAnnotations CopyAnnotations `yaml:"copyAnnotations"`
OpenshiftSCCName string `yaml:"openshiftSCCName"`
Enabled bool `yaml:"enabled"`
NodeSelector interface{} `yaml:"nodeSelector"`
ServiceType string `yaml:"serviceType"`
UseHostPorts bool `yaml:"useHostPorts"`
CopyAnnotations CopyAnnotations `yaml:"copyAnnotations"`
OpenshiftSCCName string `yaml:"openshiftSCCName"`
MapPrivilegedContainerPorts int `yaml:"mapPrivilegedContainerPorts"`
}

type Service struct {
Expand Down
4 changes: 2 additions & 2 deletions control-plane/api-gateway/binding/binder.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (b *Binder) Snapshot() *Snapshot {

// calculate the status for the gateway
gatewayValidation = validateGateway(b.config.Gateway, registrationPods, b.config.ConsulGateway)
listenerValidation = validateListeners(b.config.Gateway, b.config.Gateway.Spec.Listeners, b.config.Resources)
listenerValidation = validateListeners(b.config.Gateway, b.config.Gateway.Spec.Listeners, b.config.Resources, b.config.GatewayClassConfig)
}

// used for tracking how many routes have successfully bound to which listeners
Expand Down Expand Up @@ -182,7 +182,7 @@ func (b *Binder) Snapshot() *Snapshot {
if b.config.ConsulGateway != nil {
consulStatus = b.config.ConsulGateway.Status
}
entry := b.config.Translator.ToAPIGateway(b.config.Gateway, b.config.Resources)
entry := b.config.Translator.ToAPIGateway(b.config.Gateway, b.config.Resources, gatewayClassConfig)
snapshot.Consul.Updates = append(snapshot.Consul.Updates, &common.ConsulUpdateOperation{
Entry: entry,
OnUpdate: b.handleGatewaySyncStatus(snapshot, &b.config.Gateway, consulStatus),
Expand Down
8 changes: 7 additions & 1 deletion control-plane/api-gateway/binding/result.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ var (
// We map anything under here to a custom ListenerConditionReason of Invalid on
// an Accepted status type.
errListenerNoTLSPassthrough = errors.New("TLS passthrough is not supported")

// This custom listener validation error is used to differentiate between an errListenerPortUnavailable because of
// direct port conflicts defined by the user (two listeners on the same port) vs a port conflict because we map
// privileged ports by adding the value passed into the gatewayClassConfig.
// (i.e. one listener on 80 with a privileged port mapping of 2000, and one listener on 2080 would conflict).
errListenerMappedToPrivilegedPortMapping = errors.New("listener conflicts with privileged port mapped by GatewayClassConfig privileged port mapping setting")
)

// listenerValidationResult contains the result of internally validating a single listener
Expand Down Expand Up @@ -291,7 +297,7 @@ func (l listenerValidationResult) programmedCondition(generation int64) metav1.C
func (l listenerValidationResult) acceptedCondition(generation int64) metav1.Condition {
now := timeFunc()
switch l.acceptedErr {
case errListenerPortUnavailable:
case errListenerPortUnavailable, errListenerMappedToPrivilegedPortMapping:
return metav1.Condition{
Type: "Accepted",
Status: metav1.ConditionFalse,
Expand Down
19 changes: 17 additions & 2 deletions control-plane/api-gateway/binding/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func validateCertificateData(secret corev1.Secret) error {

// validateListeners validates the given listeners both internally and with respect to each
// other for purposes of setting "Conflicted" status conditions.
func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener, resources *common.ResourceMap) listenerValidationResults {
func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener, resources *common.ResourceMap, gwcc *v1alpha1.GatewayClassConfig) listenerValidationResults {
var results listenerValidationResults
merged := make(map[gwv1beta1.PortNumber]mergedListeners)
for i, listener := range listeners {
Expand All @@ -235,7 +235,15 @@ func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener
listener: listener,
})
}

// This list keeps track of port conflicts directly on gateways. i.e., two listeners on the same port as
// defined by the user.
seenListenerPorts := map[int]struct{}{}
// This list keeps track of port conflicts caused by privileged port mappings.
seenContainerPorts := map[int]struct{}{}
portMapping := int32(0)
if gwcc != nil {
portMapping = gwcc.Spec.MapPrivilegedContainerPorts
}
for i, listener := range listeners {
var result listenerValidationResult

Expand All @@ -249,6 +257,10 @@ func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener
result.acceptedErr = errListenerUnsupportedProtocol
} else if listener.Port == 20000 { // admin port
result.acceptedErr = errListenerPortUnavailable
} else if _, ok := seenListenerPorts[int(listener.Port)]; ok {
result.acceptedErr = errListenerPortUnavailable
} else if _, ok := seenContainerPorts[common.ToContainerPort(listener.Port, portMapping)]; ok {
result.acceptedErr = errListenerMappedToPrivilegedPortMapping
}

result.routeKindErr = validateListenerAllowedRouteKinds(listener.AllowedRoutes)
Expand All @@ -261,6 +273,9 @@ func validateListeners(gateway gwv1beta1.Gateway, listeners []gwv1beta1.Listener
}

results = append(results, result)

seenListenerPorts[int(listener.Port)] = struct{}{}
seenContainerPorts[common.ToContainerPort(listener.Port, portMapping)] = struct{}{}
}
return results
}
Expand Down
31 changes: 28 additions & 3 deletions control-plane/api-gateway/binding/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,10 @@ func TestValidateListeners(t *testing.T) {
t.Parallel()

for name, tt := range map[string]struct {
listeners []gwv1beta1.Listener
expectedAcceptedErr error
listeners []gwv1beta1.Listener
expectedAcceptedErr error
listenerIndexToTest int
mapPrivilegedContainerPorts int32
}{
"valid protocol HTTP": {
listeners: []gwv1beta1.Listener{
Expand Down Expand Up @@ -563,9 +565,32 @@ func TestValidateListeners(t *testing.T) {
},
expectedAcceptedErr: errListenerPortUnavailable,
},
"conflicted port": {
listeners: []gwv1beta1.Listener{
{Protocol: gwv1beta1.TCPProtocolType, Port: 80},
{Protocol: gwv1beta1.TCPProtocolType, Port: 80},
},
expectedAcceptedErr: errListenerPortUnavailable,
listenerIndexToTest: 1,
},
"conflicted mapped port": {
listeners: []gwv1beta1.Listener{
{Protocol: gwv1beta1.TCPProtocolType, Port: 80},
{Protocol: gwv1beta1.TCPProtocolType, Port: 2080},
},
expectedAcceptedErr: errListenerMappedToPrivilegedPortMapping,
listenerIndexToTest: 1,
mapPrivilegedContainerPorts: 2000,
},
} {
t.Run(name, func(t *testing.T) {
require.Equal(t, tt.expectedAcceptedErr, validateListeners(gatewayWithFinalizer(gwv1beta1.GatewaySpec{}), tt.listeners, nil)[0].acceptedErr)
gwcc := &v1alpha1.GatewayClassConfig{
Spec: v1alpha1.GatewayClassConfigSpec{
MapPrivilegedContainerPorts: tt.mapPrivilegedContainerPorts,
},
}

require.Equal(t, tt.expectedAcceptedErr, validateListeners(gatewayWithFinalizer(gwv1beta1.GatewaySpec{}), tt.listeners, nil, gwcc)[tt.listenerIndexToTest].acceptedErr)
})
}
}
Expand Down
4 changes: 4 additions & 0 deletions control-plane/api-gateway/common/helm_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ type HelmConfig struct {
// EnableOpenShift indicates whether we're deploying into an OpenShift environment
// and should create SecurityContextConstraints.
EnableOpenShift bool

// MapPrivilegedServicePorts is the value which Consul will add to privileged container port values (ports < 1024)
// defined on a Gateway.
MapPrivilegedServicePorts int
}

type ConsulConfig struct {
Expand Down
22 changes: 18 additions & 4 deletions control-plane/api-gateway/common/translation.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ func (t ResourceTranslator) Namespace(namespace string) string {
}

// ToAPIGateway translates a kuberenetes API gateway into a Consul APIGateway Config Entry.
func (t ResourceTranslator) ToAPIGateway(gateway gwv1beta1.Gateway, resources *ResourceMap) *api.APIGatewayConfigEntry {
func (t ResourceTranslator) ToAPIGateway(gateway gwv1beta1.Gateway, resources *ResourceMap, gwcc *v1alpha1.GatewayClassConfig) *api.APIGatewayConfigEntry {
namespace := t.Namespace(gateway.Namespace)

listeners := ConvertSliceFuncIf(gateway.Spec.Listeners, func(listener gwv1beta1.Listener) (api.APIGatewayListener, bool) {
return t.toAPIGatewayListener(gateway, listener, resources)
return t.toAPIGatewayListener(gateway, listener, resources, gwcc)
})

return &api.APIGatewayConfigEntry{
Expand All @@ -81,7 +81,7 @@ var listenerProtocolMap = map[string]string{
"tcp": "tcp",
}

func (t ResourceTranslator) toAPIGatewayListener(gateway gwv1beta1.Gateway, listener gwv1beta1.Listener, resources *ResourceMap) (api.APIGatewayListener, bool) {
func (t ResourceTranslator) toAPIGatewayListener(gateway gwv1beta1.Gateway, listener gwv1beta1.Listener, resources *ResourceMap, gwcc *v1alpha1.GatewayClassConfig) (api.APIGatewayListener, bool) {
namespace := gateway.Namespace

var certificates []api.ResourceReference
Expand All @@ -104,17 +104,31 @@ func (t ResourceTranslator) toAPIGatewayListener(gateway gwv1beta1.Gateway, list
}
}

portMapping := int32(0)
if gwcc != nil {
portMapping = gwcc.Spec.MapPrivilegedContainerPorts
}

return api.APIGatewayListener{
Name: string(listener.Name),
Hostname: DerefStringOr(listener.Hostname, ""),
Port: int(listener.Port),
Port: ToContainerPort(listener.Port, portMapping),
Protocol: listenerProtocolMap[strings.ToLower(string(listener.Protocol))],
TLS: api.APIGatewayTLSConfiguration{
Certificates: certificates,
},
}, true
}

func ToContainerPort(portNumber gwv1beta1.PortNumber, mapPrivilegedContainerPorts int32) int {
if portNumber >= 1024 {
// We don't care about privileged port-mapping, this is a non-privileged port
return int(portNumber)
}

return int(portNumber) + int(mapPrivilegedContainerPorts)
}

func (t ResourceTranslator) ToHTTPRoute(route gwv1beta1.HTTPRoute, resources *ResourceMap) *api.HTTPRouteConfigEntry {
namespace := t.Namespace(route.Namespace)

Expand Down
2 changes: 1 addition & 1 deletion control-plane/api-gateway/common/translation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestTranslator_ToAPIGateway(t *testing.T) {
resources.ReferenceCountCertificate(listenerOneCert)
resources.ReferenceCountCertificate(listenerTwoCert)

actualConfigEntry := translator.ToAPIGateway(input, resources)
actualConfigEntry := translator.ToAPIGateway(input, resources, &v1alpha1.GatewayClassConfig{})

if diff := cmp.Diff(expectedConfigEntry, actualConfigEntry); diff != "" {
t.Errorf("Translator.GatewayToAPIGateway() mismatch (-want +got):\n%s", diff)
Expand Down
Loading

0 comments on commit 71cdbc2

Please sign in to comment.