DeforaNetworks/fwknop
DeforaNetworks/fwknop
3
0
Fork 0
Code Releases Activity

another merge from master

Browse Source
This commit is contained in:
Michael Rash 2012-09-03 22:32:44 -04:00
commit 55fa4841f2
23 changed files with 238 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
CREDITS
View File

@ -60,3 +60,9 @@ Fernando Arnaboldi (IOActive)
developed along with a new fuzzing capability in the test suite.
- Found a condition in which an overly long IP from malicious authenticated
clients is not properly validated by the fwknopd server (pre-2.0.3).
- Found a local buffer overflow in --last processing with a maliciously
constructed ~/.fwknop.run file. This has been fixed with proper
validation of .fwknop.run arguments.
- Found several conditions in which the server did not properly throw out
maliciously constructed variables in the access.conf file. This has been
fixed along with new fuzzing tests in the test suite.

10
ChangeLog
View File

@ -1,6 +1,4 @@
fwknop-2.0.3 (08//2012):
- Fixed RPM builds by including the $(DESTDIR) prefix for uninstall-local
and install-exec-hook stages in Makefile.am.
fwknop-2.0.3 (09/03/2012):
- [server] Fernando Arnaboldi from IOActive found several DoS/code
execution vulnerabilities for malicious fwknop clients that manage to
get past the authentication stage (so a such a client must be in
@ -21,9 +19,15 @@ fwknop-2.0.3 (08//2012):
- [client] Fernando Arnaboldi from IOActive found a local buffer overflow
in --last processing with a maliciously constructed ~/.fwknop.run file.
This has been fixed with proper validation of .fwknop.run arguments.
- [server] Fernando Arnaboldi from IOActive found several conditions in
which the server did not properly throw out maliciously constructed
variables in the access.conf file. This has been fixed along with new
fuzzing tests in the test suite.
- [test suite] Added a new fuzzing capability to ensure proper server-side
input validation. Fuzzing data is constructed with modified fwknop
client code that is designed to emulate malicious behavior.
- Fixed RPM builds by including the $(DESTDIR) prefix for uninstall-local
and install-exec-hook stages in Makefile.am.
fwknop-2.0.2 (08/18/2012):
- [server] For GPG mode, added a new access.conf variable

3
Makefile.am
View File

@ -150,6 +150,9 @@ EXTRA_DIST = \
test/conf/subnet_source_match_access.conf \
test/conf/local_nat_fwknopd.conf \
test/conf/disable_aging_fwknopd.conf \
test/conf/fuzzing_source_access.conf \
test/conf/fuzzing_open_ports_access.conf \
test/conf/fuzzing_restrict_ports_access.conf \
test/hardening-check \
test/local_spa.key \
test/test-fwknop.pl \

2
VERSION
View File

@ -1 +1 @@
fwknop-2.0.2
fwknop-2.0.3

6
android/project/jni/config.h
View File

@ -207,13 +207,13 @@
#define PACKAGE_NAME "fwknop"
/* Define to the full name and version of this package. */
#define PACKAGE_STRING "fwknop 2.0.2"
#define PACKAGE_STRING "fwknop 2.0.3"
/* Define to the one symbol short name of this package. */
#define PACKAGE_TARNAME "fwknop"
/* Define to the version of this package. */
#define PACKAGE_VERSION "2.0.2"
#define PACKAGE_VERSION "2.0.3"
/* The size of `unsigned int', as computed by sizeof. */
#define SIZEOF_UNSIGNED_INT 4
@ -247,7 +247,7 @@
/* Version number of package */
#define VERSION "2.0.2"
#define VERSION "2.0.3"
/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
significant byte first (like Motorola and SPARC, unlike Intel). */

2
client/config_init.c
View File

@ -518,7 +518,7 @@ process_rc(fko_cli_options_t *options)
}
/* Check rc file permissions - if anything other than user read/write,
* then don't process it. This change was made to help ensure that the
* then throw a warning. This change was made to help ensure that the
* client consumes a proper rc file with strict permissions set (thanks
* to Fernando Arnaboldi from IOActive for pointing this out).
*/

2
client/fwknop.c
View File

@ -637,6 +637,7 @@ show_last_command(void)
#endif
if (get_save_file(args_save_file)) {
verify_file_perms_ownership(args_save_file);
if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) {
fprintf(stderr, "Could not open args file: %s\n",
args_save_file);
@ -681,7 +682,6 @@ run_last_args(fko_cli_options_t *options)
if (get_save_file(args_save_file))
{
verify_file_perms_ownership(args_save_file);
if ((args_file_ptr = fopen(args_save_file, "r")) == NULL)
{
fprintf(stderr, "Could not open args file: %s\n",

19
client/utils.c
View File

@ -110,6 +110,7 @@ set_file_perms(const char *file)
int
verify_file_perms_ownership(const char *file)
{
int res = 1;
#if HAVE_STAT
struct stat st;
@ -118,10 +119,18 @@ verify_file_perms_ownership(const char *file)
*/
if((stat(file, &st)) != 0)
{
fprintf(stderr, "[-] unable to run stat() against file: %s: %s\n",
/* if the path doesn't exist, just return, but otherwise something
* went wrong
*/
if(errno == ENOENT)
{
return 0;
} else {
fprintf(stderr, "[-] stat() against file: %s returned: %s\n",
file, strerror(errno));
exit(EXIT_FAILURE);
}
}
/* Make sure it is a regular file or symbolic link
*/
@ -131,7 +140,7 @@ verify_file_perms_ownership(const char *file)
"[-] file: %s is not a regular file or symbolic link.\n",
file
);
return 0;
res = 0;
}
if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
@ -140,18 +149,18 @@ verify_file_perms_ownership(const char *file)
"[-] file: %s permissions should only be user read/write (0600, -rw-------)\n",
file
);
return 0;
res = 0;
}
if(st.st_uid != getuid())
{
fprintf(stderr, "[-] file: %s not owned by current effective user id.\n",
file);
return 0;
res = 0;
}
#endif
return 1;
return res;
}
/***EOF***/

2
configure.ac
View File

@ -11,7 +11,7 @@ AC_PREREQ(2.62)
dnl Define our name, version and email.
m4_define(my_package, [fwknop])
m4_define(my_version, [2.0.2])
m4_define(my_version, [2.0.3])
m4_define(my_bug_email, [dstuart@dstuart.org])
AC_INIT(my_package, my_version, my_bug_email)

2
fwknop.spec
View File

@ -13,7 +13,7 @@
%define _mandir /usr/share/man
Name: fwknop
Version: 2.0.2
Version: 2.0.3
Epoch: 1
Release: 1%{?dist}
Summary: Firewall Knock Operator client. An implementation of Single Packet Authorization.

6
iphone/Classes/config.h
View File

@ -203,13 +203,13 @@ Copyright (C) Max Kastanas 2010
#define PACKAGE_NAME "fwknop"
/* Define to the full name and version of this package. */
#define PACKAGE_STRING "fwknop 2.0.2"
#define PACKAGE_STRING "fwknop 2.0.3"
/* Define to the one symbol short name of this package. */
#define PACKAGE_TARNAME "fwknop"
/* Define to the version of this package. */
#define PACKAGE_VERSION "2.0.2"
#define PACKAGE_VERSION "2.0.3"
/* The size of `unsigned int', as computed by sizeof. */
#define SIZEOF_UNSIGNED_INT 4
@ -243,7 +243,7 @@ Copyright (C) Max Kastanas 2010
/* Version number of package */
#define VERSION "2.0.2"
#define VERSION "2.0.3"
/* Define WORDS_BIGENDIAN to 1 if your processor stores words with the most
significant byte first (like Motorola and SPARC, unlike Intel). */

3
lib/fko.h
View File

@ -33,6 +33,7 @@
#include <time.h>
#include "fko_limits.h"
#include "fko_message.h"
#include "rijndael.h" /* For encryption modes */
#include "digest.h"
@ -57,7 +58,7 @@ extern "C" {
/* General params
*/
#define FKO_PROTOCOL_VERSION "2.0.2" /* The fwknop protocol version */
#define FKO_PROTOCOL_VERSION "2.0.3" /* The fwknop protocol version */
/* Supported FKO Message types...
*/

23
lib/fko_message.c
View File

@ -30,6 +30,7 @@
*****************************************************************************
*/
#include "fko_common.h"
#include "fko_message.h"
#include "fko.h"
/* Set the SPA message type.
@ -266,11 +267,16 @@ int
got_allow_ip(const char *msg)
{
const char *ndx = msg;
char ip_str[MAX_IPV4_STR_LEN];
int dot_ctr = 0, char_ctr = 0;
int res = FKO_SUCCESS;
#if HAVE_SYS_SOCKET_H
struct in_addr in;
#endif
while(*ndx != ',' && *ndx != '\0')
{
ip_str[char_ctr] = *ndx;
char_ctr++;
if(char_ctr >= MAX_IPV4_STR_LEN)
{
@ -287,12 +293,25 @@ got_allow_ip(const char *msg)
ndx++;
}
if (char_ctr < MIN_IPV4_STR_LEN)
if(char_ctr < MAX_IPV4_STR_LEN)
ip_str[char_ctr] = '\0';
else
res = FKO_ERROR_INVALID_ALLOW_IP;
if(dot_ctr != 3)
if ((res == FKO_SUCCESS) && (char_ctr < MIN_IPV4_STR_LEN))
res = FKO_ERROR_INVALID_ALLOW_IP;
if((res == FKO_SUCCESS) && dot_ctr != 3)
res = FKO_ERROR_INVALID_ALLOW_IP;
#if HAVE_SYS_SOCKET_H
/* Stronger IP validation now that we have a candidate that looks
* close enough
*/
if((res == FKO_SUCCESS) && (inet_aton(ip_str, &in) == 0))
res = FKO_ERROR_INVALID_ALLOW_IP;
#endif
return(res);
}

10
lib/fko_message.h
View File

@ -32,6 +32,16 @@
#ifndef FKO_MESSAGE_H
#define FKO_MESSAGE_H 1
#if PLATFORM_OPENBSD
#include <sys/types.h>
#include <netinet/in.h>
#else
#if HAVE_SYS_SOCKET_H
#include <sys/socket.h>
#endif
#endif
#include <arpa/inet.h>
#define MAX_PROTO_STR_LEN 5 /* tcp, udp, icmp for now */
#define MAX_PORT_STR_LEN 6

110
server/access.c
View File

@ -232,24 +232,6 @@ add_source_mask(fko_srv_options_t *opts, acc_stanza_t *acc, const char *ip)
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* If this is not the first entry, we walk our pointer to the
* end of the list.
*/
if(acc->source_list == NULL)
{
acc->source_list = new_sle;
}
else
{
tmp_sle = acc->source_list;
do {
last_sle = tmp_sle;
} while((tmp_sle = tmp_sle->next));
last_sle->next = new_sle;
}
/* Convert the IP data into the appropriate mask
*/
if(strcasecmp(ip, "ANY") == 0)
@ -264,12 +246,27 @@ add_source_mask(fko_srv_options_t *opts, acc_stanza_t *acc, const char *ip)
*/
if((ndx = strchr(ip, '/')) != NULL)
{
if(((ndx-ip)) >= MAX_IPV4_STR_LEN)
{
log_msg(LOG_ERR, "Error parsing string to IP");
free(new_sle);
new_sle = NULL;
return 0;
}
mask = atoi(ndx+1);
strlcpy(ip_str, ip, (ndx-ip)+1);
}
else
{
mask = 32;
if(strnlen(ip, MAX_IPV4_STR_LEN+1) >= MAX_IPV4_STR_LEN)
{
log_msg(LOG_ERR, "Error parsing string to IP");
free(new_sle);
new_sle = NULL;
return 0;
}
strlcpy(ip_str, ip, strlen(ip)+1);
}
@ -294,6 +291,25 @@ add_source_mask(fko_srv_options_t *opts, acc_stanza_t *acc, const char *ip)
*/
new_sle->maddr = ntohl(in.s_addr) & new_sle->mask;
}
/* If this is not the first entry, we walk our pointer to the
* end of the list.
*/
if(acc->source_list == NULL)
{
acc->source_list = new_sle;
}
else
{
tmp_sle = acc->source_list;
do {
last_sle = tmp_sle;
} while((tmp_sle = tmp_sle->next));
last_sle->next = new_sle;
}
return 1;
}
@ -396,7 +412,7 @@ parse_proto_and_port(char *pstr, int *proto, int *port)
/* Take a proto/port string and convert it to appropriate integer values
* for comparisons of incoming SPA requests.
*/
static void
static int
add_port_list_ent(acc_port_list_t **plist, char *port_str)
{
int proto_int, port;
@ -407,7 +423,7 @@ add_port_list_ent(acc_port_list_t **plist, char *port_str)
* are no problems with the incoming string.
*/
if(parse_proto_and_port(port_str, &proto_int, &port) != 0)
return;
return 0;
if((new_plist = calloc(1, sizeof(acc_port_list_t))) == NULL)
{
@ -437,6 +453,8 @@ add_port_list_ent(acc_port_list_t **plist, char *port_str)
new_plist->proto = proto_int;
new_plist->port = port;
return 1;
}
/* Add a string list entry to the given acc_string_list.
@ -510,7 +528,10 @@ expand_acc_port_list(acc_port_list_t **plist, char *plist_str)
return 0;
strlcpy(buf, start, (ndx-start)+1);
add_port_list_ent(plist, buf);
if(add_port_list_ent(plist, buf) == 0)
return 0;
start = ndx+1;
}
}
@ -525,14 +546,15 @@ expand_acc_port_list(acc_port_list_t **plist, char *plist_str)
strlcpy(buf, start, (ndx-start)+1);
add_port_list_ent(plist, buf);
if(add_port_list_ent(plist, buf) == 0)
return 0;
return 1;
}
/* Expand a comma-separated string into a simple acc_string_list.
*/
static void
static int
expand_acc_string_list(acc_string_list_t **stlist, char *stlist_str)
{
char *ndx, *start;
@ -549,6 +571,9 @@ expand_acc_string_list(acc_string_list_t **stlist, char *stlist_str)
while(isspace(*start))
start++;
if(((ndx-start)+1) >= 1024)
return 0;
strlcpy(buf, start, (ndx-start)+1);
add_string_list_ent(stlist, buf);
start = ndx+1;
@ -560,9 +585,14 @@ expand_acc_string_list(acc_string_list_t **stlist, char *stlist_str)
while(isspace(*start))
start++;
if(((ndx-start)+1) >= 1024)
return 0;
strlcpy(buf, start, (ndx-start)+1);
add_string_list_ent(stlist, buf);
return 1;
}
/* Free the acc source_list
@ -694,17 +724,29 @@ expand_acc_ent_lists(fko_srv_options_t *opts)
*/
if(expand_acc_source(opts, acc) == 0)
{
acc = acc->next;
continue;
log_msg(LOG_ERR, "Fatal invalid SOURCE in access stanza");
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
/* Now expand the open_ports string.
*/
if(acc->open_ports != NULL && strlen(acc->open_ports))
expand_acc_port_list(&(acc->oport_list), acc->open_ports);
{
if(expand_acc_port_list(&(acc->oport_list), acc->open_ports) == 0)
{
log_msg(LOG_ERR, "Fatal invalid OPEN_PORTS in access stanza");
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
if(acc->restrict_ports != NULL && strlen(acc->restrict_ports))
expand_acc_port_list(&(acc->rport_list), acc->restrict_ports);
{
if(expand_acc_port_list(&(acc->rport_list), acc->restrict_ports) == 0)
{
log_msg(LOG_ERR, "Fatal invalid RESTRICT_PORTS in access stanza");
clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
}
}
/* Expand the GPG_REMOTE_ID string.
*/
@ -1182,7 +1224,6 @@ parse_access_file(fko_srv_options_t *opts)
/* Expand our the expandable fields into their respective data buckets.
*/
expand_acc_ent_lists(opts);
/* Make sure default values are set where needed.
@ -1282,7 +1323,12 @@ acc_check_port_access(acc_stanza_t *acc, char *port_str)
return(0);
}
strlcpy(buf, start, (ndx-start)+1);
add_port_list_ent(&in_pl, buf);
if(add_port_list_ent(&in_pl, buf) == 0)
{
log_msg(LOG_ERR, "Invalid proto/port string");
return 0;
}
start = ndx+1;
ctr = 0;
}
@ -1298,7 +1344,11 @@ acc_check_port_access(acc_stanza_t *acc, char *port_str)
return(0);
}
strlcpy(buf, start, (ndx-start)+1);
add_port_list_ent(&in_pl, buf);
if(add_port_list_ent(&in_pl, buf) == 0)
{
log_msg(LOG_ERR, "Invalid proto/port string");
return 0;
}
if(in_pl == NULL)
{

3
server/fwknopd.c
View File

@ -677,11 +677,12 @@ get_running_pid(const fko_srv_options_t *opts)
pid_t rpid = 0;
verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]);
op_fd = open(opts->config[CONF_FWKNOP_PID_FILE], O_RDONLY);
if(op_fd > 0)
{
verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]);
if (read(op_fd, buf, PID_BUFLEN) > 0)
{
buf[PID_BUFLEN-1] = '\0';

21
server/utils.c
View File

@ -184,18 +184,27 @@ set_file_perms(const char *file)
int
verify_file_perms_ownership(const char *file)
{
int res = 1;
#if HAVE_STAT
struct stat st;
/* Every file that the fwknop client deals with should be owned
/* Every file that fwknopd deals with should be owned
* by the user and permissions set to 600 (user read/write)
*/
if((stat(file, &st)) != 0)
{
fprintf(stderr, "[-] unable to stat() file: %s: %s\n",
/* if the path doesn't exist, just return, but otherwise something
* went wrong
*/
if(errno == ENOENT)
{
return 0;
} else {
fprintf(stderr, "[-] stat() against file: %s returned: %s\n",
file, strerror(errno));
exit(EXIT_FAILURE);
}
}
/* Make sure it is a regular file
*/
@ -205,7 +214,7 @@ verify_file_perms_ownership(const char *file)
"[-] file: %s is not a regular file or symbolic link.\n",
file
);
return 0;
res = 0;
}
if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
@ -214,18 +223,18 @@ verify_file_perms_ownership(const char *file)
"[-] file: %s permissions should only be user read/write (0600, -rw-------)\n",
file
);
return 0;
res = 0;
}
if(st.st_uid != getuid())
{
fprintf(stderr, "[-] file: %s not owned by current effective user id\n",
file);
return 0;
res = 0;
}
#endif
return 1;
return res;
}
/* Determine if a buffer contains only characters from the base64

4
test/conf/fuzzing_open_ports_access.conf Normal file
View File

@ -0,0 +1,4 @@
SOURCE: 4.3.2.0/24, 127.0.0.0/24, 123.123.123.123/24, 23.43.0.0/16, 10.10.10.10;
OPEN_PORTS: udp/6001, tcp/22, tcp/80, tcp/123453;
KEY: fwknoptest;
FW_ACCESS_TIMEOUT: 3;

5
test/conf/fuzzing_restrict_ports_access.conf Normal file
View File

@ -0,0 +1,5 @@
SOURCE: 4.3.2.0/24, 127.0.0.0/24, 123.123.123.123/24, 23.43.0.0/16, 10.10.10.10;
OPEN_PORTS: udp/6001, tcp/22, tcp/80, tcp/12345;
RESTRICT_PORTS: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA;
KEY: fwknoptest;
FW_ACCESS_TIMEOUT: 3;

4
test/conf/fuzzing_source_access.conf Normal file
View File

@ -0,0 +1,4 @@
SOURCE: 4.3.2.0/24, 127.0.0.0/24, 123.123.123.1234/24, 23.43.0.0/16, A0.10.10.10;
OPEN_PORTS: udp/6001, tcp/22, tcp/80, tcp/12345;
KEY: fwknoptest;
FW_ACCESS_TIMEOUT: 3;

4
test/conf/open_ports_access.conf
View File

@ -1,4 +1,4 @@
SOURCE: 4.3.2.0/24, 127.0.0.0/24, 23.43.0.0/16, 10.10.10.10;
OPEN_PORTS: udp/6001, tcp/22, tcp/80;
SOURCE: 4.3.2.0/24, 127.0.0.0/24, 123.123.123.123/24, 23.43.0.0/16, 10.10.10.10;
OPEN_PORTS: udp/6001, tcp/22, tcp/80, tcp/12345;
KEY: fwknoptest;
FW_ACCESS_TIMEOUT: 3;

42
test/test-fwknop.pl
View File

@ -62,6 +62,9 @@ my %cf = (
'rc_file_hmac_b64_key' => "$conf_dir/fwknoprc_default_hmac_base64_key",
'base64_key_access' => "$conf_dir/base64_key_access.conf",
'disable_aging' => "$conf_dir/disable_aging_fwknopd.conf",
'fuzz_source' => "$conf_dir/fuzzing_source_access.conf",
'fuzz_open_ports' => "$conf_dir/fuzzing_open_ports_access.conf",
'fuzz_restrict_ports' => "$conf_dir/fuzzing_restrict_ports_access.conf",
);
my $default_digest_file = "$run_dir/digest.cache";
@ -1843,6 +1846,45 @@ my @tests = (
"-d $default_digest_file -p $default_pid_file $intf_str",
'fatal' => $NO
},
{
'category' => 'FUZZING',
'subcategory' => 'server',
'detail' => 'invalid SOURCE access.conf',
'err_msg' => 'server crashed or did not detect error condition',
'function' => \&generic_exec,
'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
"$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'fuzz_source'} " .
"-d $default_digest_file -p $default_pid_file $intf_str",
'positive_output_matches' => [qr/Fatal\sinvalid/],
'exec_err' => $YES,
'fatal' => $NO
},
{
'category' => 'FUZZING',
'subcategory' => 'server',
'detail' => 'invalid OPEN_PORTS access.conf',
'err_msg' => 'server crashed or did not detect error condition',
'function' => \&generic_exec,
'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
"$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'fuzz_open_ports'} " .
"-d $default_digest_file -p $default_pid_file $intf_str",
'positive_output_matches' => [qr/Fatal\sinvalid/],
'exec_err' => $YES,
'fatal' => $NO
},
{
'category' => 'FUZZING',
'subcategory' => 'server',
'detail' => 'invalid RESTRICT_PORTS access.conf',
'err_msg' => 'server crashed or did not detect error condition',
'function' => \&generic_exec,
'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
"$fwknopdCmd -c $cf{'disable_aging'} -a $cf{'fuzz_restrict_ports'} " .
"-d $default_digest_file -p $default_pid_file $intf_str",
'positive_output_matches' => [qr/Fatal\sinvalid/],
'exec_err' => $YES,
'fatal' => $NO
},
{
'category' => 'Rijndael SPA',

3
todo.org
View File

@ -9,6 +9,9 @@
*** Release fwknop-2.0.2
:CLOSED: <2012-08-18 Sat>
Make the fwknop-2.0.2 release.
*** Release fwknop-2.0.3
:CLOSED: <2012-09-03 Mon>
Make the fwknop-2.0.3 release.
*** Update fwknopd man page for GPG_ALLOW_NO_PW
:CLOSED: <2012-08-14 Tue>
*** Preserve existing configs under 'make install'