I think this is broken; specifically, "ip_allocation" is used undefined in the successful case. [I would make a patch, but apparently some major surgery has happend to the upstream repo, and my entire github repo is now broken for git fetch / git merge, and I don't have the time to sort that out now]
On 11 Oct 2013, at 11:17, Phil Mayers <p.mayers@imperial.ac.uk> wrote:
I think this is broken; specifically, "ip_allocation" is used undefined in the successful case.
[I would make a patch, but apparently some major surgery has happend to the upstream repo,
Not that I know of, I'll double check when I get into the office.
and my entire github repo is now broken for git fetch / git merge, and I don't have the time to sort that out now]
git reset --hard HEAD~10 git pull May fix things?
On 11/10/13 11:17, Phil Mayers wrote:
I think this is broken; specifically, "ip_allocation" is used undefined in the successful case.
No, that's not it; the problem is that vp_length is unset i.e. 0. For some reason: fr_strerror_printf("WARNING: Skipping zero-length attribute ...in src/lib/radius.c isn't warning about this. Fix is simple: diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index 1db4478..bb0c98e 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -607,6 +607,7 @@ static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request) vp = radius_paircreate(request, &request->reply->vps, PW_FRAMED_IP_ADDRESS, 0); vp->vp_ipaddr = ip_allocation; + vp->length = 4; DO(allocate_commit);
On 11 Oct 2013, at 16:12, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
On 11/10/13 11:17, Phil Mayers wrote:
I think this is broken; specifically, "ip_allocation" is used undefined in the successful case.
No, that's not it; the problem is that vp_length is unset i.e. 0.
For some reason:
fr_strerror_printf("WARNING: Skipping zero-length attribute
...in src/lib/radius.c isn't warning about this.
Fix is simple:
diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index 1db4478..bb0c98e 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -607,6 +607,7 @@ static rlm_rcode_t mod_post_auth(void *instance, REQUEST *request) vp = radius_paircreate(request, &request->reply->vps, PW_FRAMED_IP_ADDRESS, 0); vp->vp_ipaddr = ip_allocation; + vp->length = 4;
DO(allocate_commit);
OK, merged that. I wonder whether it might be an idea to pre-initialize the length in paircreate for fixed length attributes? Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team
Phil Mayers wrote:
Maybe, as long as they're zero-ed out - OTOH barfing more aggressively on 0-len attributes might be another choice.
I think so. The API of "create raw attribute" is arguably bad. There should instead be: "create X using data Y". The attribute can then be created and initialized in one swell foop. Alan DeKok.
participants (3)
-
Alan DeKok -
Arran Cudbard-Bell -
Phil Mayers