Skip to content

Commit

Permalink
haproxy obfuscated internal IP in routing cookie
Browse files Browse the repository at this point in the history
The cookie currently returns the openshift internal pod IP address.
This is a security issue as an attacker can develop a map of the pods
in the cluster just by observing the returned cookie.

This change returns a hash of the internal address and internal service
name to obfuscate the internal information. The service name is configured
when the service is created and is not visible to outside users. This
in combination with the internal ip:port is hashed and presented in the
cookie.

addresses: https://bugzilla.redhat.com/show_bug.cgi?id=1318796
  • Loading branch information
pecameron committed Apr 18, 2016
1 parent fc404cc commit 36c680b
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 6 deletions.
4 changes: 2 additions & 2 deletions images/router/haproxy/conf/haproxy-config.template
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ backend be_edge_http_{{$cfgIdx}}
{{ end }}
http-request set-header Forwarded for=%[src];host=%[req.hdr(host)];proto=%[req.hdr(X-Forwarded-Proto)]
{{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.ID}}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} check inter 5000ms cookie {{$endpoint.IdHash}}
{{ end }}
{{ end }}

Expand All @@ -236,7 +236,7 @@ backend be_secure_{{$cfgIdx}}
timeout check 5000ms
cookie OPENSHIFT_REENCRYPT_{{$cfgIdx}}_SERVERID insert indirect nocache httponly secure
{{ range $idx, $endpoint := endpointsForAlias $cfg $serviceUnit }}
server {{$endpoint.ID}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.ID}}
server {{$endpoint.IdHash}} {{$endpoint.IP}}:{{$endpoint.Port}} ssl check inter 5000ms verify required ca-file {{ $workingDir }}/cacerts/{{$cfgIdx}}.pem cookie {{$endpoint.IdHash}}
{{ end }}
{{ end }}
{{ end }}{{/* $serviceUnit.ServiceAliasConfigs*/}}
Expand Down
9 changes: 9 additions & 0 deletions pkg/router/template/plugin.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package templaterouter

import (
"crypto/md5"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -230,6 +231,14 @@ func createRouterEndpoints(endpoints *kapi.Endpoints, excludeUDP bool) []Endpoin
} else {
ep.TargetName = ep.IP
}

// IdHash contains an obfuscated internal IP address
// that is the value passed in the cookie. The IP address
// is made more difficult to extract by including other
// internal information in the hash.
s := ep.ID + ep.TargetName + ep.PortName
ep.IdHash = fmt.Sprintf("%x", md5.Sum([]byte(s)))

out = append(out, ep)
}
}
Expand Down
10 changes: 6 additions & 4 deletions pkg/router/template/router_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package templaterouter

import (
"crypto/md5"
"fmt"
"testing"

Expand Down Expand Up @@ -47,9 +48,10 @@ func TestAddEndpoints(t *testing.T) {
}

endpoint := Endpoint{
ID: "ep1",
IP: "ip",
Port: "port",
ID: "ep1",
IP: "ip",
Port: "port",
IdHash: fmt.Sprintf("%x", md5.Sum([]byte("ep1ipport"))),
}

router.AddEndpoints(suKey, []Endpoint{endpoint})
Expand All @@ -63,7 +65,7 @@ func TestAddEndpoints(t *testing.T) {
t.Errorf("Expected endpoint table to contain 1 entry")
} else {
actualEp := su.EndpointTable[0]
if endpoint.IP != actualEp.IP || endpoint.Port != actualEp.Port {
if endpoint.IP != actualEp.IP || endpoint.Port != actualEp.Port || endpoint.IdHash != actualEp.IdHash {
t.Errorf("Expected endpoint %v did not match actual endpoint %v", endpoint, actualEp)
}
}
Expand Down
1 change: 1 addition & 0 deletions pkg/router/template/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ type Endpoint struct {
Port string
TargetName string
PortName string
IdHash string
}

// certificateManager provides the ability to write certificates for a ServiceAliasConfig
Expand Down

0 comments on commit 36c680b

Please sign in to comment.