[strongSwan-dev] [RFC] Be lenient about downstream encryption

[Paul Stewart]

I discovered an interop issue with XAUTH authentication
with StrongSwan VPNs.  Does anyone have a deep enough
knowledge of this frame to understand what the remote
VPN is giving away?

strongSwan has an "encryption" flag for each payload type.
It uses this flag both to signal whether it should encrypt
these payloads and whether to accept such payloads in an
unencrypted form.  Some VPN endpoints, specifically SonicWall
devices, do not encrypt their ID_V1 and HASH_V1 during ID_PROT
during an XAUTH authentication flow.  This doesn't expose a
lot of information and does not induce the client to expose
anything either.  The change below whitelists these specific
payloads from being rejected if they are not encrypted, but
preserves the client behavior of encrypting these payloads
when they are sent.  It works around this issue.

Here's an example of a frame that the SonicWall sends that
would otherwise be rejected by strongSwan:

Frame 10: 106 bytes on wire (848 bits), 106 bytes captured (848 bits)
    Encapsulation type: Ethernet (1)
    Arrival Time: Mar 22, 2014 21:22:20.215818000 PDT
    [Time shift for this packet: 0.000000000 seconds]
    Epoch Time: 1395548540.215818000 seconds
    [Time delta from previous captured frame: 0.000617000 seconds]
    [Time delta from previous displayed frame: 0.000617000 seconds]
    [Time since reference or first frame: 4.048395000 seconds]
    Frame Number: 10
    Frame Length: 106 bytes (848 bits)
    Capture Length: 106 bytes (848 bits)
    [Frame is marked: False]
    [Frame is ignored: False]
    [Protocols in frame: eth:ip:udp:isakmp]
Ethernet II, Src: xxx, Dst: yyy
    Destination: yyy
        Address: yyy
        .... ..0. .... .... .... .... = LG bit: Globally unique address (factory default)
        .... ...0 .... .... .... .... = IG bit: Individual address (unicast)
    Source: xxx
        Address: xxx
        .... ..0. .... .... .... .... = LG bit: Globally unique address (factory default)
        .... ...0 .... .... .... .... = IG bit: Individual address (unicast)
    Type: IP (0x0800)
Internet Protocol Version 4, Src: 192.168.1.202 (192.168.1.202), Dst: 192.168.1.2 (192.168.1.2)
    Version: 4
    Header length: 20 bytes
    Differentiated Services Field: 0x00 (DSCP 0x00: Default; ECN: 0x00: Not-ECT (Not ECN-Capable Transport))
        0000 00.. = Differentiated Services Codepoint: Default (0x00)
        .... ..00 = Explicit Congestion Notification: Not-ECT (Not ECN-Capable Transport) (0x00)
    Total Length: 92
    Identification: 0x0233 (563)
    Flags: 0x00
        0... .... = Reserved bit: Not set
        .0.. .... = Don't fragment: Not set
        ..0. .... = More fragments: Not set
    Fragment offset: 0
    Time to live: 255
    Protocol: UDP (17)
    Header checksum: 0x3541 [correct]
        [Good: True]
        [Bad: False]
    Source: 192.168.1.202 (192.168.1.202)
    Destination: 192.168.1.2 (192.168.1.2)
User Datagram Protocol, Src Port: isakmp (500), Dst Port: isakmp (500)
    Source port: isakmp (500)
    Destination port: isakmp (500)
    Length: 72
    Checksum: 0x73cc [validation disabled]
        [Good Checksum: False]
        [Bad Checksum: False]
Internet Security Association and Key Management Protocol
    Initiator cookie: f40cb6a7d782baa6
    Responder cookie: c3abb61d0aafcf8f
    Next payload: Identification (5)
    Version: 1.0
    Exchange type: Identity Protection (Main Mode) (2)
    Flags: 0x00
        .... ...0 = Encryption: Not encrypted
        .... ..0. = Commit: No commit
        .... .0.. = Authentication: No authentication
    Message ID: 0x00000000
    Length: 28

We could make this whitelist conditional on the a vendor
ID sent by the remote (in this case I think vendor id
404bf439522ca3f6).
---
 src/libcharon/encoding/message.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/libcharon/encoding/message.c b/src/libcharon/encoding/message.c
index 11e735a..63dc664 100644
--- a/src/libcharon/encoding/message.c
+++ b/src/libcharon/encoding/message.c
@@ -1982,8 +1982,21 @@ static status_t decrypt_payloads(private_message_t *this, keymat_t *keymat)
 			{
 				DBG1(DBG_ENC, "payload type %N was not encrypted",
 					 payload_type_names, type);
-				status = FAILED;
-				break;
+				/**
+				 * Special case for some broken endpoints
+				 * that fail to encrypt here.  We are not
+				 * giving away much in these messages, however.
+				 */
+				if (this->exchange_type == ID_PROT &&
+				    !this->is_request &&
+				    (type == ID_V1 || type == HASH_V1)) {
+					DBG1(DBG_ENC,
+					     "white-listing this unencrypted "
+					     "payload");
+				} else {
+					status = FAILED;
+					break;
+				}
 			}
 		}
 		previous = payload;
-- 
1.9.1.423.g4596e3a