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

From: Stephen Farrell <[email-masked]>
Date: Wed, 26 Nov 2008 16:50:17 +0000

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.

#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.

#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.

#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); 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); 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). 

#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?)

#7: Presumably it is not ok to include a cryptographic authentiation TLV in an
OSPFv3 packet? Might be worth a MUST NOT statement.

#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?)

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?

section 2.3 - "...unique for each type of TLVs" s/TLVs/TLV/

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/

section 5 - "...as OSPFv2 protocol..." s/OSPFv2/the OSPFv2/

_______________________________________________
secdir mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/secdir