Re: [secdir] Secdir review of draft-ietf-ipsecme-failure-detection-05

[Magnus Nyström <[e-mail]>]

Yoav,
Thanks for your quick response. Inline...

On Wed, Mar 9, 2011 at 11:50 PM, Yoav Nir <[e-mail]> wrote:
> Hi Magnus, thanks for the review.  My answers are inline.
>
>
> On Mar 7, 2011, at 9:14 AM, Magnus Nyström wrote:
>
>> I have reviewed this document as part of the security directorate's
>> ongoing effort to review all IETF documents being processed by the
>> IESG.  These comments were written primarily for the benefit of the
>> security area directors. Document editors and WG chairs should treat
>> these comments just like any other last call comments.
>>
>> This document defines a new extension (the "QCD token") to the IKEv2
>> protocol that allows for faster detection of SA de-synchronization.
>>
>> - General:
>>  o "Quick Crash Detection": Is "crash" really the right term here? As
>> the document indicates, the SA de-synchronization may have had other
>> reasons than a crash...? The term "failure detection" seems more
>> accurate.
>
> There are indeed several reasons why an IKE implementation might lose state. There's bugs in implementations or underlying operating systems, which may cause state loss, and are colloquially referred to as "crashes", there are power interruptions, and every product that I know of also has a user interface command to clear state.  At least the last of these is not even a failure at all.  So we might have termed this "Quick State Desynchronization Detection" (QSDD?), but crashes were the primary motivation for writing this, so we have used the name QCD from the start.  I think it stay, as long as the introduction says that it's OK.

OK

>> - Section 1, Introduction:
>>  o "However, in many cases the rebooted peer is a VPN gateway that
>> protects only servers, or else the non-rebooted peer has a dynamic IP
>> address" - it is not clear from this how or why the dynamic IP address
>> of the non-rebooted peer impacts the tunnel re-establishment?
>
> I agree it is a little confusing and should be split into two sentences. How about:
>
> However, in many cases the rebooted peer is a VPN gateway that protects only server, so all traffic is inbound. In other cases, the non-rebooted peer has a dynamic IP address, so the rebooted peer cannot initiate IKE because its current IP address is unknown.

Yes, much clearer IMO.

>>  o Editorial: "is a quick" -> "in a quick"?
>
> Yes. Thanks.
>
>>
>> - Section 2, RFC 5996 Crash Recovery :
>>  o "There are cases where the peer loses state and is able to recover
>> immediately; in those cases it might take several minutes to recover."
>> - so a peer is able to recover immediately, yet it might take several
>> minutes to recover?? Unclear what is meant here.
>
> How about "in those cases it might still take several minutes to recover the IPsec SAs."

Thanks; that's clearer.

>> - Section 5, Token Generation and Verification:
>>  o (Not sure why these methods are called stateless as the QCD_SECRET
>> must be maintained?)
>
> That's because they don't have a per-tunnel persistent state. An obvious (though never described) alternative is to generate a random token for each IKE SA, and store that in non-volatile storage. This was actually the design in one of the early drafts.
>
>>  o By adding a nonce to the token generation the attack outlined in
>> Section 9.3 would be impossible, as the attacker would also need to
>> guess the nonce (adding a nonce to the TOKEN_SECRET_DATA generation
>> would also have the effect that even for the same SPIs, the
>> TOKEN_SECRET_DATA would be different). More generally, a standard key
>> derivation scheme such as the Concatenation KDF in NIST SP 800-56 may
>> be considered.
>
> Yes, but then we would have to keep the nonce in state for each IKE SA, and this would vastly increase the non-volatile storage requirements. The goal is for the rebooted gateway to generate the correct token based only on an encrypted (with keys it doesn't have) IKE packet.

OK.But I still think SP 800-56A's Concat KDF could or should be
considered. It would map SPI-I and SPI-R as PartyUInfo and PartyVInfo
and for "free" you would be NIST-compatible. No nonce is required.

>> - Section 9.2, Security Considerations:
>>  o Last paragraph: "This method should not be used..." - unclear what
>> method is being referred to here?
>
> Seems clear to me. The first sentence in that paragraph talks about the method in section 5.2. The following sentences begin with "This method...", referring to the same method.

I see the text has changed slightly; A minor clarification could be to
replace the last "that method" with "The method in Section 5.2".

>> - Appendix A.2:
>>  o Would have been useful with a sentence or two that indicated why
>> the working group preferred the QCD proposal over the SIR one.
>
> OLD:
>   The working group preferred the QCD proposal to this one.
>
> NEW:
>   The working group preferred the QCD proposal to this one, because
>   of the lack of cryptographic protection for the queries and
>   responses.

Thanks!

-- Magnus