-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512 diff --git a/programs/pluto/crypt_dh.c b/programs/pluto/crypt_dh.c index ef98579..d0ef344 100644 - --- a/programs/pluto/crypt_dh.c +++ b/programs/pluto/crypt_dh.c @@ -69,6 +69,7 @@ /** Compute DH shared secret from our local secret and the peer's public value. * We make the leap that the length should be that of the group * (see quoted passage at start of ACCEPT_KE). + * If there is something that upsets NSS (what?) we will return NULL. */ /* MUST BE THREAD-SAFE */ PK11SymKey *calc_dh_shared(const chunk_t g, /* converted to SECItem */ @@ -115,44 +116,45 @@ PK11SymKey *calc_dh_shared(const chunk_t g, /* converted to SECItem */ CKM_CONCATENATE_DATA_AND_BASE, CKA_DERIVE, group->bytes, lsw_return_nss_password_file_info()); - - passert(dhshared != NULL); - - - - unsigned int shortfall = group->bytes - PK11_GetKeyLength(dhshared); - - - - if (shortfall > 0) { - - /* - - * We've got to pad the result with [shortfall] 0x00 bytes. - - * The chance of shortfall being n should be 1 in 256^n. - - * So really zauto ought to be big enough for the zeros. - - * If it isn't, we allocate space on the heap - - * (this will likely never be executed). - - */ - - DBG(DBG_CRYPT, - - DBG_log("restoring %u DHshared leading zeros", shortfall)); - - unsigned char zauto[10]; - - unsigned char *z = - - shortfall <= sizeof(zauto) ? - - zauto : alloc_bytes(shortfall, "DH shortfall"); - - memset(z, 0x00, shortfall); - - CK_KEY_DERIVATION_STRING_DATA string_params = { - - .pData = z, - - .ulLen = shortfall - - }; - - SECItem params = { - - .data = (unsigned char *)&string_params, - - .len = sizeof(string_params) - - }; - - PK11SymKey *newdhshared = - - PK11_Derive(dhshared, - - CKM_CONCATENATE_DATA_AND_BASE, - - ¶ms, - - CKM_CONCATENATE_DATA_AND_BASE, - - CKA_DERIVE, 0); - - passert(newdhshared != NULL); - - if (z != zauto) - - pfree(z); - - free_any_symkey("dhshared", &dhshared); - - dhshared = newdhshared; + + if (dhshared != NULL) { + unsigned int shortfall = group->bytes - PK11_GetKeyLength(dhshared); + + if (shortfall > 0) { + /* + * We've got to pad the result with [shortfall] 0x00 bytes. + * The chance of shortfall being n should be 1 in 256^n. + * So really zauto ought to be big enough for the zeros. + * If it isn't, we allocate space on the heap + * (this will likely never be executed). + */ + DBG(DBG_CRYPT, + DBG_log("restoring %u DHshared leading zeros", shortfall)); + unsigned char zauto[10]; + unsigned char *z = + shortfall <= sizeof(zauto) ? + zauto : alloc_bytes(shortfall, "DH shortfall"); + memset(z, 0x00, shortfall); + CK_KEY_DERIVATION_STRING_DATA string_params = { + .pData = z, + .ulLen = shortfall + }; + SECItem params = { + .data = (unsigned char *)&string_params, + .len = sizeof(string_params) + }; + PK11SymKey *newdhshared = + PK11_Derive(dhshared, + CKM_CONCATENATE_DATA_AND_BASE, + ¶ms, + CKM_CONCATENATE_DATA_AND_BASE, + CKA_DERIVE, 0); + passert(newdhshared != NULL); + if (z != zauto) + pfree(z); + free_any_symkey("dhshared", &dhshared); + dhshared = newdhshared; + } } *story = enum_name(&oakley_group_names, group->group); @@ -161,35 +163,34 @@ PK11SymKey *calc_dh_shared(const chunk_t g, /* converted to SECItem */ return dhshared; } +/* NOTE: if NSS refuses to calculate DH, skr->shared == NULL */ /* MUST BE THREAD-SAFE */ void calc_dh(struct pluto_crypto_req *r) { - - struct pcr_skeyid_r *skr = &r->pcr_d.dhr; - - struct pcr_skeyid_q dhq; - - const struct oakley_group_desc *group; - - chunk_t g; - - SECKEYPrivateKey *ltsecret; - - SECKEYPublicKey *pubk; - - const char *story = NULL; - - /* copy the request, since the reply will re-use the memory of the r->pcr_d.dhq */ + struct pcr_skeyid_q dhq; memcpy(&dhq, &r->pcr_d.dhq, sizeof(r->pcr_d.dhq)); /* clear out the reply */ + struct pcr_skeyid_r *skr = &r->pcr_d.dhr; zero(skr); /* ??? pointer fields might not be NULLed */ INIT_WIRE_ARENA(*skr); - - group = lookup_group(dhq.oakley_group); + const struct oakley_group_desc *group = lookup_group(dhq.oakley_group); passert(group != NULL); - - ltsecret = dhq.secret; - - pubk = dhq.pubk; + SECKEYPrivateKey *ltsecret = dhq.secret; + SECKEYPublicKey *pubk = dhq.pubk; /* now calculate the (g^x)(g^y) */ + chunk_t g; + setchunk_from_wire(g, &dhq, dhq.role == ORIGINAL_RESPONDER ? &dhq.gi : &dhq.gr); DBG(DBG_CRYPT, DBG_dump_chunk("peer's g: ", g)); + const char *story; /* we ignore the value */ + skr->shared = calc_dh_shared(g, ltsecret, group, pubk, &story); } diff --git a/programs/pluto/crypt_start_dh.c b/programs/pluto/crypt_start_dh.c index aab74a3..a3862df 100644 - --- a/programs/pluto/crypt_start_dh.c +++ b/programs/pluto/crypt_start_dh.c @@ -111,7 +111,7 @@ stf_status start_dh_secretiv(struct pluto_crypto_req_cont *dh, return send_crypto_helper_request(&r, dh); } - -void finish_dh_secretiv(struct state *st, +bool finish_dh_secretiv(struct state *st, struct pluto_crypto_req *r) { struct pcr_skeyid_r *dhr = &r->pcr_d.dhr; @@ -123,12 +123,17 @@ void finish_dh_secretiv(struct state *st, st->st_skeyid_e_nss = dhr->skeyid_e; st->st_enc_key_nss = dhr->enc_key; - - passert(dhr->new_iv.len <= MAX_DIGEST_LEN); - - passert(dhr->new_iv.len > 0); - - memcpy(st->st_new_iv, WIRE_CHUNK_PTR(*dhr, new_iv), dhr->new_iv.len); - - st->st_new_iv_len = dhr->new_iv.len; - - st->hidden_variables.st_skeyid_calculated = TRUE; + + if (st->st_shared_nss == NULL) { + return FALSE; + } else { + passert(dhr->new_iv.len <= MAX_DIGEST_LEN); + passert(dhr->new_iv.len > 0); + memcpy(st->st_new_iv, WIRE_CHUNK_PTR(*dhr, new_iv), dhr->new_iv.len); + st->st_new_iv_len = dhr->new_iv.len; + return TRUE; + } } stf_status start_dh_secret(struct pluto_crypto_req_cont *cn, @@ -257,7 +262,7 @@ stf_status start_dh_v2(struct msg_digest *md, } } - -void finish_dh_v2(struct state *st, +bool finish_dh_v2(struct state *st, const struct pluto_crypto_req *r) { const struct pcr_skeycalc_v2_r *dhv2 = &r->pcr_d.dhv2; @@ -276,4 +281,5 @@ void finish_dh_v2(struct state *st, st->st_skey_chunk_SK_pr = dhv2->skey_chunk_SK_pr; st->hidden_variables.st_skeyid_calculated = TRUE; + return st->st_shared_nss != NULL; /* was NSS happy to DH? */ } diff --git a/programs/pluto/ikev1.c b/programs/pluto/ikev1.c index f22698b..dbce175 100644 - --- a/programs/pluto/ikev1.c +++ b/programs/pluto/ikev1.c @@ -1643,8 +1643,8 @@ void process_packet_tail(struct msg_digest **mdp) return; } if (st->st_skey_ei_nss == NULL) { - - loglog(RC_LOG_SERIOUS, "discarding encrypted message" - - " because we haven't yet negotiated keying material"); + loglog(RC_LOG_SERIOUS, + "discarding encrypted message because we haven't yet negotiated keying material"); SEND_NOTIFICATION(INVALID_FLAGS); return; } diff --git a/programs/pluto/ikev1_aggr.c b/programs/pluto/ikev1_aggr.c index d388b55..f649bb2 100644 - --- a/programs/pluto/ikev1_aggr.c +++ b/programs/pluto/ikev1_aggr.c @@ -444,7 +444,8 @@ static stf_status aggr_inI1_outR1_tail(struct msg_digest *md, * so we have to build our reply_stream and emit HDR before calling it. */ - - finish_dh_secretiv(st, r); + if (!finish_dh_secretiv(st, r)) + return STF_FAIL + INVALID_KEY_INFORMATION; /* decode certificate requests */ ikev1_decode_cr(md, &requested_ca); @@ -767,9 +768,11 @@ static void aggr_inR1_outI2_crypto_continue(struct pluto_crypto_req_cont *dh, DBG(DBG_CONTROLMORE, DBG_log("#%lu %s:%u st->st_calculating = FALSE;", st->st_serialno, __FUNCTION__, __LINE__)); st->st_calculating = FALSE; - - finish_dh_secretiv(st, r); - - - - e = aggr_inR1_outI2_tail(md, NULL); + if (!finish_dh_secretiv(st, r)) { + e = STF_FAIL + INVALID_KEY_INFORMATION; + } else { + e = aggr_inR1_outI2_tail(md, NULL); + } passert(dh->pcrc_md != NULL); complete_v1_state_transition(&dh->pcrc_md, e); diff --git a/programs/pluto/ikev1_main.c b/programs/pluto/ikev1_main.c index 06ec4d4..8e1f05d 100644 - --- a/programs/pluto/ikev1_main.c +++ b/programs/pluto/ikev1_main.c @@ -1169,6 +1169,7 @@ stf_status main_inI2_outR2(struct msg_digest *md) * main_inI2_outR2_calcdone is unlike every other crypto_req_cont_func: * the state that it is working for may not yet care about the result. * We are precomputing the DH. + * This also means that it isn't good at reporting an NSS error. */ static crypto_req_cont_func main_inI2_outR2_calcdone; /* type assertion */ @@ -1191,10 +1192,8 @@ static void main_inI2_outR2_calcdone(struct pluto_crypto_req_cont *dh, set_cur_state(st); - - finish_dh_secretiv(st, r); - - - - st->hidden_variables.st_skeyid_calculated = TRUE; - - update_iv(st); + if (finish_dh_secretiv(st, r)) + update_iv(st); /* * If there was a packet received while we were calculating, then @@ -1448,7 +1447,8 @@ static stf_status main_inR2_outI3_continue(struct msg_digest *md, chunk_t auth_chain[MAX_CA_PATH_LEN] = { { NULL, 0 } }; int chain_len = 0; - - finish_dh_secretiv(st, r); + if (!finish_dh_secretiv(st, r)) + return STF_FAIL + INVALID_KEY_INFORMATION; /* decode certificate requests */ ikev1_decode_cr(md, &requested_ca); @@ -1979,7 +1979,10 @@ static key_tail_fn main_inI3_outR3_tail; /* forward */ stf_status main_inI3_outR3(struct msg_digest *md) { - - return main_inI3_outR3_tail(md, NULL); + /* handle case where NSS balked at generating DH */ + return md->st->st_shared_nss == NULL ? + STF_FAIL + INVALID_KEY_INFORMATION : + main_inI3_outR3_tail(md, NULL); } static inline stf_status main_id_and_auth(struct msg_digest *md, diff --git a/programs/pluto/ikev1_prf.c b/programs/pluto/ikev1_prf.c index 9f151d4..b5ade42 100644 - --- a/programs/pluto/ikev1_prf.c +++ b/programs/pluto/ikev1_prf.c @@ -283,67 +283,52 @@ static void calc_skeyids_iv(struct pcr_skeyid_q *skq, /* MUST BE THREAD-SAFE */ void calc_dh_iv(struct pluto_crypto_req *r) { - - struct pcr_skeyid_r *skr = &r->pcr_d.dhr; - - struct pcr_skeyid_q dhq; - - const struct oakley_group_desc *group; - - PK11SymKey *shared; - - chunk_t g; - - SECKEYPrivateKey *ltsecret; - - PK11SymKey - - *skeyid, - - *skeyid_d, - - *skeyid_a, - - *skeyid_e, - - *enc_key; - - chunk_t new_iv; - - SECKEYPublicKey *pubk; - - const char *story = NULL; - - /* copy the request, since the reply will re-use the memory of the r->pcr_d.dhq */ + struct pcr_skeyid_q dhq; memcpy(&dhq, &r->pcr_d.dhq, sizeof(r->pcr_d.dhq)); /* clear out the reply */ + struct pcr_skeyid_r *const skr = &r->pcr_d.dhr; zero(skr); /* ??? pointer fields may not be NULLed */ INIT_WIRE_ARENA(*skr); - - group = lookup_group(dhq.oakley_group); + const struct oakley_group_desc *group = lookup_group(dhq.oakley_group); passert(group != NULL); - - ltsecret = dhq.secret; - - pubk = dhq.pubk; + SECKEYPrivateKey *ltsecret = dhq.secret; + SECKEYPublicKey *pubk = dhq.pubk; - - /* now calculate the (g^x)(g^y) --- - - * need gi on responder, gr on initiator + /* + * Now calculate the (g^x)(g^y). + * Need gi on responder and gr on initiator. */ - - setchunk_from_wire(g, &dhq, dhq.role == ORIGINAL_RESPONDER ? &dhq.gi : &dhq.gr); + chunk_t g; + setchunk_from_wire(g, &dhq, + dhq.role == ORIGINAL_RESPONDER ? &dhq.gi : &dhq.gr); - - DBG(DBG_CRYPT, - - DBG_dump_chunk("peer's g: ", g)); + DBG(DBG_CRYPT, DBG_dump_chunk("peer's g: ", g)); - - shared = calc_dh_shared(g, ltsecret, group, pubk, &story); + const char *story; /* we don't use the value set in calc_dh_shared */ + skr->shared = calc_dh_shared(g, ltsecret, group, pubk, &story); - - new_iv = empty_chunk; + if (skr->shared != NULL) { + chunk_t new_iv = empty_chunk; - - /* okay, so now calculate IV */ - - calc_skeyids_iv(&dhq, - - shared, + /* okay, so now calculate IV */ + calc_skeyids_iv(&dhq, + skr->shared, dhq.key_size, - - &skeyid, - - &skeyid_d, - - &skeyid_a, - - &skeyid_e, - - &new_iv, - - &enc_key); - - - - skr->shared = shared; - - skr->skeyid = skeyid; - - skr->skeyid_d = skeyid_d; - - skr->skeyid_a = skeyid_a; - - skr->skeyid_e = skeyid_e; - - skr->enc_key = enc_key; - - - - - - WIRE_CLONE_CHUNK(*skr, new_iv, new_iv); - - freeanychunk(new_iv); + + &skr->skeyid, /* output */ + &skr->skeyid_d, /* output */ + &skr->skeyid_a, /* output */ + &skr->skeyid_e, /* output */ + &new_iv, /* output */ + &skr->enc_key /* output */ + ); + + WIRE_CLONE_CHUNK(*skr, new_iv, new_iv); + freeanychunk(new_iv); + } } diff --git a/programs/pluto/ikev2_parent.c b/programs/pluto/ikev2_parent.c index 5c3d2a7..623992a 100644 - --- a/programs/pluto/ikev2_parent.c +++ b/programs/pluto/ikev2_parent.c @@ -2105,7 +2105,8 @@ static stf_status ikev2_parent_inR1outI2_tail( struct state *const pst = md->st; /* parent's state object */ struct connection *const pc = pst->st_connection; /* parent connection */ - - finish_dh_v2(pst, r); + if (!finish_dh_v2(pst, r)) + return STF_FAIL + v2N_INVALID_KE_PAYLOAD; /* ??? this is kind of odd: regular control flow only selecting DBG output */ if (DBGP(DBG_PRIVATE) && DBGP(DBG_CRYPT)) @@ -2692,7 +2693,8 @@ static stf_status ikev2_parent_inI2outR2_tail( unsigned char idhash_in[MAX_DIGEST_LEN]; /* extract calculated values from r */ - - finish_dh_v2(st, r); + if (!finish_dh_v2(st, r)) + return STF_FAIL + v2N_INVALID_KE_PAYLOAD; /* ??? this is kind of odd: regular control flow only selecting DBG output */ if (DBGP(DBG_PRIVATE) && DBGP(DBG_CRYPT)) diff --git a/programs/pluto/ikev2_prf.c b/programs/pluto/ikev2_prf.c index 016878b..2da07af 100644 - --- a/programs/pluto/ikev2_prf.c +++ b/programs/pluto/ikev2_prf.c @@ -209,31 +209,14 @@ static void calc_skeyseed_v2(struct pcr_skeyid_q *skq, DBG_dump_chunk("calc_skeyseed_v2 SK_pr", chunk_SK_pr)); } +/* NOTE: if NSS refuses to calculate DH, skr->shared == NULL */ /* MUST BE THREAD-SAFE */ void calc_dh_v2(struct pluto_crypto_req *r, const char **story) { - - struct pcr_skeycalc_v2_r *skr = &r->pcr_d.dhv2; - - struct pcr_skeyid_q dhq; - - const struct oakley_group_desc *group; - - PK11SymKey *shared; - - chunk_t g; - - SECKEYPrivateKey *ltsecret; - - PK11SymKey *skeyseed; - - PK11SymKey - - *SK_d, - - *SK_ai, - - *SK_ar, - - *SK_ei, - - *SK_er, - - *SK_pi, - - *SK_pr; - - chunk_t initiator_salt; - - chunk_t responder_salt; - - chunk_t chunk_SK_pi; - - chunk_t chunk_SK_pr; - - SECKEYPublicKey *pubk; + struct pcr_skeycalc_v2_r *const skr = &r->pcr_d.dhv2; /* copy the request, since the reply will re-use the memory of the r->pcr_d.dhq */ + struct pcr_skeyid_q dhq; memcpy(&dhq, &r->pcr_d.dhq, sizeof(r->pcr_d.dhq)); /* clear out the reply (including pointers) */ @@ -241,52 +224,41 @@ void calc_dh_v2(struct pluto_crypto_req *r, const char **story) *skr = zero_pcr_skeycalc_v2_r; INIT_WIRE_ARENA(*skr); - - group = lookup_group(dhq.oakley_group); + const struct oakley_group_desc *group = lookup_group(dhq.oakley_group); passert(group != NULL); - - ltsecret = dhq.secret; - - pubk = dhq.pubk; + SECKEYPrivateKey *ltsecret = dhq.secret; + SECKEYPublicKey *pubk = dhq.pubk; /* now calculate the (g^x)(g^y) --- need gi on responder, gr on initiator */ + chunk_t g; setchunk_from_wire(g, &dhq, dhq.role == ORIGINAL_RESPONDER ? &dhq.gi : &dhq.gr); DBG(DBG_CRYPT, DBG_dump_chunk("peer's g: ", g)); - - shared = calc_dh_shared(g, ltsecret, group, pubk, story); + skr->shared = calc_dh_shared(g, ltsecret, group, pubk, story); + if (skr->shared != NULL) { /* okay, so now all the shared key material */ - - calc_skeyseed_v2(&dhq, /* input */ - - shared, /* input */ - - dhq.key_size, /* input */ - - dhq.salt_size, /* input */ - - - - &skeyseed, /* output */ - - &SK_d, /* output */ - - &SK_ai, /* output */ - - &SK_ar, /* output */ - - &SK_ei, /* output */ - - &SK_er, /* output */ - - &SK_pi, /* output */ - - &SK_pr, /* output */ - - &initiator_salt, /* output */ - - &responder_salt, /* output */ - - &chunk_SK_pi, /* output */ - - &chunk_SK_pr); /* output */ - - - - skr->shared = shared; - - skr->skeyseed = skeyseed; - - skr->skeyid_d = SK_d; - - skr->skeyid_ai = SK_ai; - - skr->skeyid_ar = SK_ar; - - skr->skeyid_ei = SK_ei; - - skr->skeyid_er = SK_er; - - skr->skeyid_pi = SK_pi; - - skr->skeyid_pr = SK_pr; - - skr->skey_initiator_salt = initiator_salt; - - skr->skey_responder_salt = responder_salt; - - skr->skey_chunk_SK_pi = chunk_SK_pi; - - skr->skey_chunk_SK_pr = chunk_SK_pr; + calc_skeyseed_v2(&dhq, /* input */ + skr->shared, /* input */ + dhq.key_size, /* input */ + dhq.salt_size, /* input */ + + &skr->skeyseed, /* output */ + &skr->skeyid_d, /* output */ + &skr->skeyid_ai, /* output */ + &skr->skeyid_ar, /* output */ + &skr->skeyid_ei, /* output */ + &skr->skeyid_er, /* output */ + &skr->skeyid_pi, /* output */ + &skr->skeyid_pr, /* output */ + &skr->skey_initiator_salt, /* output */ + &skr->skey_responder_salt, /* output */ + &skr->skey_chunk_SK_pi, /* output */ + &skr->skey_chunk_SK_pr); /* output */ + } } static PK11SymKey *ikev2_prfplus(const struct hash_desc *hasher, diff --git a/programs/pluto/pluto_crypt.h b/programs/pluto/pluto_crypt.h index c5426fe..a0b9869 100644 - --- a/programs/pluto/pluto_crypt.h +++ b/programs/pluto/pluto_crypt.h @@ -369,7 +369,7 @@ extern stf_status start_dh_secretiv(struct pluto_crypto_req_cont *dh, enum original_role role, oakley_group_t oakley_group2); - -extern void finish_dh_secretiv(struct state *st, +extern bool finish_dh_secretiv(struct state *st, struct pluto_crypto_req *r); extern stf_status start_dh_secret(struct pluto_crypto_req_cont *cn, @@ -386,7 +386,7 @@ extern stf_status start_dh_v2(struct msg_digest *md, enum original_role role, crypto_req_cont_func pcrc_func); - -extern void finish_dh_v2(struct state *st, +extern bool finish_dh_v2(struct state *st, const struct pluto_crypto_req *r); extern void unpack_KE_from_helper( -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJV223KAAoJEIX/S0OzD8b5XeYQAIDGgfXfSnQc/RlBgZvDEoMP RtsdsAN8R8SSUX4TRBQZZy3Bij8pX+Cozpdm9FPqM5gGwny3NZ5miHaQ7kx9v6R5 aNqy5cbx7Hso2FFSQHT/i7vTIzxR1cSkFIyP563hG3kC6qEZsIIYjN5Y4RsWcosx /UC0YIOQJsG4XeEV3Hmyv8u3YWnX0gIlTUQChzESepUTPE2731NYC7XNb5FzyYas +aBprzugm7mJZe+DrfcY/YcY8WS+hXb9Pd3BSLyUGJsDSgV00owcL1hsjeyRcpqt wJNoG+pj6l8mqvqGa40suDqu1x+UaysIbN3aexw9CuxP5DINGAkrx4aincz/5UjG D0Xa9O6V57IBm26P+O2fKE1ReW5fpbN02DSue4eGYdTEXcaQ5lz+hA59fZmOkeom 0O1wnXJ6AZnd29TPtHgTBfyO3U2xXBncol/c9fzFVTy/3vreICGsk0XYOUTiMh5u ggVXWSCd5eUV4BhrMjgRsuWDmmUrystnV/gFCs1rFEgHB8Rx7w0Q9pKEGV9Y2Tj1 QtXkz+Fe9bJzONq1qs5yk1rpjo0kSRdt1/ux8hqZopJquLAV6KAVpHGBcMtTBcqr Bebfyo4AvE/p2XknqNrM14PPDubSCG14XGKC6JKaAwvtPL1t9CBthLkYCcVrl02L y9hlPAn6Cc4MwPnlfb6l =UFlu -----END PGP SIGNATURE-----