Subject: Re: [secdir] secdir review of draft-ietf-ospf-lls

From: Abhay Roy <[email-masked]>
Date: Fri, 16 Jan 2009 17:53:38 -0800

Hi Stephen,

Thanks again for your comments. Please see inline:

On 12/18/2008 4:10 AM, Stephen Farrell wrote:
> Hiya,
> 
> Abhay Roy wrote:
>> Hi Stephen,
>>
>> Thanks for the very detailed review!. Please see inline:
>>
>> On 11/26/2008 8:50 AM, Stephen Farrell wrote:
>>> Hi,
>>>
>>> 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.
>>>
>>> The draft [1] specifies a way to add new extension TLVs to/after OSPF
>>> packets and includes a way to authenticate those.
>>>
>>> I have a few things here that I think need attention, but they
>>> can probably mostly be easily handled with additional clarifying
>>> text. My comments are attached.
>>>
>>> Stephen.
>>>
>>> [1] http://tools.ietf.org/html/draft-ietf-ospf-lls
>>>
>>>
>>>
>>>
>>> #1: Section 2.2 describes the use of the checksum field, but never
>>> says what to
>>> do if the checksum is wrong. Is just the LLS block igored or the
>>> entire OSPF
>>> message?
>>> #2: Section 2.2 doesn't say whether the checksum bits (presumably
>>> zero'd?) are
>>> considered part of the LLS block when calculating the checksum.
>> The behavior is the same as we do for standard OSPF packet, only
>> difference being the scope is only the LLS block. I have added a
>> clarifying text here (for the two points above):
>>
>> "The Checksum field contains the standard IP checksum for
>> the entire contents of the LLS block. Before computing the
>> checksum, the checksum field is set to 0. If the checksum
>> is incorrect, LLS block MUST NOT be processed."
> 
> Sorry to be dense, but does that mean that the entire
> message is ignored or only the LLS block? I don't
> really care, but think it ought be crystal clear.

Only LLS block is discarded. I will revise this text a bit.

>>> #3: I don't understand the reason for omitting the checksum if the
>>> cryptographic authentication TLV is present. Since the spec doesn't
>>> say what to
>>> put in the checksum field in that case, the absence of the checksum
>>> can't be
>>> checked which'd lead the implementation to have to poke about looking
>>> for the
>>> crypto. TLV before it knows what to do. That seems error prone. At the
>>> least,
>>> more clarity as to the receiver actions is required.
>> This is to keep things compatible with current OSPF standard. Please see
>> the following quote from rfc2328 section D.4.3:
>>
>>            (2) The checksum field in the standard OSPF header is not
>>                calculated, but is instead set to 0.
>>
>> I will add the explicit setting to 0 in the text:
>>
>> "Note that if the OSPF packet is cryptographically
>> authenticated, the LLS data block MUST also be
>> cryptographically authenticated. In this case, the regular
>> LLS checksum is not calculated, but is instead set to 0."
> 
> Fine.
> 
>>> #4: If the OSPF packet is not cryptographically authenticated, is it
>>> still ok
>>> to include a cryptographic authentication TLV in the LLS? If so, what
>>> should I
>>> do if the LLS crypto. fails to validate? Simplest might be to insist that
>>> either both are protected or neither is, but I don't know if that
>>> matches the
>>> requirements.
>> It's not Ok as per this:
>>                                        "For the LLS block to
>> be considered authentic, the Sequence Number in the CA-TLV MUST match
>> the Sequence Number in the OSPFv2 packet header Authentication field.
>> In the event of Sequence Number mismatch or Authentication failure,
>> the whole LLS block MUST be ignored."
> 
> Again I'm a bit dense, so I'd find it clearer if that said: "...MUST
> match the Sequence Number in the OSPFv2 packet header Authentication
> field (which MUST be present)."

Sure, will add.

>>> #5: Section 2.5 has no references and doesn't describe which algorithm
>>> is to be
>>> used, now how keying is achieved. Questions arising include: a) rfc 2328
>>> defines three types of authentication in appendix D, including Null
>>> authentication - that one says to ignore the authentication field and
>>> use the
>>> checksum, which seems to conflict with what this draft says (if crypto
>>> TLV
>>> present, ignore checksum);
>> In case of Null authentication, checksum is the only protection in both
>> main OSPF payload and LLS block. In case of Crypto Authentication,
>> checksums are set to 0, and digest is the one used to verify both parts.
>> So I think they are consistent.
> 
> Are you saying that Null authentication is allowed, and that in that
> case there is no checksum, but that that's ok? If so, that is not at
> all clear and seems quite odd - why add the Crypto Authentication TLV
> to the LLS block at all then?

Null auth requires checksum per rfc2328.

Also, I will reword the text about CA-TLV to be present ONLY when the 
ospf message contains crypto digest. So Null auth or simple password 
(the case below) would never have a CA-TLV.

>> b) similarly, the "simple password" method from 2328
>>> also seems to require checking the checksum. (It might be as easy to
>>> just say
>>> those two MUST NOT be used); 
>> If "simple password" is used it's just a way to deter a random
>> miscofigured router from becoming part of OSPF routing domain. It's not
>> exactly providing any protection, hence checksums are still necessary to
>> protect both ospf payloads.
> 
> But again, if simple password is allowed and the checksum is zero'd
> (because the Crypto Authentication TLV is in the LLS block) then
> something is broken. Also - why would you want to put the same
> "simple password" in clear twice in the same message - once in the
> OCSPv2 packet and once in the LLS?
> 
> I think saying that the LLS Crypto Authentication TLV MUST use
> the MD5 based scheme makes most sense.
 >
