[Openvpn-devel,v2,2/2] port-share: Add proxy protocol v2 support
Commit Message
In addition to the custom journal solution, also support the widely
used binary PROXY protocol version 2 to convey the original client
connection parameters to the proxy receiver. This makes the port-share
journal feature more accessable and easier to use, because one doesn't
need a custom integration.
While this is a spec-compliant sender implementation of the PROXY
protocol, it does not implement it in full. Version 1 was left out
entirely, in favour of the superior and easier-to-implement version 2.
The implementation was also kept minimal with regards to what OpenVPN
supports/requires: Local commands, unix sockets, UDP and TLVs are not
implemented.
Signed-off-by: Corubba Smith <corubba@gmx.de>
---
doc/man-sections/server-options.rst | 4 +
src/openvpn/ps.c | 110 +++++++++++++++++++++++++++-
2 files changed, 113 insertions(+), 1 deletion(-)
--
2.47.1
Comments
Hi,
On Mon, Dec 16, 2024 at 01:22:51PM +0100, corubba via Openvpn-devel wrote:
> In addition to the custom journal solution, also support the widely
> used binary PROXY protocol version 2 to convey the original client
> connection parameters to the proxy receiver. This makes the port-share
> journal feature more accessable and easier to use, because one doesn't
> need a custom integration.
>
> While this is a spec-compliant sender implementation of the PROXY
> protocol, it does not implement it in full. Version 1 was left out
> entirely, in favour of the superior and easier-to-implement version 2.
> The implementation was also kept minimal with regards to what OpenVPN
> supports/requires: Local commands, unix sockets, UDP and TLVs are not
> implemented.
Are there any reference URLs for the PROXY protocol? If yes, I think it
would be good to include a reference into one of the comments - the next
person to read that code would then know which document this is based on.
Besides this, I'm not sure what to do with these
> + ASSERT(4 >= sizeof(src.addr.in4.sin_addr));
> + ASSERT(4 >= sizeof(dst.addr.in4.sin_addr));
> + memcpy(&header[16], &src.addr.in4.sin_addr, sizeof(src.addr.in4.sin_addr));
> + memcpy(&header[20], &dst.addr.in4.sin_addr, sizeof(dst.addr.in4.sin_addr));
... this is all obviously correct, but it feels heavy handed, and many
lines of code... (especially 2x sizeof on the same structure element)
I think we can fairly safely assume that an in4.sin_addr will always be
4 bytes, an in4.sin_port will always be 2 bytes, and an IPv6 address will
always be 16 bytes... so I wonder if it wouln't be easier to just copy
a well-known number of bytes (openvpn_sockaddr is also "always bigger
than that", so no read overrun) here.
I know that "magic numbers" in code are considered a bad pattern, but
these things are really constant length, and the buffer layout *requires*
"exactly 4" and "exactly 2" bytes...
gert
Hi,
On 26.12.24 14:13, Gert Doering wrote:
> On Mon, Dec 16, 2024 at 01:22:51PM +0100, corubba via Openvpn-devel wrote:
>> In addition to the custom journal solution, also support the widely
>> used binary PROXY protocol version 2 to convey the original client
>> connection parameters to the proxy receiver. This makes the port-share
>> journal feature more accessable and easier to use, because one doesn't
>> need a custom integration.
>>
>> While this is a spec-compliant sender implementation of the PROXY
>> protocol, it does not implement it in full. Version 1 was left out
>> entirely, in favour of the superior and easier-to-implement version 2.
>> The implementation was also kept minimal with regards to what OpenVPN
>> supports/requires: Local commands, unix sockets, UDP and TLVs are not
>> implemented.
>
> Are there any reference URLs for the PROXY protocol? If yes, I think it
> would be good to include a reference into one of the comments - the next
> person to read that code would then know which document this is based on.
I had included the link to the spec I used only in the cover letter, but
I agree putting it into the commit message is more sensible.
> Besides this, I'm not sure what to do with these
>
>> + ASSERT(4 >= sizeof(src.addr.in4.sin_addr));
>> + ASSERT(4 >= sizeof(dst.addr.in4.sin_addr));
>> + memcpy(&header[16], &src.addr.in4.sin_addr, sizeof(src.addr.in4.sin_addr));
>> + memcpy(&header[20], &dst.addr.in4.sin_addr, sizeof(dst.addr.in4.sin_addr));
>
> ... this is all obviously correct, but it feels heavy handed, and many
> lines of code... (especially 2x sizeof on the same structure element)
>
> I think we can fairly safely assume that an in4.sin_addr will always be
> 4 bytes, an in4.sin_port will always be 2 bytes, and an IPv6 address will
> always be 16 bytes... so I wonder if it wouln't be easier to just copy
> a well-known number of bytes (openvpn_sockaddr is also "always bigger
> than that", so no read overrun) here.
>
> I know that "magic numbers" in code are considered a bad pattern, but
> these things are really constant length, and the buffer layout *requires*
> "exactly 4" and "exactly 2" bytes...
I wasn't sure about that either, and in the end opted for the "better
safe than sorry" variant. If your better-informed opinion too is that
these are "practically constant", then I am happy to oblige.
Best regards
--
Corubba
@@ -435,6 +435,10 @@ fast hardware. SSL/TLS authentication must be used in this mode.
the origin of the connection. Each generated file will be automatically
deleted when the proxied connection is torn down.
+ ``dir`` can be set to the special value ``proxy_protocol_v2`` to make
+ OpenVPN use the binary PROXY protocol version 2 towards the proxy receiver.
+ No temporary files will be written in this mode.
+
Not implemented on Windows.
--push option
@@ -393,6 +393,107 @@ journal_add(const char *journal_dir, struct proxy_connection *pc, struct proxy_c
gc_free(&gc);
}
+/*
+ * Send the proxy protocol v2 binary header, so that the receiving
+ * server knows the true client connection parameters.
+ */
+static void
+send_proxy_protocol_v2_header(const struct proxy_connection *const pc, const struct proxy_connection *const cp)
+{
+ static const uint8_t PP2_AF_UNSPEC = 0x0, PP2_AF_INET = 0x1, PP2_AF_INET6 = 0x2;
+ static const uint8_t PP2_PROTO_STREAM = 0x1;
+
+ struct openvpn_sockaddr src, dst;
+ socklen_t src_len, dst_len;
+ unsigned char header[52] = {
+ "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A" /* signature */
+ "\x21" /* version=2 + command=proxy */
+ /* initialize the rest to zero for now */
+ };
+ uint8_t addr_fam, header_len = 16;
+ uint16_t addr_len;
+
+ src_len = sizeof(src.addr);
+ dst_len = sizeof(dst.addr);
+ if (0 != getpeername(pc->sd, &src.addr.sa, &src_len)
+ || 0 != getsockname(pc->sd, &dst.addr.sa, &dst_len))
+ {
+ msg(M_WARN, "PORT SHARE PROXY: getting client connection parameters failed");
+ src.addr.sa.sa_family = dst.addr.sa.sa_family = AF_UNSPEC;
+ }
+
+ transform_mapped_v4_sockaddr(&src);
+ transform_mapped_v4_sockaddr(&dst);
+ if (src.addr.sa.sa_family != dst.addr.sa.sa_family)
+ {
+ msg(M_WARN, "PORT SHARE PROXY: address family mismatch between peer and socket");
+ /* src wins, because that is usually the more important info */
+ dst.addr.sa.sa_family = src.addr.sa.sa_family;
+ }
+
+ if (msg_test(D_PS_PROXY_DEBUG))
+ {
+ struct gc_arena gc = gc_new();
+ dmsg(D_PS_PROXY_DEBUG, "PORT SHARE PROXY: client connection is %s -> %s",
+ print_openvpn_sockaddr(&src, &gc), print_openvpn_sockaddr(&dst, &gc));
+ gc_free(&gc);
+ }
+
+ switch (src.addr.sa.sa_family)
+ {
+ case AF_INET:
+ addr_fam = PP2_AF_INET;
+ addr_len = 12;
+ ASSERT(4 >= sizeof(src.addr.in4.sin_addr));
+ ASSERT(4 >= sizeof(dst.addr.in4.sin_addr));
+ memcpy(&header[16], &src.addr.in4.sin_addr, sizeof(src.addr.in4.sin_addr));
+ memcpy(&header[20], &dst.addr.in4.sin_addr, sizeof(dst.addr.in4.sin_addr));
+ ASSERT(2 >= sizeof(src.addr.in4.sin_port));
+ ASSERT(2 >= sizeof(dst.addr.in4.sin_port));
+ memcpy(&header[24], &src.addr.in4.sin_port, sizeof(src.addr.in4.sin_port));
+ memcpy(&header[26], &dst.addr.in4.sin_port, sizeof(dst.addr.in4.sin_port));
+ break;
+
+ case AF_INET6:
+ addr_fam = PP2_AF_INET6;
+ addr_len = 36;
+ ASSERT(16 >= sizeof(src.addr.in6.sin6_addr));
+ ASSERT(16 >= sizeof(dst.addr.in6.sin6_addr));
+ memcpy(&header[16], &src.addr.in6.sin6_addr, sizeof(src.addr.in6.sin6_addr));
+ memcpy(&header[32], &dst.addr.in6.sin6_addr, sizeof(dst.addr.in6.sin6_addr));
+ ASSERT(2 >= sizeof(src.addr.in6.sin6_port));
+ ASSERT(2 >= sizeof(dst.addr.in6.sin6_port));
+ memcpy(&header[48], &src.addr.in6.sin6_port, sizeof(src.addr.in6.sin6_port));
+ memcpy(&header[50], &dst.addr.in6.sin6_port, sizeof(dst.addr.in6.sin6_port));
+ break;
+
+ /* AF_UNIX is currently not suppported by OpenVPN */
+
+ default:
+ addr_fam = PP2_AF_UNSPEC;
+ addr_len = 0;
+ break;
+ }
+
+ const uint8_t proto = PP2_PROTO_STREAM; /* DGRAM is currently not supported by port-share */
+ header[13] = (addr_fam << 4) | proto;
+
+ /* TLV is currently not implemented */
+
+ header_len += addr_len;
+ const uint16_t addr_len_n = htons(addr_len);
+ memcpy(&header[14], &addr_len_n, sizeof(addr_len_n));
+
+ ASSERT(header_len <= sizeof(header));
+ const socket_descriptor_t sd = cp->sd;
+ const int status = send(sd, header, header_len, MSG_NOSIGNAL);
+ dmsg(D_PS_PROXY_DEBUG, "PORT SHARE PROXY: proxy protocol v2 wrote[%d] %d", (int) sd, status);
+ if (status < (int) header_len)
+ {
+ msg(M_WARN, "PORT SHARE PROXY: failed to send proxy protocol v2 header");
+ }
+}
+
/*
* Cleanup function, on proxy process exit.
*/
@@ -488,7 +589,14 @@ proxy_entry_new(struct proxy_connection **list,
/* add journal entry */
if (journal_dir)
{
- journal_add(journal_dir, pc, cp);
+ if (0 == strcmp("proxy_protocol_v2", journal_dir))
+ {
+ send_proxy_protocol_v2_header(pc, cp);
+ }
+ else
+ {
+ journal_add(journal_dir, pc, cp);
+ }
}
dmsg(D_PS_PROXY_DEBUG, "PORT SHARE PROXY: NEW CONNECTION [c=%d s=%d]", (int)sd_client, (int)sd_server);