Netflix / ribbon

Ribbon is a Inter Process Communication (remote procedure calls) library with built in software load balancers. The primary usage model involves REST calls with various serialization scheme support.
Apache License 2.0
4.57k stars 1.24k forks source link

Using port 443 for https protocol on DynamicServerListLoadBalancer and SimpleHostRoutingFilter #347

Open danielkwinsor opened 7 years ago

danielkwinsor commented 7 years ago

Hi,

[INFO] +- org.springframework.cloud:spring-cloud-starter-zuul:jar:1.3.2.RELEASE:compile

I see a difference in how SimpleHostRoutingFilter and DynamicServerListLoadBalancer handle the https protocol. At first I configured the simple Zuul route as such

  routes:
    simple:
      path: /simple/**
      url: https://test.com

2017-08-23 15:02:24.776 DEBUG 74458 --- [nio-8080-exec-1] o.s.c.n.zuul.filters.SimpleRouteLocator : route matched=ZuulProperties.ZuulRoute(id=simple, path=/simple/**, serviceId=null, url=https://test.com/, stripPrefix=true, retryable=null, sensitiveHeaders=[], customSensitiveHeaders=false)

As expected SimpleHostRoutingFilter sent a request over https to the correct port 443.

Next, I wanted all the great features of Ribbon so I made it a ribbon service, as such

  routes:
    simple:
      path: /httpsPort/**
      serviceId: httpsPort
httpsPort:
  ribbon:
    listOfServers: https://test.com

2017-08-23 15:10:01.378 DEBUG 74458 --- [nio-8080-exec-4] o.s.c.n.zuul.filters.SimpleRouteLocator : route matched=ZuulProperties.ZuulRoute(id= httpsPort, path=/httpsPort/**, serviceId= httpsPort, url=null, stripPrefix=true, retryable=null, sensitiveHeaders=[], customSensitiveHeaders=true)

2017-08-23 15:10:01.665 INFO 74458 --- [nio-8080-exec-4] c.netflix.loadbalancer.BaseLoadBalancer : Client: protocol instantiated a LoadBalancer: DynamicServerListLoadBalancer:{NFLoadBalancer:name= httpsPort,current list of Servers=[],Load balancer stats=Zone stats: {},Server stats: []}ServerList:null

2017-08-23 15:10:01.694 INFO 74458 --- [nio-8080-exec-4] c.n.l.DynamicServerListLoadBalancer : DynamicServerListLoadBalancer for client httpsPort initialized: DynamicServerListLoadBalancer:{NFLoadBalancer:name= httpsPort,current list of Servers=[test.com:80],Load balancer stats=Zone stats: {unknown=[Zone:unknown; Instance count:1; Active connections count: 0; Circuit breaker tripped count: 0; Active connections per server: 0.0;]

Testing showed that https protocol scheme was not utilized, and a request was sent to :80. Things worked as expected intended when listOfServers: test.com:443 So, it wasn't expected that listOfServers with https:// was ignoring the https protocol, and could be a source of stealthy misconfiguration. The 2-part question is whether others see this as an area of improvement, and if so where the code is for a PR? I didn't see much in DynamicServerListLoadBalancer.

brharrington commented 7 years ago

See #275.