On Wed, 9 Nov 2016 13:16:46 +0000 Thomas Klute <thomas.kl...@achelos.de> wrote:
> Hi tech@, > > this patch contains fixes for two bugs that break IKE rekeying > initiated by iked. Please review, and apply or let me know what has to > be changed! Both bugs are fixed by initializing the respective > structures of the new IKE SA (struct iked_sa *nsa in the > ikev2_ike_sa_rekey function): Thanks, we are looking into it. > > For [1]: Copying the address information is required to send any > request messages over the new IKE SA after rekeying, otherwise errors > like the following happen because the IP addresses and ports remain > initialized to zero: > > ikev2_msg_send: INFORMATIONAL request from any to any msgid 1, 80 > bytes ikev2_msg_send: sendtofrom: Invalid argument > > For [2]: Setting the DH group based on the currently used one is > necessary because iked proposes only the currently used transforms > during IKE rekeying, so trying to use any other group for the DH > exchange will fail even if it is preferred by local policy (see > comment in the patch for details). > > This patch includes and supersedes the one for only the first bug I > sent yesterday. > > Best regards, > Thomas > > [1] https://marc.info/?l=openbsd-bugs&m=147739504516767&w=2 > [2] https://marc.info/?l=openbsd-bugs&m=147747405806461&w=2 > > Index: src/sbin/iked/ikev2.c > =================================================================== > RCS file: /cvs/src/sbin/iked/ikev2.c,v > retrieving revision 1.131 > diff -u -p -u -r1.131 ikev2.c > --- src/sbin/iked/ikev2.c 2 Jun 2016 07:14:26 -0000 > 1.131 +++ src/sbin/iked/ikev2.c 9 Nov 2016 13:12:32 -0000 > @@ -2658,6 +2658,18 @@ ikev2_ike_sa_rekey(struct iked *env, voi > goto done; > } > > + /* Select the DH group ID based on the currently used > + * one. Otherwise the call to ikev2_sa_initiator below would > + * set it to the first DH transform in the policy, while the > + * SA payload contains only one proposal matching the > + * currently used transforms. If a different DH transform has > + * been negotiated this means KE payload and negotiated DH > + * transform cannot match, causing rekeying to fail. */ > + if ((nsa->sa_dhgroup = group_get(sa->sa_dhgroup->id)) == > NULL) { > + log_debug("%s: failed to initialize DH group", > __func__); > + goto done; > + } > + > if (ikev2_sa_initiator(env, nsa, sa, NULL)) { > log_debug("%s: failed to setup DH", __func__); > goto done; > @@ -2665,6 +2677,13 @@ ikev2_ike_sa_rekey(struct iked *env, voi > sa_state(env, nsa, IKEV2_STATE_AUTH_SUCCESS); > nonce = nsa->sa_inonce; > > + /* Copy local and peer address from the old SA */ > + if (sa_address(nsa, &nsa->sa_peer, &sa->sa_peer.addr) == -1 > || > + sa_address(nsa, &nsa->sa_local, &sa->sa_local.addr) == > -1) { > + log_debug("%s: failed copy address data", __func__); > + goto done; > + } > + > if ((e = ibuf_static()) == NULL) > goto done; > >