>> c) I assume that the key lifecycle requirements
>>> are the same as in 2328, appendix D.3 (if LLS is allowed to be
>>> authenticated
>>> when the OSPFv2 packet is not, then this could be an issue). 
>> Right.
>>
>>> #6: Is it ok to use the same key & alg on the different inputs if the
>>> MD5 alg
>>> from 2328 is re-used here with the same key and but different inputs?
>>> In that
>>> case the OSPFv2 packet will contain MD5(OSPFv2-packet||key||pad||len)
>>> and the
>>> LLS will contain MD5(LLS-data-block||key) where both the OSPFv2-packet
>>> and the
>>> LLS-data-block contain the same sequence number. I don't have a specific
>>> problem that I can call out with that but do wonder if its ok.  Ought
>>> it be
>>> looked at by some cryptgrapher? (If a HMAC option is available then
>>> using that
>>> would probably be better, or if a diiferent key, e.g. one derived from
>>> the key
>>> protecting the OSPFv2-packet, were used for the LLS that might also be
>>> better.
>>> But those would change the bits on the wire so might not be an option
>>> anymore?)
>> OSPFv2 packet MD5 is done with  MD5(OSPFv2-packet) - note that it
>> doesn't include the Authentication header.
>> LLS block MD5 is done with MD5(LLS-data) - note that it doesn't include
>> the CA-TLV.
>>
>> I know of implementation deployed with this algorithm for many years.
> 
> Sure. I didn't imagine that this'd change, but its not the construct
> that a crypto person'd choose (two related values being protected with
> the same key based on a fairly broken hash function), so all I wanted
> to say was that I think it might be good to get a cryptographer to
> look it over if you (or someone) can.
> 
> Separately, it might also be good to develop a new authentication
> scheme not based on MD5 and (maybe) where a KDF is used to derive
> a new key for the LLS block from the one used to protect the OCSP
> packet. But of course, that'd be a bunch of work and I've no idea
> if it'd be worthwhile or not.

OSPF will also use SHA-1 and other SHA variants soon using the same CA 
type (please see draft-ietf-ospf-hmac-sha). So that should give us more 
stronger auth mechanisms.

>>> #7: Presumably it is not ok to include a cryptographic authentiation
>>> TLV in an
>>> OSPFv3 packet? Might be worth a MUST NOT statement.
>> Sure. Will add something to this effect.
>>
>>> #8: If the crypto TLV is not the last TLV, then are the subsequent TLVs
>>> included in the "LLS data block" input to the crypto processing? I
>>> think the
>>> definition of the "LLS data block" as input to crypto processing needs
>>> to be
>>> clearer - maybe take D.4.3 from 2328 as an example if the same MD5 alg
>>> is to be
>>> used.
>>> #9: If there are specific reasons why the crypto TLV might not be the
>>> last one,
>>> that'd be worth noting.
>>> #10: The cryptographic TLV doesn't (I think) protect the overall
>>> length of the
>>> LLS (if I'm right in #8 above that TLVs following the crypto TLV are not
>>> protected). In that case a single LLS mixes authenticated and
>>> unauthenticated
>>> TLVs, which could allow an attacker to replace those that follow the
>>> crypto
>>> TLV, or mess with the overall length or do other bad things. I think some
>>> statement is required as to what a receiver MUST do in such cases, but
>>> I don't
>>> know enough to suggest any text.  (Since the LLS can contain any
>>> extension, how
>>> do we know its ok to mix authenticated and unauthenticated TLVs like
>>> that?)
>> CA-TLV needs to be the last TLV (see Liem's email to Spencer).
> 
> I don't recall that email. If the draft said it MUST be the last
> TLV then that'd be fine, but the version I read said SHOULD. If the
> SHOULD remains then I believe the draft really has to say what to do
> if there are subsequent unauthenticated TLVs.

Yes, it says MUST..

Regards,
-Abhay

> Cheers,
> Stephen.
> 
>> Conceptually it has similar semantics as to how a digest gets computed
>> and attached at the end of a OSPF packet.
>>
>>> Nits/Typos:
>>>
>>> section 2.1 - "The L bit is only set in Hello and DD packets." Would
>>> that be
>>> better as: "The L bit MUST NOT be set except in Hello and DD packets that
>>> conain LLS." Might be more likely to be picked up by implementers?
>> Sure, will go with your suggestion.
>>
>>> section 2.3 - "...unique for each type of TLVs" s/TLVs/TLV/
>> changed
>>
>>> section 2.3 - Seems odd to have a mixture of length fields that're
>>> sometimes in
>>> 32-bit words and sometimes in bytes but I assume its too late to
>>> change that
>>> now.
>>>
>>> section 4 - "...since OSPF router not..." s/router/routers/
>> changed
>>
>>> section 5 - "...as OSPFv2 protocol..." s/OSPFv2/the OSPFv2/
>> changed
>>
>> Regards,
>> -Abhay
>>
>>
_______________________________________________
secdir mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/secdir