6900753 Calls to dump_key in ikeadm.c could be refactored

6896962 ipsecconf incorrectly parses misconfigured hyphenated tokens
6898695 ipsecalgs -s causes kernel buffer corruption
6440628 ipseckey should ensure that argument is a file before parsing

5 files changed, 57 insertions, 22 deletions

diff --git a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c
index 1a7e30f77f..76b786b946 100644
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c
+++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c
@@ -1798,50 +1798,40 @@ print_keys(char *prefix, ike_p1_key_t *keyp, int size)
 		case IKE_KEY_PRESHARED:
 			(void) printf(gettext("%s Pre-shared key (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_SKEYID:
 			(void) printf(gettext("%s SKEYID (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_SKEYID_D:
 			(void) printf(gettext("%s SKEYID_d (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_SKEYID_A:
 			(void) printf(gettext("%s SKEYID_a (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_SKEYID_E:
 			(void) printf(gettext("%s SKEYID_e (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_ENCR:
 			(void) printf(gettext("%s Encryption key (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		case IKE_KEY_IV:
 			(void) printf(
 			    gettext("%s Initialization vector (%d bytes): "),
 			    prefix, len);
-			(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
-			    stdout, B_FALSE);
 			break;
 		default:
 			(void) printf(gettext("%s Unidentified key info %p %d"),
 			    prefix, p, p1klen);
+			goto badkey;
 		}
+		(void) dump_key((uint8_t *)(p + 1), SADB_8TO1(len), 0,
+		    stdout, B_FALSE);
+badkey:
 		(void) printf("\n");
 		assert(IS_P2ALIGNED(p1klen, 8));
 		curp += (p1klen >> 2);
diff --git a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c
index 0e1b1d06f5..41a7484fb5 100644
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c
+++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c
@@ -3456,6 +3456,7 @@ static int
 parse_ipsec_alg(char *str, ips_act_props_t *iap, int alg_type)
 {
 	int alg_value;
+	int remainder;
 	char tstr[VALID_ALG_LEN];
 	char *lens = NULL;
 	char *l1_str;
@@ -3471,8 +3472,10 @@ parse_ipsec_alg(char *str, ips_act_props_t *iap, int alg_type)
 	 * Make sure that we get a null terminated string.
 	 * For a bad input, we truncate at VALID_ALG_LEN.
 	 */
+	remainder = strlen(str);
 	(void) strlcpy(tstr, str, VALID_ALG_LEN);
 	lens = strtok(tstr, "()");
+	remainder -= strlen(lens);
 	lens = strtok(NULL, "()");
 
 	if (lens != NULL) {
@@ -3480,6 +3483,15 @@ parse_ipsec_alg(char *str, ips_act_props_t *iap, int alg_type)
 		int len2 = SPD_MAX_MAXBITS;
 		int len_all = strlen(lens);
 		int dot_start = (lens[0] == '.');
+
+		/*
+		 * Check to see if the keylength arg is at the end of the
+		 * token, the "()" is 2 characters.
+		 */
+		remainder -= strlen(lens);
+		if (remainder > 2)
+			return (1);
+
 		l1_str = strtok(lens, ".");
 		l2_str = strtok(NULL, ".");
 		if (l1_str != NULL) {
diff --git a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c
index 24f3410dde..175b6f348a 100644
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c
+++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c
@@ -3613,14 +3613,40 @@ main(int argc, char *argv[])
 		case 'f':
 			if (dosave)
 				usage();
+
+			/*
+			 * Use stat() to check and see if the user inadvertently
+			 * passed in a bad pathname, or the name of a directory.
+			 * We should also check to see if the filename is a
+			 * pipe. We use stat() here because fopen() will block
+			 * unless the other end of the pipe is open. This would
+			 * be undesirable, especially if this is called at boot
+			 * time. If we ever need to support reading from a pipe
+			 * or special file, this should be revisited.
+			 */
+			if (stat(optarg, &sbuf) == -1) {
+				EXIT_BADCONFIG2("Invalid pathname: %s\n",
+				    optarg);
+			}
+			if (!(sbuf.st_mode & S_IFREG)) {
+				EXIT_BADCONFIG2("%s - Not a regular file\n",
+				    optarg);
+			}
 			infile = fopen(optarg, "r");
 			if (infile == NULL) {
 				EXIT_BADCONFIG2("Unable to open configuration "
 				    "file: %s\n", optarg);
 			}
 			/*
-			 * Check file permissions/ownership and warn or
-			 * fail depending on state of SMF control.
+			 * The input file contains keying information, because
+			 * this is sensative, we should only accept data from
+			 * this file if the file is root owned and only readable
+			 * by privileged users. If the command is being run by
+			 * the administrator, issue a warning, if this is run by
+			 * smf(5) (IE: boot time) and the permissions are too
+			 * open, we will fail, the SMF service will end up in
+			 * maintenace mode. The check is made with fstat() to
+			 * eliminate any possible TOT to TOU window.
 			 */
 			if (fstat(fileno(infile), &sbuf) == -1) {
 				(void) fclose(infile);
@@ -3634,10 +3660,10 @@ main(int argc, char *argv[])
 					    "%s has insecure permissions.",
 					    optarg);
 				} else 	{
-					(void) fprintf(stderr, "%s %s\n",
-					    optarg, gettext(
-					    "has insecure permissions, will be "
-					    "rejected in permanent config."));
+					(void) fprintf(stderr, gettext(
+					    "Config file %s has insecure "
+					    "permissions, will be rejected in "
+					    "permanent config.\n"), optarg);
 				}
 			}
 			configfile = strdup(optarg);
diff --git a/usr/src/uts/common/inet/ip/spd.c b/usr/src/uts/common/inet/ip/spd.c
index 229795b2c3..4d53f76c2b 100644
--- a/usr/src/uts/common/inet/ip/spd.c
+++ b/usr/src/uts/common/inet/ip/spd.c
@@ -4958,6 +4958,11 @@ ipsec_alg_free(ipsec_alginfo_t *alg)
 		    (alg->alg_nblock_sizes + 1) * sizeof (uint16_t));
 		alg->alg_block_sizes = NULL;
 	}
+	if (alg->alg_params != NULL) {
+		kmem_free(alg->alg_params,
+		    (alg->alg_nparams + 1) * sizeof (uint16_t));
+		alg->alg_params = NULL;
+	}
 	kmem_free(alg, sizeof (*alg));
 }
 
diff --git a/usr/src/uts/common/inet/ip/spdsock.c b/usr/src/uts/common/inet/ip/spdsock.c
index 1b25af4a97..5a26f6e612 100644
--- a/usr/src/uts/common/inet/ip/spdsock.c
+++ b/usr/src/uts/common/inet/ip/spdsock.c
@@ -2549,6 +2549,7 @@ spdsock_do_updatealg(spd_ext_t *extv[], spd_stack_t *spds)
 
 #define	ALG_KEY_SIZES(a)   (((a)->alg_nkey_sizes + 1) * sizeof (uint16_t))
 #define	ALG_BLOCK_SIZES(a) (((a)->alg_nblock_sizes + 1) * sizeof (uint16_t))
+#define	ALG_PARAM_SIZES(a) (((a)->alg_nparams + 1) * sizeof (uint16_t))
 
 	while (attr < endattr) {
 		switch (attr->spd_attr_tag) {
@@ -2665,14 +2666,14 @@ spdsock_do_updatealg(spd_ext_t *extv[], spd_stack_t *spds)
 		case SPD_ATTR_ALG_NPARAMS:
 			if (alg->alg_params != NULL) {
 				kmem_free(alg->alg_params,
-				    ALG_BLOCK_SIZES(alg));
+				    ALG_PARAM_SIZES(alg));
 			}
 			alg->alg_nparams = attr->spd_attr_value;
 			/*
 			 * Allocate room for the trailing zero block size
 			 * value as well.
 			 */
-			alg->alg_params = kmem_zalloc(ALG_BLOCK_SIZES(alg),
+			alg->alg_params = kmem_zalloc(ALG_PARAM_SIZES(alg),
 			    KM_SLEEP);
 			cur_block = 0;
 			break;
@@ -2737,6 +2738,7 @@ spdsock_do_updatealg(spd_ext_t *extv[], spd_stack_t *spds)
 
 #undef	ALG_KEY_SIZES
 #undef	ALG_BLOCK_SIZES
+#undef	ALG_PARAM_SIZES
 
 	/* update the algorithm tables */
 	spdsock_merge_algs(spds);