| Message ID | CAJXQW9Vzf_ypQGSez8X96imiy2Hat0buoY2fp3gmsYs1EXdcpg@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Allow DNS autoconf by passing hostname by IV variables when using push-peer-info | expand |
Hi, On Sun, Sep 18, 2022 at 10:23:14AM +0900, Ricardo Manriquez wrote: > + > + char hostname[64]; > + gethostname(hostname, 63); > + buf_printf(&out, "IV_HOSTNAME=%s\n", hostname ); Without entering the discussion if this is a useful addition, the implementation definitely lacks error handling - gethostname() can fail, and then we have an unintialized buffer passed to buf_printf(). gert
Am 18.09.2022 um 03:23 schrieb Ricardo Manriquez: > Author: Ricardo Manríquez <ricardo.manriquez@gmail.com> > > To enable the possibility of DNS autoconfiguration the IP address and > hostname of the client are needed to register at the DNS level, this > patch adds this information when using push-peer-info. > > The motivation is that the domain name is as intrusive as the MAC > address and DNS autoconfiguration is helpful to be able to communicate > back to the clients, this generates a problem when the client connects > to the network directly and then uses the VPN connection, now the DNS > records do not match and when using remote assistance or remote > management tools the benefits of DNS are negated. Could you expain why this needs to be in OpenVPN itself and cannot be done with something like starting openvpn with an additional parameter like --setenv UV_HOSTNAME "$(hostname)" or derived from another parameter/variable from the client like CN, username etc? Space in the packet carrying IV_/UV_ variables is already limited and I am not sure if spending another 64 for the hostname is a good thing. > --- > src/openvpn/ssl.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c > index 80e0d5acb4..3031566585 100644 > --- a/src/openvpn/ssl.c > +++ b/src/openvpn/ssl.c > @@ -2321,6 +2321,11 @@ push_peer_info(struct buffer *buf, struct tls_session *session) > { > buf_printf(&out, "IV_HWADDR=%s\n", format_hex_ex(rgi.hwaddr, 6, 0, 1, ":", &gc)); > } > + > + char hostname[64]; > + gethostname(hostname, 63); > + buf_printf(&out, "IV_HOSTNAME=%s\n", hostname ); Isn't there a MAX_HOSTNAME define or similar instead of hardcoding 64 here? The handling of the string of hostname is not very well here. The man page of the function (gethostname(2) - Linux manual page (man7.org) <https://man7.org/linux/man-pages/man2/sethostname.2.html>) says null termination is not guaranteed for long hostnames. > + > buf_printf(&out, "IV_SSL=%s\n", get_ssl_library_version() ); > #if defined(_WIN32) > buf_printf(&out, "IV_PLAT_VER=%s\n", win32_version_string(&gc, false)); > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c index 80e0d5acb4..3031566585 100644 --- a/src/openvpn/ssl.c +++ b/src/openvpn/ssl.c @@ -2321,6 +2321,11 @@ push_peer_info(struct buffer *buf, struct tls_session *session) { buf_printf(&out, "IV_HWADDR=%s\n", format_hex_ex(rgi.hwaddr, 6, 0, 1, ":", &gc)); } + + char hostname[64]; + gethostname(hostname, 63); + buf_printf(&out, "IV_HOSTNAME=%s\n", hostname ); + buf_printf(&out, "IV_SSL=%s\n", get_ssl_library_version() );
Author: Ricardo Manríquez <ricardo.manriquez@gmail.com> To enable the possibility of DNS autoconfiguration the IP address and hostname of the client are needed to register at the DNS level, this patch adds this information when using push-peer-info. The motivation is that the domain name is as intrusive as the MAC address and DNS autoconfiguration is helpful to be able to communicate back to the clients, this generates a problem when the client connects to the network directly and then uses the VPN connection, now the DNS records do not match and when using remote assistance or remote management tools the benefits of DNS are negated. Signed-off-by: Ricardo Manríquez <ricardo.manriquez@gmail.com> --- src/openvpn/ssl.c | 5 +++++ 1 file changed, 5 insertions(+) #if defined(_WIN32) buf_printf(&out, "IV_PLAT_VER=%s\n", win32_version_string(&gc, false));