[PATCH] Save latest ESP sequence number even if replay protection isn't in use

[Daniel Lenski]

On Mon, Jan 8, 2018 at 12:30 AM, David Woodhouse <dwmw2 at infradead.org> wrote:
> On Sun, 2018-01-07 at 17:54 -0800, Daniel Lenski wrote:
>>
>> This patch tracks the latest sequence number even if ESP replay protection
>> isn't in use -- however inadvisable that may be -- allowing the handover to
>> work correctly.
>
> This implies that the seq# *is* being set in these packets. So we come
> back to my question in the source code from three years ago:
>
>        /* Why in $DEITY's name would you ever *not* set this? Perhaps we
>         * should do th check anyway, but only warn instead of discarding
>         * the packet? */
>        if (vpninfo->esp_replay_protect &&
>
> (Shudder. I hate seeing old typos of my own)
>

Oh yes, I enjoyed this comment :-). I have access to at least one
Juniper VPN that disables replay protection, so when I observed this I
realized it was probably the reason why Oracle connections often hang
after a couple hours.

Perhaps the correct solution here is to turn replay protection on as a
warning but not a fatal error, as you suggest.

The other alternative to this patch is just to ignore the sequence
numbers entirely during the rekey handover (if replay protection is
off), as follows. This is all assuming that the intention was *not* to
prevent the rekey handover from working at all on VPNs that turn it
off…?

diff --git a/esp.c b/esp.c
index 1b4b227..d869350 100644
--- a/esp.c
+++ b/esp.c
@@ -297,7 +297,7 @@ int esp_mainloop(struct openconnect_info *vpninfo,
int *timeout)
  if (decrypt_esp_packet(vpninfo, esp, pkt))
  continue;
  } else if (pkt->esp.spi == old_esp->spi &&
-    ntohl(pkt->esp.seq) + esp->seq < vpninfo->old_esp_maxseq) {
+    vpninfo->esp_replay_protect ? ntohl(pkt->esp.seq) + esp->seq <
vpninfo->old_esp_maxseq : 1) {
  vpn_progress(vpninfo, PRG_TRACE,
-      _("Consider SPI 0x%x, seq %u against outgoing ESP setup\n"),
+      _("Received ESP packet from old SPI 0x%x, seq %u\n"),
       _("Received ESP packet from old SPI 0x%x, seq %u\n"),
       (unsigned)ntohl(old_esp->spi), (unsigned)ntohl(pkt->esp.seq));


-Dan