> On May 17, 2017, at 12:12 PM, Scott Fluhrer (sfluhrer) <sfluh...@cisco.com> 
> wrote:
> 
>  
> From: Yoav Nir [mailto:ynir.i...@gmail.com <mailto:ynir.i...@gmail.com>] 
> Sent: Wednesday, May 17, 2017 2:54 PM
> To: Scott Fluhrer (sfluhrer)
> Cc: IPsecme WG (ipsec@ietf.org <mailto:ipsec@ietf.org>)
> Subject: Re: [IPsec] Question about ipsecme-tcp-encaps
>  
>  
> On 17 May 2017, at 20:39, Scott Fluhrer (sfluhrer) <sfluh...@cisco.com 
> <mailto:sfluh...@cisco.com>> wrote:
>  
> I’ve been looking over the draft, and I think I see a potential DoS attack 
> that does not appear to be addressed.  I’m writing this to see if there is 
> something I missed (and if there isn’t, start discussion on how we might 
> patch things up).
>  
> This is the scenario I’m looking at: Alice and Bob have a TCP-based IKE/IPsec 
> connection established.
>  
> Then, Eve injects a TCP packet to Bob with Alice’s source IP (and with the 
> appropriate TCP sequence numbers), and whose body consists of a single FF 
> byte.  Eve does nothing else than that (other than possibly absorbing the 
> TCP-ACK that Bob would send out, if that’d confuse Alice’s TCP stack…)
>  
> Alice will then send a legitimate packet, consisting of (for example) [Length 
> = 0x0124] [ESP Header][Body].  However, Bob’s TCP stack thinks it has already 
> received the first byte, and so it’ll ignore it, and so will tell the 
> application (IPsec) that it has received [0xff34][ESP Header][Body].
>  
> My TCP may be rusty, but I think Alice’s legitimate packet has the sequence 
> number to indicate it is retransmitting the byte that Bob already has. I 
> don’t know if that means that the new data overwrites the old data, that the 
> old data remains, of that the stack checks that it matches.
>  
> I don’t think it’s defined within TCP (rather, it’s up to the individual 
> stack); on the other hand, in general, the TCP stack has already handed off 
> the byte to the application (the IPsec packet stream parser), and so it 
> *can’t* overwrite it.
>  
> Of course, we could say “Eve modifies a valid TCP-encapsulated IPsec packet 
> so that the first byte is 0xff”, and we have the same attack…
> 
> 
> The IPsec packet parsing code would interpret this as an extremely long 
> encrypted packet, and so will continue to absorb the next 0xfe00 bytes from 
> Alice.
>  
> It’ll then try to decrypt it; it’ll fail.  That, in itself, is not that big 
> of a deal; we assume that an attacker who can modify packets at will is able 
> to force a few packets to be dropped.
>  
> However, look what happens after that; the IPsec stream parsing code will 
> then take the next two bytes of the stream, and try to parse them as ‘packet 
> length’.  We stopped at a random location within the TCP stream, and so quite 
> likely, we’re in the middle of an encrypted packet, and so the length will be 
> a random value.  We’ll then try to parse the next bytes as a packet, and this 
> will keep on going (blocking all Alice -> Bob traffic) until the 
> end-of-packet the IPsec stream parser assumes just happens to fall on an 
> actual packet boundary – obviously, that might be a while.
>  
> I agree. Once synchronization is lost, it may as well never be regained.
> 
> 
> TLS uses a similar ‘record lengths appear in the TCP stream’ concept; however 
> in the case of TLS, on decryption failure, you MUST close the connection (and 
> so this repeated ‘get a random sequence of bytes and try to decrypt’ isn’t an 
> issue); I didn’t see a similar mandate in the IPsec draft.
>  
>  
> Was there something I missed?  The draft does have the text:
>  
>    If either TCP Originator or TCP Responder
>    receives a stream that cannot be parsed correctly (for example, if
>    the TCP Originator stream is missing the stream prefix, or message
>    frames are not parsable as IKE or ESP messages), it MUST close the
>    TCP connection.
>  
> However:
> a)      That’s in a paragraph that starts “If a TCP connection is being used 
> to resume a previous IKE session…”; does it apply only in that case?

No, the MUST close applies for all connections, regardless of resumption. We 
could add a paragraph break to make that explicit.

> b)      An ESP message is of the form [SPI][Sequence number][Random bytes]; 
> unless you happen to get a SPI < 256 or length < 8, it’s not clear how you 
> could get something that is not of that format (unless you mandate that the 
> ESP length must be a multiple of 4 bytes; in that case, you should state that 
> explicitly)

My assumption was that receiving an unknown SPI would automatically cause the 
parsing to fail as a valid ESP message. I can add that to the text. Adding 
bytes to the stream would shift the valid SPI. Beyond that, as you mention the 
packet would not be decryptable, and certainly the next bytes after the invalid 
frame's length would not parse as a valid SPI. The reading would stop by then 
at least. We can also recommended that readers enforce a sane limit on frame 
size.

So the attacker can cause one large frame to be read, but after that the 
connection will be torn down.

Thanks,
Tommy

>  
>  
> Thoughts?
>  
> I’d like to hear how TCP stacks really behave.
>  
> That said, most of us feed the IPsec stack with packets generated on networks 
> with an MTU of 1500. Even if we add the IPsec over head on top of that, it’s 
> still less than 2.5% of the space afforded by a 16-bit length field. At least 
> for ESP packets.
>  
> If the receiver were to break the connection whenever it received some number 
> (2?  3?) of packets that had a length field that exceeded 1544 (or whatever 
> the maximum packet is for the particular algorithm) followed by an ESP field 
> that was not zero,
>  
> “ESP field”; do you mean SPI?
>  
> this would fix the problem without letting Eve break the connection with just 
> one injected byte.
>  
> I f you don’t’ use IKE fragmentation, then I believe IKE packets could be 
> over the limit we picked (unless we picked a rather conservative one); would 
> that be an issue?  So, was that something you were trying to address with the 
> ESP field above?
>  
>  
> But there does seem to be something missing.
>  
> A more predictable was to detect loss-of-synchronization between the sender 
> and the receiver would be to include one or two fixed (or predictable) bytes 
> in the header (along with the Length field); this would mean that we would 
> have quite high probability of detecting sync loss, without having an 
> arbitrary length limit on the encrypted region.  If we pick a 2 byte value, 
> that means that things stay on 4 byte boundaries…
>  
> Yoav
>  
> _______________________________________________
> IPsec mailing list
> IPsec@ietf.org <mailto:IPsec@ietf.org>
> https://www.ietf.org/mailman/listinfo/ipsec 
> <https://www.ietf.org/mailman/listinfo/ipsec>

_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec

Reply via email to