Allow white spaces (spaces and tabs) in shared secret by the use of a quoted string (addresses issue #105), add TCP support for specified servers (starting with "tcp://"), and added RADIUS ovre TLS (RADSEC) support for servers starting with "tls://"#110
Conversation
… quoted string, and add size checks on src_ip and vrf.
If the secret begins with a single quote "'" treat it has a quoted string an read
up to the ending quote. If any other single quotes are present they must be escaped
with a backslash ("\").
Eg. ' secret with spaces \' quotes and tabs'
If the secret value starts with a single quote ("'") and has no spaces, you may
just escape it with a blackslash ("\").
E.g. \'secret_starts_with_quote_but_is_not_enclosed
Without _PARSE_STRICT, as before, parsing ignores non numerical timeouts and any extra trailing
content (words) after the vfr field.
By defining _PARSE_STRICT such bad timeouts or extra content raise error.
|
As mentioned on issue #102 I also added support for TCP, if the server name or address begins with "tcp://", TCP connection is used. |
WARNING: Openssl library is used and required.
Basically to use it provide required client certificate, and optionally the authenticating CAs, by adding the following parameters to the pam config line (both auth and session):
cert = absolute pathname of the client certificate file (PEM format)
key = absolute pathname of the client private key file
ca = absolute pathname to the know and authentication CAs file
Then specify RADSEC usage by adding the prefix "tls://" to the desired server address or hostname in the pam_radius_auth.conf file.
By default servers certificates are verified, you may ignore failures by adding the "verify=no" option.
You may force RADSEC usage on all servers, without the "tls://" prefix, by setting "radsec=yes".
You may disable RADSEC usage, falling back all "tls://" to RADIUS over UDP, by setting "radsec=no".
By default, with "radsec=try" , if SSL setup works, RADSEC is used for "tls://", otherwise they fallback to RADIUS over UDP.
WARNING: SELinux may deny proper functioning.
For testing propouse disable it with:
setenforce 0
In production environment add, for example, the following rules to authorize usage of 1812/tcp, 1813/tcp and 2083/tcp by sshd:
require {
type radius_port_t;
type radsec_port_t;
type sshd_t;
type radacct_port_t;
class tcp_socket name_connect;
}
allow sshd_t radacct_port_t:tcp_socket name_connect;
allow sshd_t radius_port_t:tcp_socket name_connect;
allow sshd_t radsec_port_t:tcp_socket name_connect;
|
Hi @alandekok , Build artifacts (configure and make, macro HAVE_OPENSSL) are still missing. But I will add them if needed. |
|
This looks very interesting, thanks! I'm pretty much out until the end of March (traveling / conferences) but I will take a look when I get back. I think these patches are very useful. |
jake-scott
left a comment
There was a problem hiding this comment.
Hi - its funny how we both submitted TCP support more or less at the same time!
Your changes are certainly simpler than mine. A few notes, plus I think we need separate timeouts for connect vs. reading a response -- usually you only want to wait a second or two max for a connect before failing over to a working server but you might want to wait much longer for a response especially if there is an out of band auth going on for example.
|
|
||
| /* open a socket. Dies if it fails */ | ||
| *sockfd = socket(AF_INET, SOCK_DGRAM, 0); | ||
| *sockfd = socket(AF_INET, (tcp? SOCK_STREAM: SOCK_DGRAM), 0); |
There was a problem hiding this comment.
The issue with initializing the TCP socket here is that you can't re-use a TCP socket once you have tried to connect it. You have to close it and re-create with a new socket().
There was a problem hiding this comment.
Checkout initialize() in src/pam_radius_auth.c at line 1028:
/* TCP needs its own socket */
if (valid_src_ip == 0 || vrf[0] || server->tcp) {
#ifndef NDEBUG
if(server->tcp) _pam_log(LOG_DEBUG,"Use TCP for %s\n",server->hostname);
#endif
if (initialize_sockets(conf, &server->sockfd, &server->sockfd6, &salocal4, &salocal6, vrf, server->tcp) != 0) {
For each TCP server a dedicated socket is used.
Compile and try it your self with various faulty servers.
There was a problem hiding this comment.
ah I'm sorry I didn't notice you were stuffing the TCP socket into the server struct and not storing it in the global context..
| } | ||
|
|
||
| /* set up the local end of the socket communications */ | ||
| if (bind(*sockfd, (struct sockaddr *)salocal4, sizeof (struct sockaddr_in)) < 0) { |
There was a problem hiding this comment.
It is valid to bind TCP sockets to source addresses as it is for UDP.
There was a problem hiding this comment.
Checkout at line:
/* If not TCP or has a source address set up the local end of the socket communications /
if(!tcp || ((struct sockaddr_in)salocal4)->sin_addr.s_addr != 0) {
if (bind(*sockfd, (struct sockaddr *)salocal4, sizeof (struct sockaddr_in)) < 0) {
char error_string[BUFFER_SIZE];
get_error_string(errno, error_string, sizeof(error_string));
_pam_log(LOG_ERR, "Failed binding to port: %s", error_string);
return -1;
}
}
If a source address is specified I do bind the TCP socket, otherwise I let do it at connect().
|
|
||
| while(1) { | ||
| #ifdef HAVE_POLL_H | ||
| rcode = poll((struct pollfd *) &pollfds, 1, tv.tv_sec * 1000); |
There was a problem hiding this comment.
It is likely that tv_usec won't be zero in anything except the first loop. Maybe add tv_sec * 1000 here.
There was a problem hiding this comment.
Basically the problem is that poll(), unlike select() (and ppoll() that is less portable), does not updates the timeout.
So I borrowed the code from the EINTR case of the recvfrom loop already in use.
As time() works on seconds, while manually updating the timeout, we may underestimate or overestimate. In real cases we may wait a little more (<1 sec) than the specified timeout.
More precise time keeping may be done using clock_gettime();
There was a problem hiding this comment.
right - you have to get the current time of day -- I used gettimeofday() - take a look at : https://github.com/jake-scott/pam_radius/blob/jake/tcp-support/src/pam_radius_auth.c#L1040 if that's helpful.
There was a problem hiding this comment.
Thanks @jake-scott for the nice suggestion.
I will change the timeout updating code with gettimeofday(). Just give me some days,
I'm temporarily busy with my usual corporate IT job.
By the way, I think it would be very nice (mostly for TCP and TLS), make the most of poll() and select() multiplexing, attempting to establish a connection to the servers in parallel, and sending the request (Access or Accounting) to the fastest.
Already done very similar stuff in the context of mass concurrent SNMP polling and ICMP monitoring (for the aforementioned job).
Hope to be back soon with more code updates.
|
|
||
| tv.tv_sec = end - now; | ||
| if (tv.tv_sec == 0) { /* keep waiting */ | ||
| tv.tv_sec = 1; |
There was a problem hiding this comment.
I think its valid to wait < 1second?..
| total_length = 0; | ||
| while (ok) { | ||
| #ifdef HAVE_POLL_H | ||
| rcode = poll((struct pollfd *) &pollfds, 1, tv.tv_sec * 1000); |
There was a problem hiding this comment.
Again I don't think we have to wait for exact multiples of one second
| } else { | ||
| tv.tv_sec = end - now; | ||
| if (tv.tv_sec == 0) { /* keep waiting */ | ||
| tv.tv_sec = 1; |
| /* If TCP we must connect */ | ||
| if(server->tcp) { | ||
| if(conf->debug) _pam_log(LOG_DEBUG,"Setup connection for %s\n",server->hostname); | ||
| rcode = connect_tmout(sockfd, server->ip, server->ip->sa_family == AF_INET? sizeof(struct sockaddr_in): sizeof(struct sockaddr_in6), server->timeout); |
There was a problem hiding this comment.
This won't work if sockfd has been used before like if this is not the first attempt of the first TCP server.
There was a problem hiding this comment.
I know.
Each TCP server has its own dedicated socket.
|
Hi @jake-scott, Actually, having some spare time, I was looking at the issues, and working on what seem interesting for me. So sorry. You are right, having separate connect and read timeout for TCP make absolutely. I will added soon. Now let me respond to your other questions. |
Use ./configure --disable-radsec to compile without openssl and RADSEC. Use ./configure --enable-radsec to force support for RADSEC, if openssl is not usable configuration fails. By default ./configure checks openssl usability and if present enables RADSEC support. WARNING. My autoconf is older (2.69) then the last ìn use (2.71)
Now the timeout field (in pam_radius_auth.conf) accepts the following syntax: read_timeout[,connect_timeout] This means you may additionally specify a connection timeout, by default it is set to 1 second, and it is used only for TCP or TLS (RADSEC) servers.
|
Now the timeout field (in pam_radius_auth.conf) accepts the following syntax: read_timeout[,connect_timeout] This means you may additionally specify a connection timeout, |
… keeping on poll()
In the most general case the radius protocol exchange may require more then a single request-response (eg: Access-Challenge). So we must avoid reconnectiong an already connected socket, and keep track of the last usable SSL context.
| tv.tv_sec = end - now; | ||
| if (tv.tv_sec == 0) { /* keep waiting */ | ||
| tv.tv_sec = 1; | ||
| timersub(&end,&now,&tv); |
There was a problem hiding this comment.
Wasn't aware these were included in glibc. That's nice -- is it there on all the platforms supported by this code though? (is Solaris still supported?)
There was a problem hiding this comment.
Maybe not, but it's an easy macro from /usr/include/sys/time.h:
define timersub(a, b, result) \
do {
(result)->tv_sec = (a)->tv_sec - (b)->tv_sec;
(result)->tv_usec = (a)->tv_usec - (b)->tv_usec;
if ((result)->tv_usec < 0) {
--(result)->tv_sec;
(result)->tv_usec += 1000000;
}
} while (0)
If missing we may add such definition.
There was a problem hiding this comment.
Ah OK good news. Reading the glibc page that I got the timeval_subtract function from again, that extra complexity is to deal with systems where tv_sec is unsigned. I'm not sure which those are! (https://ftp.gnu.org/old-gnu/Manuals/glibc-2.2.3/html_node/libc_418.html). Solaris/Linux/BSD have signed members thankfully.
If the client private key is encrypted you may specify the encryption password by adding the "key_password=" parameter to the pam config file.
|
Hi @alandekok did you have any thoughts about this PR? |
|
I'll take a look at it next week. |
resource. At first, if any, all TCP and TLS servers are tried together. The first one ready, connected and handshaked, is used. If something goes wrong the others get used, as they are ready. When no more TCP or TLS server are usable, UDP are tried one by one in the config order.
|
Hello, I have improved the code. Now all TCP/TLS servers are tried first and together, so that we can use the first one that is ready. |
|
thanks. I'll review this in the next week or two, and pull it in. |
Allow white spaces (spaces and tabs) in shared secret by the use of a quoted string, and add size checks on src_ip and vrf.