Skip to content

io: tls: support SNI routing for outputs#1313

Merged
edsiper merged 1 commit intofluent:masterfrom
andreykaipov:feature/sni
Sep 25, 2019
Merged

io: tls: support SNI routing for outputs#1313
edsiper merged 1 commit intofluent:masterfrom
andreykaipov:feature/sni

Conversation

@andreykaipov
Copy link
Copy Markdown
Contributor

Addresses #1312. Please let me know what you guys think! :-)

@andreykaipov
Copy link
Copy Markdown
Contributor Author

Hi @edsiper - what do you think about this addition? 😄 The use case for us is to route logs to different sources in a locked-down environment with a limited port range.

I can add a docs PR soon if that's what missing.

@edsiper
Copy link
Copy Markdown
Member

edsiper commented Jun 11, 2019

thanks for the contribution.

How can I test the changes/feature ?

@andreykaipov
Copy link
Copy Markdown
Contributor Author

So to test this feature out, we first need to see SNI routing in action. The setup for this is a bit involved, but I've created an example using Docker, so I hope it's straightforward to reproduce.

We'll stand up two TLS-enabled servers behind one load balancer.

Create a network for our containers to share:

└▶ docker network create sni

Create a script called tls-server.sh that generates a dummy cert/key pair and starts up a TLS server:

apk add -U openssl
app="$1"; shift
port="$1"; shift
openssl req -x509 -nodes -newkey rsa:2048 -keyout "$app.pem" -out "$app.crt" -subj "/CN=$app"
openssl s_server -accept "$port" -key "$app.pem" -cert "$app.crt" -quiet

In two separate terminals, start up two containers called app1 and app2, both running a TLS server on ports 1515 and 1516 respectively:

└▶ docker run --rm --tty --network sni --name app1 -v $PWD:/tmp alpine sh /tmp/tls-server.sh app1 1515
└▶ docker run --rm --tty --network sni --name app2 -v $PWD:/tmp alpine sh /tmp/tls-server.sh app2 1516

For the load balancer in front of these guys, I've used HAProxy, but it can be anything that supports SNI routing. Create the following haproxy.cfg:

defaults
  timeout server 10s
  timeout client 10s
  timeout connect 10s

frontend 8443
  bind *:8443
  mode tcp
  tcp-request inspect-delay 5s
  tcp-request content accept if { req_ssl_hello_type 1 }
  use_backend app1 if { req_ssl_sni -i app1 }
  use_backend app2 if { req_ssl_sni -i app2 }

backend app1
  mode tcp
  server pod app1:1515

backend app2
  mode tcp
  server pod app2:1516

Finally, start up HAProxy on the same network, using the above config:

└▶ docker run --rm --network sni -v $PWD:/tmp -p 8443:8443 haproxy:1.9-alpine -- /tmp/haproxy.cfg

Now we're ready to test it out! Note how even though localhost:8443 doesn't serve any kind of certificate, if we specify the server name we want to connect to, HAProxy will route our request appropriately:

└▶ openssl s_client -connect localhost:8443 </dev/null 2>/dev/null | openssl x509 -noout -subject
unable to load certificate
└▶ openssl s_client -connect localhost:8443 -servername app1 </dev/null 2>/dev/null | openssl x509 -noout -subject
subject= /CN=app1
└▶ openssl s_client -connect localhost:8443 -servername app2 </dev/null 2>/dev/null | openssl x509 -noout -subject
subject= /CN=app2

To reel it back in, this PR adds the ability for Fluent Bit to specify the server name through the new tls.vhost parameter.

With our above system still running, a sample Fluent Bit config to ship bananas to app1 and oranges to app2 might look like this:

[INPUT]
    Name    exec
    Tag     app1
    Command echo bananas

[INPUT]
    Name    exec
    Tag     app2
    Command echo oranges

[OUTPUT]
    Name       forward
    Match      app1
    Host       127.0.0.1
    Port       8443
    tls        On
    tls.verify Off
    tls.vhost  app1

[OUTPUT]
    Name       forward
    Match      app2
    Host       127.0.0.1
    Port       8443
    tls        On
    tls.verify Off
    tls.vhost  app2

Running Fluent Bit with this config, we'll find the routing works as expected! 🎉

@fujimotos
Copy link
Copy Markdown
Contributor

@edsiper @andreykaipov

While reviewing the code, I noticed a subtle conflict with the recent change made
to flb_output.c (details posted in the review comment above). It needs to be
resolved to be merged into the master.

Other than that point, this patch works good. I can confirm sni-routing is working
fine using the haproxy + fluentd + fluent-bit stack.

@edsiper
Copy link
Copy Markdown
Member

edsiper commented Sep 24, 2019

the variable tmp is flb_sds_t type now, so you need to destroy it with flb_sds_destroy() instead of flb_free(). Please adjust that change so we can merge the contribution

@edsiper edsiper added the waiting-for-user Waiting for more information, tests or requested changes label Sep 24, 2019
Comment thread src/flb_output.c Outdated
Addresses #1312.

When sending logs to TLS-enabled outputs, it may be ideal to route
based on a virtual hostname of the origin server. This is possible
with SNI. This commit adds a new setting `tls.vhost` for the secure
forwarding mode. By default, the virtual hostname will be set to
the origin hostname during the TLS handshake.

Signed-off-by: Andrey Kaipov <andreykaipov@users.noreply.github.com>
@andreykaipov
Copy link
Copy Markdown
Contributor Author

andreykaipov commented Sep 25, 2019

Hi @edsiper and @fujimotos - thanks for taking another look at this PR and confirming it works. I updated the offending line. Please lemme know if there's anything else I can do to get this merged in.

edit: The Windows AppVeyor build failed. :-( Looking at the history, it seems like I'm not the only one.

@fujimotos
Copy link
Copy Markdown
Contributor

FWIW I fixed the testing failure occurring on AppVeyor in #1590.

I think this patch should be ready to be merged now.

@edsiper edsiper merged commit dd405e0 into fluent:master Sep 25, 2019
@edsiper
Copy link
Copy Markdown
Member

edsiper commented Sep 25, 2019

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-for-user Waiting for more information, tests or requested changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants