Hi Erik,

sorry for the long delay.

On Thu, May 12, 2016 at 11:26:29AM +0200, Erik Seres wrote:
> If the client provides the server name it intends to connect to, per RFC3546,
> Section 3.1. Server Name Indication, this patch will pass the server name
> onto the backend server as part of the proxy protocol v2 header.
> 
> The patch defines the new SSL subtype PP2_TYPE_SSL_SNI and the corresponding
> flag PP2_CLIENT_SNI to accomplish this in an additional TLV.
> 
> Please review.

I think this is OK technically speaking. And it's something we've indeed been
missing. It is also important to note that even with this there is no way for
haproxy to extract the SNI information from the PP header, but I don't see
how to do this cleanly and efficiently for now.

I just have one minor comment regarding a style issue (which is not yours
but comes from the few lines you copied a few lines above) :

diff --git a/src/connection.c b/src/connection.c
index 330f3ef..f7efac3 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -820,6 +820,11 @@ int make_proxy_line_v2(char *buf, int buf_len, struct 
server *srv, struct connec
                                        ssl_tlv_len += 
make_tlv(&buf[ret+ssl_tlv_len], (buf_len - ret - ssl_tlv_len), PP2_TYPE_SSL_CN, 
cn_trash->len, cn_trash->str);
                                }
                        }
+                       value = ssl_sock_get_servername(remote);
+                       if (value) {
+                               tlv->client |= PP2_CLIENT_SNI;
+                               ssl_tlv_len += make_tlv(&buf[ret+ssl_tlv_len], 
(buf_len-ret-ssl_tlv_len), PP2_TYPE_SSL_SNI, strlen(value), value);
+                       }

Please always put spaces around operators above, that makes expressions
much more readable.

Otherwise I'm fine with merging it.

Regards,
Willy

Reply via email to