Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IP in logs for https traffic #690

Merged
merged 3 commits into from
May 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions controllers/nginx/pkg/cmd/controller/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ func newNGINXController() ingress.Controller {
resolver: h,
proxy: &proxy{
Default: &server{
Hostname: "localhost",
IP: "127.0.0.1",
Port: 442,
Hostname: "localhost",
IP: "127.0.0.1",
Port: 442,
ProxyProtocol: true,
},
},
Expand Down Expand Up @@ -534,9 +534,9 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, er

//TODO: Allow PassthroughBackends to specify they support proxy-protocol
servers = append(servers, &server{
Hostname: pb.Hostname,
IP: svc.Spec.ClusterIP,
Port: port,
Hostname: pb.Hostname,
IP: svc.Spec.ClusterIP,
Port: port,
ProxyProtocol: false,
})
}
Expand Down
23 changes: 10 additions & 13 deletions controllers/nginx/pkg/cmd/controller/tcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import (
)

type server struct {
Hostname string
IP string
Port int
Hostname string
IP string
Port int
ProxyProtocol bool
}

Expand Down Expand Up @@ -41,19 +41,16 @@ func (p *proxy) Handle(conn net.Conn) {
return
}

var proxy *server
proxy := p.Default
hostname, err := parser.GetHostname(data[:])
if err == nil {
glog.V(3).Infof("parsed hostname from TLS Client Hello: %s", hostname)
glog.V(4).Infof("parsed hostname from TLS Client Hello: %s", hostname)
proxy = p.Get(hostname)
if proxy == nil {
return
}
} else {
proxy = p.Default
if proxy == nil {
return
}
}

if proxy == nil {
glog.V(4).Infof("there is no configured proxy for SSL connections")
return
}

