freeradius-client with IPv6 and DTLS
Hello, I've modified the freeradius-client to simplify its API, remove all IPv4-only legacy code and add DTLS support (rfc7360). The DTLS support I've tried to add in a transparent way, i.e., an old client could still have basic DTLS support only by modifying its configuration file. This is currently experimental code and most likely will have bugs, but I'd like to see any comments on the approach or possible improvements. My main concern is that there have been no releases of freeradius-client for many years, although the intention is to send back the changes as pull requests to the original repository once they stabilize. regards, Nikos The code: https://github.com/nmav/radiusclient-dtls Changes since the last release of freeradius-client: https://github.com/nmav/radiusclient-dtls/blob/master/CHANGES
On Dec 22, 2014, at 7:11 AM, Nikos Mavrogiannopoulos <nmav@gnutls.org> wrote:
I've modified the freeradius-client to simplify its API, remove all IPv4-only legacy code and add DTLS support (rfc7360).
That’s a lot of work. I’ve avoided doing that because the code in the freeradius-server implements almost everything RADIUS, and I don’t want to duplicate effort.
The DTLS support I've tried to add in a transparent way, i.e., an old client could still have basic DTLS support only by modifying its configuration file. This is currently experimental code and most likely will have bugs, but I'd like to see any comments on the approach or possible improvements.
As always.
My main concern is that there have been no releases of freeradius-client for many years,
Because there’s been little need. The existing users of the code haven’t requested changes. They need a basic RADIUS library, and not much more. The code does that.
although the intention is to send back the changes as pull requests to the original repository once they stabilize.
Arran and I have been working on merging your changes back into the main repository. The points of disagreement are largely code style, methods, etc. There are sometimes things which are easy to do, but which cause problems for portability, maintainability, etc. As an example, the lack of a “configure” script in the repository is an issue. We’ve had that discussion on the github issues page, but here’s the side effect: Mac OSX: $ ./autogen.sh .. .configure.ac:32: error: possibly undefined macro: AC_PROG_LIBTOOL If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. configure.ac:38: error: possibly undefined macro: AM_PROG_LIBTOOL autoreconf: /usr/local/Cellar/autoconf/2.69/bin/autoconf failed with exit status: 1 Ubuntu 14.04: $ ./autogen.sh … $ ./configure … configure: creating ./config.status config.status: error: cannot find input file: `Makefile.in’ I would suggest regularly checking out the repository into a separate directory, and seeing if it builds. Or, updating the Makefile rules so that a “make distclean” will really clean everything. I’ll take a look at the patches. My guess is that most of them can be pulled over as-is. If it’s all right with you, I’d like to pull over all of the IPv6 fixes, and then do a release. We can leave the DTLS code as experimental, and add it to a later release. Alan DeKok.
On Mon, 2014-12-22 at 10:35 -0500, Alan DeKok wrote:
On Dec 22, 2014, at 7:11 AM, Nikos Mavrogiannopoulos <nmav@gnutls.org> wrote: > I've modified the freeradius-client to simplify its API, remove all > IPv4-only legacy code and add DTLS support (rfc7360). That’s a lot of work. I’ve avoided doing that because the code in the freeradius-server implements almost everything RADIUS, and I don’t want to duplicate effort.
Hi, The advantage of freeradius-client is that it has a very easy high level API and could be used with a simple config file without a low level knowledge of the radius protocol. That is why I preferred to modify that library, and the changes for DTLS weren't that large actually. They simply wrap over the sendto() and recvfrom(). The only tricky part is that they serialize every communication over a single DTLS session, and that would affect multi-threaded programs using that library, as the sendto(), recvfrom() need to be atomic. I introduced locks to solve that, but a proper multiplexer could be a better solution for such programs. Unrelated: If I can mention a small omission in the DTLS rfc is that it doesn't specify a key purpose in the certificates to be used for radius. For web servers the "TLS WWW server authentication" OID is used in extended key usage, but there is no equivalent for DTLS radius servers.
although the intention is to send back the changes as > pull requests to the original repository once they stabilize.
Arran and I have been working on merging your changes back into the main repository. The points of disagreement are largely code style, methods, etc. There are sometimes things which are easy to do, but which cause problems for portability, maintainability, etc.
Thanks.
As an example, the lack of a “configure” script in the repository is an issue. We’ve had that discussion on the github issues page, but here’s the side effect:
Mac OSX:
$ ./autogen.sh .. .configure.ac:32: error: possibly undefined macro: AC_PROG_LIBTOOL If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. configure.ac:38: error: possibly undefined macro: AM_PROG_LIBTOOL autoreconf: /usr/local/Cellar/autoconf/2.69/bin/autoconf failed with exit status: 1
I'd really prefer to fix any issues that result of that change instead of having the auto-generated files included. It is very cumbersome to work with a repository which has 20 files modified because I changed something in a Makefile.am. I've almost forgot to add all new files because git status shows so many changes it is difficult to distinguish what matters or not. Indeed I could run make distclean before every git related command, but that is very inconvenient. In any case, that removal doesn't affect the patch set, as the set of files involved are distinct.
$ ./autogen.sh … $ ./configure … configure: creating ./config.status config.status: error: cannot find input file: `Makefile.in’
The autogen.sh run has failed for some reason in that platform. It's best to fix that anyway.
I would suggest regularly checking out the repository into a separate directory, and seeing if it builds. Or, updating the Makefile rules so that a “make distclean” will really clean everything.
I often use "make distcheck" which does that automatically.
I’ll take a look at the patches. My guess is that most of them can be pulled over as-is. If it’s all right with you, I’d like to pull over all of the IPv6 fixes, and then do a release. We can leave the DTLS code as experimental, and add it to a later release.
Would be nice. Note that the IPv6 changes break the API and ABI by removing several helper functions. However, the removed functions were simply a wrap over the standard socket (IPv4) library, so I find it unlikely that will seriously affect any project using that library. regards, Nikos
On Dec 23, 2014, at 2:49 AM, Nikos Mavrogiannopoulos <nmav@gnutls.org> wrote:
The advantage of freeradius-client is that it has a very easy high level API and could be used with a simple config file without a low level knowledge of the radius protocol.
It wouldn’t be that hard to wrap libfreeradius-radius with such an API. But… OK.
That is why I preferred to modify that library, and the changes for DTLS weren't that large actually. They simply wrap over the sendto() and recvfrom(). The only tricky part is that they serialize every communication over a single DTLS session, and that would affect multi-threaded programs using that library, as the sendto(), recvfrom() need to be atomic. I introduced locks to solve that, but a proper multiplexer could be a better solution for such programs.
If someone needs to send hundreds or thousands of RADIUS packets, they can spend the time to fix the code.
Unrelated: If I can mention a small omission in the DTLS rfc is that it doesn't specify a key purpose in the certificates to be used for radius. For web servers the "TLS WWW server authentication" OID is used in extended key usage, but there is no equivalent for DTLS radius servers.
The "TLS WWW server authentication” OID is not required by RADIUS. Microsoft requires it for their clients, which is annoying. I’m not sure there’s a major benefit to adding an OID specifically for RADIUS authentication.
I'd really prefer to fix any issues that result of that change instead of having the auto-generated files included. It is very cumbersome to work with a repository which has 20 files modified because I changed something in a Makefile.am. I've almost forgot to add all new files because git status shows so many changes it is difficult to distinguish what matters or not.
Sure. You could create an “autoconf” branch, and put the generated files there. Have a script which merges all of the patches from “master", regenerates the files, commits them, and then checks out “master” again. That way you get the best of both worlds.
$ ./autogen.sh … $ ./configure … configure: creating ./config.status config.status: error: cannot find input file: `Makefile.in’
The autogen.sh run has failed for some reason in that platform. It's best to fix that anyway.
The autogen.sh file didn’t fail. It succeeded. And this is on an Ubuntu 14.04 system. If autogen doesn’t work there, there’s no reason to expect it to work *anywhere*. This is why I *loath* projects that don’t include configure, etc. in a checkout. autoconf, etc. are such horrible crap that they just don’t work across multiple systems. The only person capable of creating a “configure” script is the person who committed the “autoconf” crap. No one else can do it, which makes autoconf nearly useless.
Would be nice. Note that the IPv6 changes break the API and ABI by removing several helper functions. However, the removed functions were simply a wrap over the standard socket (IPv4) library, so I find it unlikely that will seriously affect any project using that library.
That’s fine. Alan DeKok.
On Tue, 2014-12-23 at 11:13 -0500, Alan DeKok wrote:
Unrelated: If I can mention a small omission in the DTLS rfc is that it doesn't specify a key purpose in the certificates to be used for radius. For web servers the "TLS WWW server authentication" OID is used in extended key usage, but there is no equivalent for DTLS radius servers. The "TLS WWW server authentication” OID is not required by RADIUS. Microsoft requires it for their clients, which is annoying.
I’m not sure there’s a major benefit to adding an OID specifically for RADIUS authentication.
The reason for different key purposes is to prevent cross-protocol attacks, i.e., someone forwarding the radius packets to another service on the same host. This is particularly dangerous with text-based protocols like SMTP and HTTP and a common CA signing all services. Requiring "TLS WWW server authentication" in a radius server certificate is of course wrong as it defeats that purpose. regards, Nikos
Nikos Mavrogiannopoulos <nmav@gnutls.org> wrote: > I've modified the freeradius-client to simplify its API, remove all > IPv4-only legacy code and add DTLS support (rfc7360). The DTLS support > I've tried to add in a transparent way, i.e., an old client could still > have basic DTLS support only by modifying its configuration file. This > is currently experimental code and most likely will have bugs, but I'd > like to see any comments on the approach or possible improvements. > My main concern is that there have been no releases of freeradius-client > for many years, although the intention is to send back the changes as > pull requests to the original repository once they stabilize. I posted about: https://github.com/credil/freeradius-client a few months back. I was mostly adding support for IPv6 TLVs. I think that you made it work over IPv6, whereas I cared about carrying information about IPv6 connections. > The code: https://github.com/nmav/radiusclient-dtls I'm interested in mergering things. -- ] Never tell me the odds! | ipv6 mesh networks [ ] Michael Richardson, Sandelman Software Works | network architect [ ] mcr@sandelman.ca http://www.sandelman.ca/ | ruby on rails [
On Mon, 2014-12-22 at 18:00 -0500, Michael Richardson wrote:
Nikos Mavrogiannopoulos <nmav@gnutls.org> wrote: > I've modified the freeradius-client to simplify its API, remove all > IPv4-only legacy code and add DTLS support (rfc7360). The DTLS support > I've tried to add in a transparent way, i.e., an old client could still > have basic DTLS support only by modifying its configuration file. This > is currently experimental code and most likely will have bugs, but I'd > like to see any comments on the approach or possible improvements.
> My main concern is that there have been no releases of freeradius-client > for many years, although the intention is to send back the changes as > pull requests to the original repository once they stabilize.
I posted about: https://github.com/credil/freeradius-client
a few months back. I was mostly adding support for IPv6 TLVs. I think that you made it work over IPv6, whereas I cared about carrying information about IPv6 connections.
Incidentally I think I've reimplemented it as I also needed IPv6 TLVs and is now included in the main freeradius-client repository. regards, Nikos
participants (3)
-
Alan DeKok -
Michael Richardson -
Nikos Mavrogiannopoulos