From: Michael Rash Date: Sat, 21 Jul 2012 19:32:15 +0000 (-0400) Subject: Better SPA message validation upon SPA decrypt/decode. X-Git-Tag: fwknop-2.0.1-pre5~7 X-Git-Url: http://www.cipherdyne.com/cgi-bin/gitweb.cgi?p=fwknop.git;a=commitdiff_plain;h=5ef07c73e2a6bcecbc3c9914340cc63c266f816b Better SPA message validation upon SPA decrypt/decode. Added SPA message validation calls to fko decoding routines to help ensure that SPA messages conform to expected values. --- diff --git a/ChangeLog b/ChangeLog index de743bd..c077db4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,8 +9,10 @@ fwknop-2.0.1 (07//2012): digest list right after the first access.conf stanza match, so when SPA packet data matched the second access.conf stanza a matching replay digest would already be there. + - Added SPA message validation calls to fko decoding routines to help + ensure that SPA messages conform to expected values. - Bug fix for PF firewalls: updated the PF anchor check to not rely on - listing the PF policy - use 'pfctl -s Anchor' instead. + listing the PF policy - fwknopd now uses 'pfctl -s Anchor' instead. - [test suite] Added parsing of valgrind output to produce a listing of functions that have been flagged - this assists in the development process to ensure that fwknop is not leaking memory. diff --git a/Makefile.am b/Makefile.am index aaa8001..2aac555 100644 --- a/Makefile.am +++ b/Makefile.am @@ -103,10 +103,10 @@ EXTRA_DIST = \ perl/FKO/lib/FKO.pm \ perl/FKO/lib/FKO_Constants.pl \ perl/FKO/Changes \ - python/README \ - python/setup.py \ - python/fkomodule.c \ - python/fko.py \ + python/README \ + python/setup.py \ + python/fkomodule.c \ + python/fko.py \ ShortLog* \ test/conf/client-gpg/pubring.gpg \ test/conf/client-gpg/secring.gpg \ diff --git a/common/common.h b/common/common.h index c063cfd..0c1c26d 100644 --- a/common/common.h +++ b/common/common.h @@ -104,8 +104,6 @@ enum { #define MAX_PORT 65535 #define MAX_PORT_STR_LEN 6 #define MAX_PROTO_STR_LEN 6 -#define MAX_IPV4_STR_LEN 16 -#define MIN_IPV4_STR_LEN 9 #define MAX_SERVER_STR_LEN 50 #define MAX_LINE_LEN 1024 diff --git a/lib/Makefile.am b/lib/Makefile.am index ba6e09a..b0ee5a7 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -4,8 +4,8 @@ libfko_source_files = \ base64.c base64.h cipher_funcs.c cipher_funcs.h digest.c digest.h \ fko_client_timeout.c fko_common.h fko_digest.c fko_encode.c \ fko_decode.c fko_encryption.c fko_error.c fko_funcs.c fko_message.c \ - fko_nat_access.c fko_rand_value.c fko_server_auth.c fko.h fko_limits.h \ - fko_timestamp.c fko_user.c fko_util.h md5.c md5.h \ + fko_message.h fko_nat_access.c fko_rand_value.c fko_server_auth.c \ + fko.h fko_limits.h fko_timestamp.c fko_user.c fko_util.h md5.c md5.h \ rijndael.c rijndael.h sha1.c sha1.h sha2.c sha2.h strlcat.c \ strlcpy.c fko_state.h fko_context.h gpgme_funcs.c gpgme_funcs.h diff --git a/lib/fko.h b/lib/fko.h index 1f5af83..64f4b12 100644 --- a/lib/fko.h +++ b/lib/fko.h @@ -32,6 +32,7 @@ #define FKO_H 1 #include +#include "fko_limits.h" #ifdef __cplusplus extern "C" { diff --git a/lib/fko_common.h b/lib/fko_common.h index c874b20..b86e6e8 100644 --- a/lib/fko_common.h +++ b/lib/fko_common.h @@ -120,6 +120,7 @@ #include "fko_limits.h" #include "fko_state.h" #include "fko_context.h" +#include "fko_message.h" /* Try to cover for those that do not have bzero. */ diff --git a/lib/fko_decode.c b/lib/fko_decode.c index abc2975..5b3b7e7 100644 --- a/lib/fko_decode.c +++ b/lib/fko_decode.c @@ -281,6 +281,14 @@ fko_decode_spa_data(fko_ctx_t ctx) b64_decode(tbuf, (unsigned char*)ctx->message); + /* Require a message similar to: 1.2.3.4,tcp/22 + */ + if(validate_access_msg(ctx->message) != FKO_SUCCESS) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + /* Extract nat_access string if the message_type indicates so. */ if( ctx->message_type == FKO_NAT_ACCESS_MSG diff --git a/lib/fko_limits.h b/lib/fko_limits.h index 97d2e66..1eeba5f 100644 --- a/lib/fko_limits.h +++ b/lib/fko_limits.h @@ -48,6 +48,9 @@ #define MIN_SPA_FIELDS 6 #define MAX_SPA_FIELDS 10 +#define MAX_IPV4_STR_LEN 16 +#define MIN_IPV4_STR_LEN 7 + /* Misc. */ #define FKO_ENCODE_TMP_BUF_SIZE 1024 diff --git a/lib/fko_message.c b/lib/fko_message.c index fcb3ac2..1a9051a 100644 --- a/lib/fko_message.c +++ b/lib/fko_message.c @@ -32,15 +32,6 @@ #include "fko_common.h" #include "fko.h" -/* SPA message format validation functions. - * (These called from the spa_message function here only). -*/ -int validate_cmd_msg(const char *msg); -int validate_access_msg(const char *msg); -int validate_proto_port_spec(const char *msg); -int validate_nat_access_msg(const char *msg); -int got_allow_ip(const char *msg); - /* Set the SPA message type. */ int @@ -90,13 +81,13 @@ fko_set_spa_message(fko_ctx_t ctx, const char *msg) /* Gotta have a valid string. */ - if(msg == NULL || strlen(msg) == 0) + if(msg == NULL || strnlen(msg, MAX_SPA_MESSAGE_SIZE) == 0) return(FKO_ERROR_INVALID_DATA); /* --DSS XXX: Bail out for now. But consider just * truncating in the future... */ - if(strlen(msg) > MAX_SPA_MESSAGE_SIZE) + if(strnlen(msg, MAX_SPA_MESSAGE_SIZE) == MAX_SPA_MESSAGE_SIZE) return(FKO_ERROR_DATA_TOO_LARGE); /* Basic message type and format checking... diff --git a/lib/fko_message.h b/lib/fko_message.h new file mode 100644 index 0000000..f56e33f --- /dev/null +++ b/lib/fko_message.h @@ -0,0 +1,45 @@ +/* + ***************************************************************************** + * + * File: fko_message.h + * + * Author: Michael Rash + * + * Purpose: Provide validation functions for SPA messages + * + * Copyright 2012 Michael Rash (mbr@cipherdyne.org) + * + * License (GNU Public License): + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 + * USA + * + ***************************************************************************** +*/ + +#ifndef FKO_MESSAGE_H +#define FKO_MESSAGE_H 1 + +/* SPA message format validation functions. +*/ +int validate_cmd_msg(const char *msg); +int validate_access_msg(const char *msg); +int validate_proto_port_spec(const char *msg); +int validate_nat_access_msg(const char *msg); +int got_allow_ip(const char *msg); + +#endif /* FKO_MESSAGE_H */ + +/***EOF***/ diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 2c33192..07f0920 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -569,7 +569,21 @@ incoming_spa(fko_srv_options_t *opts) continue; } - strlcpy(spadat.spa_message_src_ip, spadat.spa_message, (spa_ip_demark-spadat.spa_message)+1); + strlcpy(spadat.spa_message_src_ip, + spadat.spa_message, (spa_ip_demark-spadat.spa_message)+1); + + if(strnlen(spadat.spa_message_src_ip, + MIN_IPV4_STR_LEN) < MIN_IPV4_STR_LEN) + { + log_msg(LOG_WARNING, "(stanza #%d) Invalid source IP in SPA message, ignoring SPA packet", + stanza_num, fko_errstr(res)); + + if(ctx != NULL) + fko_destroy(ctx); + acc = acc->next; + break; + } + strlcpy(spadat.spa_message_remain, spa_ip_demark+1, 1024); /* If use source IP was requested (embedded IP of 0.0.0.0), make sure it