Discussion:
[Unbound-users] [PATCH] support for remote control over local sockets
Dag-Erling Smørgrav
2015-01-05 15:37:06 UTC
Permalink
(sounds like an oxymoron, but by "local socket" I mean AF_LOCAL, which
is the correct name for AF_UNIX.)

I just committed a heavily modified version of Ilya Bakulin's patch
(contrib/unbound_unixsock.diff) to FreeBSD 11. I have attached a
version of the patch relative to Unbound 1.5.1. It also applies cleanly
to ***@3302, but I have not tested the result.

Here is a summary:

Add support for using a local socket for the remote control connection
by specifying its path instead of (or in addition to) an IP address as
an argument to the control-interface configuration variable.

Add support for unencrypted and unauthenticated control connections
through a new configuration variable, control-use-cert. To avoid the
complexity of supporting both SSL socket and plain socket descriptors
in the same code, we just use an unencrypted SSL context and forego
authentication. The downside is that we still have to perform DH kex
when establishing the connection.

This patch was derived (with significant modifications) from the
contrib/unbound_unixsock.diff patch originally submitted by Ilya
Bakulin of Genua mbH.

Note that my patch does not update generated files, so remember to run
autoreconf and regenerate the configuration parser and lexer.

Genua have already released Ilya's part of the patch under the BSD
license. I release my version under the same license.

DES
--
Dag-Erling SmÞrgrav - ***@des.no
W.C.A. Wijngaards
2015-01-06 13:28:41 UTC
Permalink
Hi Dag-Erling,
Post by Dag-Erling Smørgrav
(sounds like an oxymoron, but by "local socket" I mean AF_LOCAL,
which is the correct name for AF_UNIX.)
I just committed a heavily modified version of Ilya Bakulin's
patch (contrib/unbound_unixsock.diff) to FreeBSD 11. I have
attached a version of the patch relative to Unbound 1.5.1. It also
Thank you for the patch, it looks very good, and I'll put its
inclusion on todo (I need a bit more time to spend on looking it over).

Best regards,
Wouter
Post by Dag-Erling Smørgrav
Add support for using a local socket for the remote control
connection by specifying its path instead of (or in addition to) an
IP address as an argument to the control-interface configuration
variable.
Add support for unencrypted and unauthenticated control
connections through a new configuration variable, control-use-cert.
To avoid the complexity of supporting both SSL socket and plain
socket descriptors in the same code, we just use an unencrypted SSL
context and forego authentication. The downside is that we still
have to perform DH kex when establishing the connection.
This patch was derived (with significant modifications) from the
contrib/unbound_unixsock.diff patch originally submitted by Ilya
Bakulin of Genua mbH.
Note that my patch does not update generated files, so remember to
run autoreconf and regenerate the configuration parser and lexer.
Genua have already released Ilya's part of the patch under the BSD
license. I release my version under the same license.
DES
_______________________________________________ Unbound-users
http://unbound.nlnetlabs.nl/mailman/listinfo/unbound-users
Dag-Erling Smørgrav
2015-01-12 07:08:09 UTC
Permalink
Thank you for committing the patch.

In r3308, you made this change in two places:

- sun.sun_len = sizeof(sun);
+ sun.sun_len = (sa_family_t)sizeof(sun);

I believe the cast should be to socklen_t, not sa_family_t.

DES
--
Dag-Erling Smørgrav - ***@des.no
W.C.A. Wijngaards
2015-01-12 08:22:38 UTC
Permalink
Hi Dag-Erling,
Post by Dag-Erling Smørgrav
Thank you for committing the patch.
- sun.sun_len = sizeof(sun); + sun.sun_len =
(sa_family_t)sizeof(sun);
I believe the cast should be to socklen_t, not sa_family_t.
OK, done.

Best regards,
Wouter
Dag-Erling Smørgrav
2015-01-12 09:19:14 UTC
Permalink
Post by W.C.A. Wijngaards
Post by Dag-Erling Smørgrav
I believe the cast should be to socklen_t, not sa_family_t.
OK, done.
Wow, that was quick. I'm actually not 100% sure this is correct either:
sun_len (not in POSIX) is char while socklen_t is required by POSIX to
be 32 bits. But it's no less wrong than the original, since on systems
where sockaddrs have a length field, it is used to pass around the same
values that POSIX uses socklen_t for.

In any case, the cast is not really necessary. There are many needless
casts in unbound, which I assume are the result of running a simplistic
lint on the code and trying to fix everything it flags instead of tuning
it to reduce false positives.

DES
--
Dag-Erling Smørgrav - ***@des.no
Loading...