On Wed, 2016-10-19 at 08:39 -0700, Daniel Lenski wrote:
> -???????unsigned char secrets[0x40];
> +???????unsigned char secrets[0x40]; /* Encryption key bytes, then HMAC key bytes */
You're allowed to object to that horridness and split it into two
separate fields for the encryption and HMAC keys, instead of just
documenting it.
In fact, one might argue that would be the better approach...
> -???????vpninfo->old_esp_maxseq = vpninfo->esp_in[vpninfo->current_esp_in].seq + 32;
> -???????vpninfo->current_esp_in ^= 1;
> +???????if (new_keys) {
> +???????????????vpninfo->old_esp_maxseq = vpninfo->esp_in[vpninfo->current_esp_in].seq + 32;
> +???????????????vpninfo->current_esp_in ^= 1;
> +???????}
Hm, do you not want that part in setup_esp_keys()? That's where we flip
to the other incoming ESP setup,?while still allowing a few more
packets to be received on the existing one. Is there really no
rekeying?
Perhaps that part wants moving *out* of setup_esp_keys() to it callers?
And in fact perhaps you could do the new_keys bit that way too? Just
make an oncp_setup_esp_keys() function which creates the new keys and
*then* calls the generic setup_esp_keys()? Note that the flip to the
new incoming ESP setup has to be done *first*, or at least taken into
account.
--
dwmw2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5760 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20161215/86c1ffeb/attachment-0001.bin>
