Mercurial > illumos > illumos-gate
changeset 11136:5049dc761777
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
author | Mark Fenwick <Mark.Fenwick@Sun.COM> |
---|---|
date | Fri, 20 Nov 2009 14:54:27 -0800 |
parents | 5b5c2c84a4d5 |
children | 0a1ed2ae30eb |
files | usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c usr/src/uts/common/inet/ip/spd.c usr/src/uts/common/inet/ip/spdsock.c |
diffstat | 5 files changed, 57 insertions(+), 22 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c Fri Nov 20 13:51:47 2009 -0800 +++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ikeadm.c Fri Nov 20 14:54:27 2009 -0800 @@ -1798,50 +1798,40 @@ 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);
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c Fri Nov 20 13:51:47 2009 -0800 +++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipsecconf.c Fri Nov 20 14:54:27 2009 -0800 @@ -3456,6 +3456,7 @@ 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 @@ * 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 @@ 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) {
--- a/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c Fri Nov 20 13:51:47 2009 -0800 +++ b/usr/src/cmd/cmd-inet/usr.sbin/ipsecutils/ipseckey.c Fri Nov 20 14:54:27 2009 -0800 @@ -3613,14 +3613,40 @@ 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 @@ "%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);
--- a/usr/src/uts/common/inet/ip/spd.c Fri Nov 20 13:51:47 2009 -0800 +++ b/usr/src/uts/common/inet/ip/spd.c Fri Nov 20 14:54:27 2009 -0800 @@ -4958,6 +4958,11 @@ (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)); }
--- a/usr/src/uts/common/inet/ip/spdsock.c Fri Nov 20 13:51:47 2009 -0800 +++ b/usr/src/uts/common/inet/ip/spdsock.c Fri Nov 20 14:54:27 2009 -0800 @@ -2549,6 +2549,7 @@ #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 @@ 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 @@ #undef ALG_KEY_SIZES #undef ALG_BLOCK_SIZES +#undef ALG_PARAM_SIZES /* update the algorithm tables */ spdsock_merge_algs(spds);