Conversation
A Fortigate that authenticates with a FortiToken sends XAUTH_TYPE 2 rather than 0, so requiring the generic type aborts the handshake with xauth packet unsupported: (ISAKMP_N_ATTRIBUTES_NOT_SUPPORTED)(13) right after the password prompt, on a gateway that is otherwise happy to talk to us. Nothing in the exchange is driven by that value: which credentials we send is decided by the attributes the server asks for, which we validate individually anyway. So check that the type is well formed and note anything unusual at debug level 2 instead of refusing to continue. Name the offending attribute when we do reject one, too. Without it an unsupported attribute and an unsupported type produce the same bare (13) and neither says which attribute was at fault.
We advertise md5, des, 3des and null in both the IKE and the quick mode proposals, and then call error(1) the moment the peer selects one of them, telling the user to pass --enable-weak-authentication or --enable-weak-encryption. Offering an algorithm we will not accept can only lose us a handshake we had already won: a Fortigate that pairs md5 with dh2 in one of its phase 1 proposals answers with IKE SA selected psk+xauth-aes256-md5 Peer has selected md5 as authentication method. and the connection dies after phase 1 completed, even though the same gateway negotiates aes256-sha1 happily once md5 is off the table. Filter the proposals through the --enable-* options that already decide whether we would accept the result. The checks on the peer's selection stay as they are; they still catch a gateway that picks something we never offered.
A Cisco concentrator sends CISCO_SPLIT_INC entries as 14 bytes: network, mask, protocol, source port, destination port. strongSwan's charon sends only the network and the mask, 8 bytes per entry, and so do appliances built on it. We derive the entry count by dividing the payload by 14, so a payload that is not a whole number of Cisco entries is rejected with ISAKMP_N_PAYLOAD_MALFORMED. That discards the modecfg reply carrying the split networks, and with it the connection, against a gateway that is otherwise working. Fall back to the 8 byte layout when the payload does not divide into whole Cisco entries but does divide into 8, and treat the three missing fields as zero, which is what "any" is encoded as when they are present. A payload that fits neither layout is still rejected.
Both receive paths take the readiness reported by select() or poll() as a promise that the recv which follows will not block. It is not one: the kernel can discard the datagram they saw before we ask for it, a bad UDP checksum being the usual way, and then the recv blocks with nothing else on the way. On the data path that hangs the tunnel until a packet happens to arrive; during negotiation it hangs vpnc outright, because sendrecv() is single threaded and nothing else runs meanwhile. Set SO_RCVTIMEO on the sockets we create and treat a timed out receive as "nothing arrived". Both data path callers already handle a failed receive that way, so they only needed to stop logging it as an error. recv_ignore_dup() needed more than that: it called error(1) on any failed receive, so adding the timeout without this would turn a stall into an exit. It now returns -1 and lets sendrecv() retry, the same as for a duplicate.
We can only offer CBC ciphers paired with an HMAC. Gateways are steadily moving the other way: an IPsec policy that has been hardened to AEAD only leaves vpnc with nothing to propose and the quick mode exchange fails, even though every other part of the connection would work. Add the three AES-GCM transforms from RFC 4106 with a 16 byte ICV, which is what a gateway configured for aes128gcm/aes256gcm expects. They are ESP only, so they carry no IKE id and are skipped when building the phase 1 proposal, and they authenticate by themselves, so their transform offers no AUTH_ALG and pairs with no hash. Key material for them is four bytes longer than the cipher key: the tail is the salt that RFC 4106 prepends to the explicit IV to form the nonce, and it has to be copied again on a rekey along with the keys. The proposal list is built back to front, so the AEAD ciphers are walked last to have them offered first. This goes through libgcrypt's GCM mode, which vpnc already links against, so it works the same whether the build uses GnuTLS or OpenSSL for the certificate paths.
vpnc knows two integrity algorithms, md5 and sha1, and md5 is refused unless it is enabled explicitly. That leaves sha1 as the only thing we can propose, so a gateway whose policy has been hardened to sha256 has nothing to agree with us on, in either phase, and the connection fails on a client that is otherwise perfectly able to talk to it. Add sha256, sha384 and sha512 for both IKE and ESP. The IKE side needs nothing else: key material and hash payloads are already sized from gcry_md_get_algo_dlen(). ESP is not so simple. RFC 2403 and RFC 2404 truncate md5 and sha1 to 96 bits, and the ICV length is written into the packet paths as a literal 12. RFC 4868 truncates the SHA-2 family to half the digest instead, so sha256, sha384 and sha512 want 16, 24 and 32 bytes. The length is now computed per algorithm, carried on the SA next to md_len, and passed to hmac_compute() rather than assumed by it. Getting this wrong is not a visible error, just every packet failing its integrity check, so the receive path also grows the length check the fixed size never needed. The wire ids are the ones RFC 4868 assigns, 5 to 7, continuing the authentication algorithm attribute after KPDK.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.