On Friday, March 3, 2017, Adam Brightwell <adam.brightw...@crunchydata.com> wrote:
> I've given an initial review of this patch. It applies cleanly and > compiles without issue as of 6da9759. I'm going to continue with > testing it against a set of RADIUS servers in the next few days. But > in the meantime, I have a few questions (below). > > > It supports multiple RADIUS servers. For all other parameters (secret, > port, > > identifier) one can specify either the exact same number of entries, in > > which case each server gets it's own, or exactly one entry in which case > > that entry will apply to all servers. (Or zero entries for everything > except > > secret, which will make it the default). > > I wonder if removing the complexity of maintaining two separate lists > for the server and port would be a better/less complex approach. For > instance, why not go with a list of typical 'host:port' strings for > 'radiusservers'? If no port is specified, then simply use the default > for that specific host. Therefore, we would not have to worry about > keeping the two lists in sync. Thoughts? > If we do that we should do it for all the parameters, no? So not just host:port, but something like host:port:secret:identifier? Mixing the two ways of doing it would be quite confusing I think. And I wonder if that format wouldn't get even more confusing if you for example want to use default ports, but non-default secrets. I can see how it would probably be easier in some of the simple cases, but I wonder if it wouldn't make it worse in a lot of other cases. > > Each server is tried in order. If it responds positive, auth is OK. If it > > responds negative, auth is rejected. If it does not respond at all, we > move > > on to the next one. > > > > I'm wondering if in doing this we should also make the RADIUS timeout a > > configurable as HBA option, since it might become more important now? > > Yes, I think this would make sense and would be a good idea. > > //Magnus -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
