[Swan-dev] ikev1-algo-ike-aes-02 Re: please fix ikev1-algo-05-3des-sha2

[Andrew Cagney]

I claim the code conspired against me.  This is my revenge:

    ikev1: fix optional key-length regression in an ESP proposal

    Merge ESP algorithm checks that were scattered across
    check_kernel_encrypt_alg, parse_ipsec_transform() and
    parse_ipsec_sa_body() into ikev1_verify_esp().  For key-length, just
    check it is valid, and that earlier code handled the missing /
    optional cases.

    In parse_ipsec_transform() remove all but the checks for a missing or
    optional key-length.  When optional, force .enckeylen to .keydeflen
    (it will remain 0 when 'null' encryption).  This way latter code can
    assume .enckeylen is correct and check it.

    In parse_ipsec_sa_body() use ikev1_verify_esp() to verify each
    proposal as it is parsed and not at the end after it has been sort of
    accepted.

    Delete check_kernel_encrypt_alg() as no longer used.
    Delete crypto_req_keysize(CRK_ESPorAH,...) as no longer used.

    Regression in 6e1368a4a51ab42ffa0e229e6c6b1b649776fd6e spotted
    by Hugh.

both this and the 3des test now pass.
On Wed, 18 Jul 2018 at 20:17, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
>
> Same problem with ikev1-algo-ike-aes-02.
>
> east.pluto.log:
>
> "westnet-eastnet-3des" #1: STATE_MAIN_R3: sent MR3, ISAKMP SA established {auth=RSA_SIG cipher=aes_256 integ=sha2_256 group=MODP2048}
> "westnet-eastnet-3des" #1: the peer proposed: 192.0.2.0/24:0/0 -> 192.0.1.0/24:0/0
> "westnet-eastnet-3des" #2: IPsec encryption transform rejected: 3DES_CBC key_len 0 is incorrect
> "westnet-eastnet-3des" #2: sending encrypted notification BAD_PROPOSAL_SYNTAX to 192.1.2.45:500
> "westnet-eastnet-3des" #2: deleting state (STATE_QUICK_R0) and NOT sending notification
>
> | From: D. Hugh Redelmeier <hugh at mimosa.com>
> | To: Libreswan Development List <swan-dev at lists.libreswan.org>
> | Date: Wed, 18 Jul 2018 18:36:12 -0400 (EDT)
> | Subject: [Swan-dev] please fix ikev1-algo-05-3des-sha2
> |
> | Pluto log on east says:
> |
> | "westnet-eastnet-ipv4-psk-ikev1" #1: STATE_MAIN_R3: sent MR3, ISAKMP SA established {auth=PRESHARED_KEY cipher=aes_256 integ=sha2_256 group=MODP2048}
> | "westnet-eastnet-ipv4-psk-ikev1" #1: the peer proposed: 192.0.2.0/24:0/0 -> 192.0.1.0/24:0/0
> | "westnet-eastnet-ipv4-psk-ikev1" #2: IPsec encryption transform rejected: 3DES_CBC key_len 0 is incorrect
> | "westnet-eastnet-ipv4-psk-ikev1" #2: sending encrypted notification BAD_PROPOSAL_SYNTAX to 192.1.2.45:500
> |
> | I think that the test is broken.  But it might be that Pluto is
> | broken.  I guess that this is something that Andrew is best equipped
> | to fix.
> |
> | Bonus points (not just for Andrew):
> |
> | (1) I assume that this notification was encrypted.  But nothing about a
> | notification shows up on west.console.diff.
> |
> | Should we not report authenticated notifications to whack?  It is
> | reported in Pluto's log on west:
> |
> | "westnet-eastnet-ipv4-psk-ikev1" #1: ignoring informational payload BAD_PROPOSAL_SYNTAX, msgid=00000000, length=12
> | | ISAKMP Notification Payload
> | |   00 00 00 0c  00 00 00 01  01 00 00 0f
> |
> | (3) the message should make clear whether the informational payload
> | was authenticated.
> |
> | (3) It would be nice of Pluto to report a decoded version of this
> | payload to the user.  That would help the user understand the problem.
> |
> | (4) It would probably be smart not to keep resending the same message
> | when it is so very likely that the outcome will be identical.
> | This isn't a bug so it isn't very important.
> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev