Hi Chris,
Thank you for your reply to my question. Now, I did checkout
freeradius-client from CVS and the function that I asked for exist.
However, I did run into a few problems / issues. I am trying to
cross-compile this for an ARM Intel ixp425 platform.
1) I needed to export ac_cv_func_uname=no into the environment before
executing ./configure. Configure just didn't like the fact that I was
cross-compiling with a different toolchain and barfed when it figured
it couldn't run the test program. This should probably get fixed for
1.1.6.
2) There are some added macro "DEBUG" added to the
freeradius-client.h. This conflicts with a different define of DEBUG
defined somewhere else. I'd say either remove it, or rename it. It's
for debugging anyway.
3) Cross-compiling 1.1.5, I had a problem with sending authentication
requests where the password got corrupted due to some alignment errors
in rc_pack_list() and rc_md5_calc(). It appears that the problem is
gone in CVS, however; the code looks pretty much the same to me. What
I did to circumvent the problem was to declare a char
md5[MD5_DIGEST_LENGTH] and use that for MD5Final(), then do a
memcpy(output, md5, MD5_DIGEST_LENGTH). This works well, when the
output variable doesn't align with the boundary of my processor, in
gdb it showed up as 0x...0a and it stomped on 0x...08 (the two first
bytes got corrupted and my radiusd didn't understand squat).
4) I looked at the function signature for rc_add_config(...), and it
does not seem to require "line", and "filename". It's more an artifact
of your static functions requiring some input in case they fail. In my
opinion, since the code is shared between rc_read_config(), the static
functions should not need to know anything about filename and
position, that's a responsibility of the caller function. Return codes
can be used to solve this better refactoring the functions to better
fit a users (freeradius-client api) need. The filename and line
arguments simply doesn't have any value when you embed the radius
client.
Please consider the following as comments:
5) If you really want to 'embed' freeradius-client, it would be nice
if I could specify something similar to "authserv
<ip>:<port>:<secret>" and not needing to write a "server" file which
declares the servers w/secrets.
6) I don't necessarily understand the radius-protocol in detail, but I
would think it would be nice to be able to specify the dictionary
attributes/values as you kind of do with rc_add_config(..). This again
relates to the fact of reducing external dependencies when embedding
freeradius-client.
7) This comment is related to logs, and I haven't studied the use of
rc_log too closely. Just in general, this is a library and not an
application. It would be nice to possibly use logs to trace calls
within the library, but for error reporting it seems like it would be
more useful to return an error code, and then call a
rc_geterror_str(errno) or something. I am speaking of this as an
integrator of OSS and I often like the ability to control the output
of an application and not to necessarily "pollute" the system's event
logs.
BTW, I think freeradius is a great project, keep up the good work :o)
I've attached a patch as a set of differences that fixes #2 #4.
----------------------------------------------------------
diff -ruNBb ../../vendor-drop/src/include/freeradius-client.h
../src/include/freeradius-client.h
--- ../../vendor-drop/src/include/freeradius-client.h 2007-10-05
08:37:26.000000000 -0700
+++ ../src/include/freeradius-client.h 2007-11-07 08:15:23.000000000 -0800
@@ -17,12 +17,6 @@
#ifndef RADIUSCLIENT_NG_H
#define RADIUSCLIENT_NG_H
-#ifdef CP_DEBUG
-#define DEBUG rc_log
-#else
-#define DEBUG
-#endif
-
#include <sys/types.h>
/*
* Include for C99 uintX_t defines is stdint.h on most systems. Solaris uses
@@ -447,7 +441,7 @@
SERVER *rc_conf_srv(rc_handle *, char *);
int rc_find_server(rc_handle *, char *, uint32_t *, char *);
void rc_config_free(rc_handle *);
-int rc_add_config(rc_handle *, const char *, const char *, const char
*, const int);
+int rc_add_config(rc_handle *, const char *, const char *);
rc_handle *rc_config_init(rc_handle *);
int test_config(rc_handle *, char *);
diff -ruNBb ../../vendor-drop/src/lib/config.c ../src/lib/config.c
--- ../../vendor-drop/src/lib/config.c 2007-07-11 10:29:29.000000000 -0700
+++ ../src/lib/config.c 2007-11-07 08:16:14.000000000 -0800
@@ -52,7 +52,7 @@
* Returns: 0 on success, -1 on failure
*/
-static int set_option_str(const char *filename, int line, OPTION
*option, const char *p)
+static int set_option_str(OPTION *option, const char *p)
{
if (p) {
option->val = (void *) strdup(p);
@@ -67,12 +67,11 @@
return 0;
}
-static int set_option_int(const char *filename, int line, OPTION
*option, const char *p)
+static int set_option_int(OPTION *option, const char *p)
{
int *iptr;
if (p == NULL) {
- rc_log(LOG_ERR, "%s: line %d: bogus option value",
filename, line);
return -1;
}
@@ -87,7 +86,7 @@
return 0;
}
-static int set_option_srv(const char *filename, int line, OPTION
*option, const char *p)
+static int set_option_srv(OPTION *option, const char *p)
{
SERVER *serv;
char *p_pointer;
@@ -100,13 +99,11 @@
p_dupe = strdup(p);
if (p_dupe == NULL) {
- rc_log(LOG_ERR, "%s: line %d: Invalid option or memory
failure", filename, line);
return -1;
}
serv = (SERVER *) option->val;
if (serv == NULL) {
- DEBUG(LOG_ERR, "option->val / server is NULL,
allocating memory");
serv = malloc(sizeof(*serv));
if (serv == NULL) {
rc_log(LOG_CRIT, "read_config: out of memory");
@@ -152,12 +149,11 @@
else
serv->port[serv->max] = ntohs
((unsigned int) svp->s_port);
else {
- rc_log(LOG_ERR, "%s: line %d: no default port
for %s", filename, line, option->name);
if (option->val == NULL) {
free(p_dupe);
free(serv);
}
- return -1;
+ return -2;
}
}
@@ -180,7 +176,7 @@
return 0;
}
-static int set_option_auo(const char *filename, int line, OPTION
*option, const char *p)
+static int set_option_auo(OPTION *option, const char *p)
{
int *iptr;
char *p_dupe = NULL;
@@ -190,7 +186,6 @@
p_dupe = strdup(p);
if (p_dupe == NULL) {
- rc_log(LOG_WARNING, "%s: line %d: bogus option value",
filename, line);
return -1;
}
@@ -200,18 +195,15 @@
}
*iptr = 0;
- /*if(strstr(p_dupe,", \t") != NULL) {*/
p_pointer = strtok_r(p_dupe, ", \t", &p_save);
- /*}*/
if (!strncmp(p_pointer, "local", 5))
*iptr = AUTH_LOCAL_FST;
else if (!strncmp(p_pointer, "radius", 6))
*iptr = AUTH_RADIUS_FST;
else {
- rc_log(LOG_ERR,"%s: auth_order: unknown keyword: %s",
filename, p);
free(p_dupe);
- return -1;
+ return -2;
}
p_pointer = strtok_r(NULL, ", \t", &p_save);
@@ -222,9 +214,8 @@
else if ((*iptr & AUTH_LOCAL_FST) &&
!strcmp(p_pointer, "radius"))
*iptr = (*iptr) | AUTH_RADIUS_SND;
else {
- rc_log(LOG_ERR,"%s: auth_order: unknown or
unexpected keyword: %s", filename, p);
free(p_dupe);
- return -1;
+ return -2;
}
}
@@ -242,7 +233,7 @@
* Returns: 0 on success, -1 on failure
*/
-int rc_add_config(rc_handle *rh, const char *option_name, const char
*option_val, const char *source, const int line)
+int rc_add_config(rc_handle *rh, const char *option_name, const char
*option_val)
{
OPTION *option;
@@ -260,22 +251,22 @@
switch (option->type) {
case OT_STR:
- if (set_option_str(source, line, option,
option_val) < 0) {
+ if (set_option_str(option, option_val) < 0) {
return -1;
}
break;
case OT_INT:
- if (set_option_int(source, line, option,
option_val) < 0) {
+ if (set_option_int(option, option_val) < 0) {
return -1;
}
break;
case OT_SRV:
- if (set_option_srv(source, line, option,
option_val) < 0) {
+ if (set_option_srv(option, option_val) < 0) {
return -1;
}
break;
case OT_AUO:
- if (set_option_auo(source, line, option,
option_val) < 0) {
+ if (set_option_auo(option, option_val) < 0) {
return -1;
}
break;
@@ -353,6 +344,7 @@
char buffer[512], *p;
OPTION *option;
int line;
+ int rcode;
size_t pos;
rc_handle *rh;
@@ -420,28 +412,46 @@
switch (option->type) {
case OT_STR:
- if (set_option_str(filename, line,
option, p) < 0) {
+ rcode = set_option_str(option, p);
+ if (rcode < 0) {
+ rc_log(LOG_ERR, "%s: line %d:
unable to set option value", filename, line );
fclose(configfd);
rc_destroy(rh);
return NULL;
}
break;
case OT_INT:
- if (set_option_int(filename, line,
option, p) < 0) {
+ rcode = set_option_int(option, p);
+ if (rcode < 0) {
+ rc_log(LOG_ERR, "%s: line %d:
unable to set option value", filename, line);
fclose(configfd);
rc_destroy(rh);
return NULL;
}
break;
case OT_SRV:
- if (set_option_srv(filename, line,
option, p) < 0) {
+ rcode = set_option_srv(option, p);
+ if (rcode == -1)
+ rc_log(LOG_ERR, "%s: line %d:
unable to set option value", filename, line);
+
+ if (rcode == -2)
+ rc_log(LOG_ERR, "%s: line %d:
no default port for %s", filename, line, option->name);
+
+ if (rcode < 0) {
fclose(configfd);
rc_destroy(rh);
return NULL;
}
break;
case OT_AUO:
- if (set_option_auo(filename, line,
option, p) < 0) {
+ rcode = set_option_auo(option, p);
+ if (rcode == -1)
+ rc_log(LOG_ERR, "%s: line %d:
unable to set option value", filename, line);
+
+ if (rcode == -2)
+ rc_log(LOG_ERR, "%s: line %d:
found unknown keyword/value: %s", filename, line, p);
+
+ if (rcode < 0) {
fclose(configfd);
rc_destroy(rh);
return NULL;
Binary files ../../vendor-drop/src/lib/.config.c.swp and
../src/lib/.config.c.swp differ
diff -ruNBb ../../vendor-drop/src/lib/sendserver.c ../src/lib/sendserver.c
--- ../../vendor-drop/src/lib/sendserver.c 2007-08-07
07:26:29.000000000 -0700
+++ ../src/lib/sendserver.c 2007-11-07 07:35:44.000000000 -0800
@@ -226,8 +226,6 @@
/*}*/
}
- DEBUG(LOG_ERR, "DEBUG: rc_send_server: creating socket to:
%s", server_name);
-
sockfd = socket (AF_INET, SOCK_DGRAM, 0);
if (sockfd < 0)
{
@@ -297,10 +295,6 @@
auth->length = htons ((unsigned short) total_length);
}
- DEBUG(LOG_ERR, "DEBUG: local %s : 0, remote %s : %u\n",
- inet_ntoa(sinlocal.sin_addr),
- inet_ntoa(sinremote.sin_addr), data->svc_port);
-
for (;;)
{
sendto (sockfd, (char *) auth, (unsigned int)
total_length, (int) 0,
Binary files ../../vendor-drop/src/lib/.sendserver.c.swp and
../src/lib/.sendserver.c.swp differ
diff -ruNBb ../../vendor-drop/src/src/radembedded.c ../src/src/radembedded.c
--- ../../vendor-drop/src/src/radembedded.c 2007-10-08
08:48:06.000000000 -0700
+++ ../src/src/radembedded.c 2007-11-07 08:18:46.000000000 -0800
@@ -52,42 +52,42 @@
/* set auth_order to be radius only */
- if (rc_add_config(rh, "auth_order", "radius", "config", 0) != 0)
+ if (rc_add_config(rh, "auth_order", "radius") != 0)
{
printf("ERROR: Unable to set auth_order.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "login_tries", "4", "config", 0) != 0)
+ if (rc_add_config(rh, "login_tries", "4") != 0)
{
printf("ERROR: Unable to set login_tries.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "dictionary",
"/usr/local/radius/dictionary", "config", 0) != 0)
+ if (rc_add_config(rh, "dictionary",
"/usr/local/radius/dictionary") != 0)
{
printf("ERROR: Unable to set dictionary.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "seqfile", "/var/run/radius.seq",
"config", 0) != 0)
+ if (rc_add_config(rh, "seqfile", "/var/run/radius.seq") != 0)
{
printf("ERROR: Unable to set seq file.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "radius_retries", "3", "config", 0) != 0)
+ if (rc_add_config(rh, "radius_retries", "3") != 0)
{
printf("ERROR: Unable to set radius_retries.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "radius_timeout", "5", "config", 0) != 0)
+ if (rc_add_config(rh, "radius_timeout", "5") != 0)
{
printf("ERROR: Unable to set radius_timeout.\n");
rc_destroy(rh);
@@ -100,14 +100,14 @@
* be used.
*/
- if (rc_add_config(rh, "authserver", "localhost::testing123",
"config", 0) != 0)
+ if (rc_add_config(rh, "authserver", "localhost::testing123") != 0)
{
printf("ERROR: Unable to set authserver.\n");
rc_destroy(rh);
exit(1);
}
- if (rc_add_config(rh, "acctserver",
"localhost:1813:testing123", "config", 0) != 0)
+ if (rc_add_config(rh, "acctserver", "localhost:1813:testing123") != 0)
{
printf("ERROR: Unable to set acctserver.\n");
rc_destroy(rh);