clientConn, err := net.Dial("tcp", fmt.Sprintf("%s:%d", proxy.IP, proxy.Port))
Expand Down
7 changes: 2 additions & 5 deletions controllers/nginx/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const (

gzipTypes = "application/atom+xml application/javascript application/x-javascript application/json application/rss+xml application/vnd.ms-fontobject application/x-font-ttf application/x-web-app-manifest+json application/xhtml+xml application/xml font/opentype image/svg+xml image/x-icon text/css text/plain text/x-component"

logFormatUpstream = `%v - [$proxy_add_x_forwarded_for] - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status`
logFormatUpstream = `%v - [$the_x_forwarded_for] - $remote_user [$time_local] "$request" $status $body_bytes_sent "$http_referer" "$http_user_agent" $request_length $request_time [$proxy_upstream_name] $upstream_addr $upstream_response_length $upstream_response_time $upstream_status`

logFormatStream = `[$time_local] $protocol $status $bytes_sent $bytes_received $session_time`

Expand Down Expand Up @@ -332,10 +332,7 @@ func NewDefault() Configuration {
// is enabled.
func (cfg Configuration) BuildLogFormatUpstream() string {
if cfg.LogFormatUpstream == logFormatUpstream {
if cfg.UseProxyProtocol {
return fmt.Sprintf(cfg.LogFormatUpstream, "$proxy_protocol_addr")
}
return fmt.Sprintf(cfg.LogFormatUpstream, "$remote_addr")
return fmt.Sprintf(cfg.LogFormatUpstream, "$the_x_forwarded_for")
}

return cfg.LogFormatUpstream
Expand Down
4 changes: 2 additions & 2 deletions controllers/nginx/pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ func TestBuildLogFormatUpstream(t *testing.T) {
curLogFormat string
expected string
}{
{true, logFormatUpstream, fmt.Sprintf(logFormatUpstream, "$proxy_protocol_addr")},
{false, logFormatUpstream, fmt.Sprintf(logFormatUpstream, "$remote_addr")},
{true, logFormatUpstream, fmt.Sprintf(logFormatUpstream, "$the_x_forwarded_for")},
{false, logFormatUpstream, fmt.Sprintf(logFormatUpstream, "$the_x_forwarded_for")},
{true, "my-log-format", "my-log-format"},
{false, "john-log-format", "john-log-format"},
}
Expand Down
3 changes: 1 addition & 2 deletions controllers/nginx/pkg/template/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func TestMergeConfigMapToStruct(t *testing.T) {
}

func TestDefaultLoadBalance(t *testing.T) {
conf := map[string]string{
}
conf := map[string]string{}
to := ReadConfig(conf)
if to.LoadBalanceAlgorithm != "least_conn" {
t.Errorf("default load balance algorithm wrong")
Expand Down
16 changes: 15 additions & 1 deletion controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ http {

server_tokens {{ if $cfg.ShowServerTokens }}on{{ else }}off{{ end }};

# disable warnings
uninitialized_variable_warn off;

log_format upstreaminfo '{{ buildLogFormatUpstream $cfg }}';

{{/* map urls that should not appear in access.log */}}
Expand Down Expand Up @@ -127,6 +130,16 @@ http {
'' $server_port;
}

map $pass_access_scheme $the_x_forwarded_for {
default $remote_addr;
https $proxy_protocol_addr;
}

map $pass_access_scheme $the_real_ip {
default $remote_addr;
https $proxy_protocol_addr;
}

# map port 442 to 443 for header X-Forwarded-Port
map $pass_server_port $pass_port {
442 443;
Expand Down Expand Up @@ -352,7 +365,8 @@ http {
proxy_set_header Upgrade $http_upgrade;
proxy_set_header Connection $connection_upgrade;

proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Real-IP $the_real_ip;
proxy_set_header X-Forwarded-For $the_x_forwarded_for;
proxy_set_header X-Forwarded-Host $best_http_host;
proxy_set_header X-Forwarded-Port $pass_port;
proxy_set_header X-Forwarded-Proto $pass_access_scheme;
Expand Down
3 changes: 1 addition & 2 deletions core/pkg/ingress/controller/backend_ssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ func (ic *GenericController) syncSecret() {
var cert *ingress.SSLCert
var err error

keys := ic.secretTracker.List()
for _, k := range keys {
for _, k := range ic.secretTracker.List() {
key := k.(string)
cert, err = ic.getPemCertificate(key)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions core/pkg/ingress/controller/backend_ssl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"fmt"

meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
testclient "k8s.io/client-go/kubernetes/fake"
api_v1 "k8s.io/client-go/pkg/api/v1"
Expand Down
19 changes: 16 additions & 3 deletions core/pkg/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,16 @@ func newIngressController(config *Configuration) *GenericController {
}

secrEventHandler := cache.ResourceEventHandlerFuncs{
UpdateFunc: func(old, cur interface{}) {
if !reflect.DeepEqual(old, cur) {
ic.syncSecret()
}
},
DeleteFunc: func(obj interface{}) {
sec := obj.(*api.Secret)
ic.sslCertTracker.Delete(fmt.Sprintf("%v/%v", sec.Namespace, sec.Name))
key := fmt.Sprintf("%v/%v", sec.Namespace, sec.Name)
ic.sslCertTracker.Delete(key)
ic.secretTracker.Delete(key)
},
}

Expand Down Expand Up @@ -1007,9 +1014,11 @@ func (ic *GenericController) createServers(data []interface{},
} else {
glog.Warningf("ssl certificate %v does not contain a common name for host %v", key, host)
}
} else {
glog.Warningf("ssl certificate \"%v\" does not exist in local store", key)

continue
}

glog.Infof("ssl certificate \"%v\" does not exist in local store", key)
}
}
}
Expand Down Expand Up @@ -1151,6 +1160,10 @@ func (ic GenericController) extractSecretNames(ing *extensions.Ingress) {
}

for _, tls := range ing.Spec.TLS {
if tls.SecretName == "" {
continue
}

key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName)
_, exists := ic.secretTracker.Get(key)
if !exists {
Expand Down