[Swan-dev] ikev1 retransmits: only save the received packet when responding

[Andrew Cagney]

Here's the thread:
https://lists.libreswan.org/pipermail/swan-dev/2018-May/002419.html

"Thinking about it some more, since there's nothing to send back, why
even save last received?  It turns out that
complete_v1_state_transition() unconditionally saves the last
received, even when there's no reply."


On Thu, 28 Jun 2018 at 19:37, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> Andrew:
>
> I don't understand this title.

Try: Don't save the received packet when not responding?

> In IKEv1 both Initiator and Responder may need to retransmit.

Mumble something about IKEv1 crossing the streams.

> Maybe you mean "Replying", not "responding"?  These are synonyms in
> english but a useful distinction can be made in IKE, I think.
>
> If we are not replying, what would "remember_received_packet" actually do
> (before your change)?  I would have guessed that it saved an empty chunk
> or the like.  But then this change would be unnecessary.

> How have we lived without this so long?

bandaids

- my recent change:

    ikev1: apply another bandaid to code trying to send empty packets

    Duplicate the bandaid in send_chunks() that rejects empty packets.

    send_or_resend_v1_ike_msg_from_state() when passed an empty
    st_tpacket, was able to stumble past a passert(st_tpacket.len!=0) (see
    1f61a49a6f2d83997fcad50da20ed7cd5924b9f0 which left .len non-zero).
    Only later, in send_chunks(), was a "bandaid" detecting the problem
    (st_tpacket.ptr==NULL) and reject the attempt to send (grep for
    "Cannot send packet - a.ptr is NULL" in code and old test results).

- and before that:

    pluto: Add two bandaids for unexpected scenario's

    These are safeguarding an unexpected case of retransmiting a packet
    while having no stored packet. Once the cause of this bug has been
    found and solved, these two bandaids can be removed.

    This also adds some logging changes to help determine some NAT
    behaviours.


> commit 49cfd21870994d1afc038ecd0830c9ad0a14e6d1
> Author: Andrew Cagney <cagney at gnu.org>
> Date:   Tue May 29 09:24:49 2018 -0400
>
>     ikev1 retransmits: only save the received packet when responding
>
>     Should eliminate problems such as the responder, when receiving a
>     response to its XAUTH request from the initiator (remember, an IKEv1
>     exchange can flip initiator and responder part way through), would see
>     the received packet matched .st_rpacket and assume it needed to
>     re-transmit something.
>
>     Really fix 8f440ae125a1d29eb4507bd94b123d22bbd3cb2a
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev