[quagga-dev 10801] Re: RFC-6506(Supporting Authentication Trailer for OSPFv3) implementation in quagga-0.99.21 version
jyotsna.priya1 at tcs.com
Fri Oct 11 13:22:07 BST 2013
Hi Denis/ List,
Thank you for the the last review.
A new revision of the patch is attached with this mail. The keychain mechanism as done in RIP has been implemented in OSPFv3. As far as sequence number check is concerned, we think that what has been implemented is according to the specs and thus it is not changed in new revision. Other smaller issues like using a common function and using the __func__ macro are considered and resolved.
We look forward to your response.
Thanks & Regards
Next Gen R&D,
Tata Consultancy Services
-----Denis Ovsienko wrote: -----
To: Jyotsna Priya1 <jyotsna.priya1 at tcs.com>, "quagga-dev at lists.quagga.net" <quagga-dev at lists.quagga.net>
From: Denis Ovsienko <infrastation at yandex.ru>
Date: 09/04/2013 10:14PM
Subject: [quagga-dev 10727] Re: RFC-6506(Supporting Authentication Trailer for OSPFv3) implementation in quagga-0.99.21 version
Hello. Thank you for the next revision, I'm available for the review again. 20.08.2013, 13:55, "Jyotsna Priya1" <jyotsna.priya1 at tcs.com>: > Hi Denis, > > In response to your mail following is the update: > >>“Now the crypto sequence number (CSN) check looks correct. However, please note that in the RFC the CSN check is done before the HMAC check, for a reason (CSN is cheap, HMAC is expensive CPU-wise). To fix this properly, please note that the remote CSN must not be stored before the HMAC check passes.” > > Yes you are right and we have fixed this issue. > I confirm it reads OK now. >>“CSN initialization and updating don't follow its intended use. At initialization the high-order part is set to 0 and the low-order part is set to UNIX timestamp. At update the high-order part is incremented on rollover and the low-order part is set to a number that most of the time is not less than the UNIX timestamp. Although this is not explicitly against the spec, the effective space of a CSN operated this way is not far from the space of a 32-bit CSN. Setting the high-order part to the UNIX timestamp at initialization/rollover and incrementing the low-order part on each packet sent can deliver better results.” > > This point is not so clear. Referring to RFC-6506, Section 4.1.1, the higher order sequence number should only be increased when lower-order sequence number wraps. Could you please elaborate how will the initialization of higher-order sequence number with timestamp deliver better results? > In the patch soon after the rollover the low-order CSN part becomes UNIX timestamp again and the CSN exhausts faster than it could given its 64-bit size. Again, technically this is not against the spec and can be left for future generations to improve. >>“This revision still seems to reinvent the functionality of lib/keychain.c (struct ospf6_crypt_key, functions ospf6_crypt_key_new(), ospf6_crypt_key_lookup() etc) for no obvious gain.” > > The current keychain mechanism used in the implementation of RFC-6505 is in reference with the key mechanism used for ospfv2. The suggested keychain mechanism does not associate any key-id with the cryptographic key entered from cli by user. It only deals with the secret-key (for example - ip rip authentication key-chain <secret key>). But according to RFC-6506, Section 4.1, the Security Association Identifier (SAID or key-id) should map to the cryptographic key being used. This point needs some more research. We will revert you back soon regarding this. We also look forward to your suggestions. > Please note that references to OSPFv2 should be made with great care when implementing an extension to OSPFv3. On one hand it makes sense to reuse existing code (after factoring it out, where needed), but on the other it is much more important for each protocol to comply with its own specs rather than to look consistent (for example, mind the CSN nuances fixed in an earlier round). This line is fine, but drawing it may be hard. The "ip rip authentication key-chain" command refers to a key chain, that is, a named sequence of keys managed by functions in keychain.c. Each key in the sequence has a "key-id", which is purposed as a direct equivalent of the SAID: ! key chain chain1 key 10 key-string red key 20 key-string green ! key chain chain2 key 30 key-string blue ! Even given some drawbacks of lib/keychain.c, it makes sense to reuse it. This way one has to make improvements and fix bugs in one place rather than in each protocol at a time. As a side note, please see that the patch extends both ospf6_hello_print() and ospf6_dbdesc_print() with exactly the same code fragment, which would better be implemented as a separate function. Also there are several debug calls in the patch referring to the name of the current function (and sometimes that name is different from the real function name). A common way of keeping this consistent is to use "__func__" macro with "%s" format string, for example: zlog_debug ("%s: interface description alignment error", __func__); Of course, the terminology in no way impacts functional compliance, but it may help future developers take care of this code. The patch refers to just "SHA" in a few places (e.g. "ipv6 ospf6 sha-key <1-255> sha KEY"). However, "SHA" stands for a series of hash algorithms falling into 3 families (the current revision of the patch implements SHA-1 and SHA-2). The code would look cleaner if comments and messages referred to either particular algorithm (e.g. SHA-256) or "cryptographic authentication", whichever applies. Generally speaking, authentication is not limited to cryptographic, cryptographic authentication is not limited to hash algorithms and hash algorithms are not limited to SHA. The difference is not well seen within RFC6506 because OSPFv3 authentication is a relatively late design that skipped previous developments in this field. But you could find it useful to study the terminology in the RFC4822-enabled ripd, because in RIP cryptographic is just one possible type of authentication and SHA-1 is just one of the possible algorithms. Thank you. -- Denis Ovsienko _______________________________________________ Quagga-dev mailing list Quagga-dev at lists.quagga.net http://lists.quagga.net/mailman/listinfo/quagga-dev
Notice: The information contained in this e-mail
message and/or attachments to it may contain
confidential or privileged information. If you are
not the intended recipient, any dissemination, use,
review, distribution, printing or copying of the
information contained in this e-mail message
and/or attachments to it are strictly prohibited. If
you have received this communication in error,
please notify us by reply e-mail or telephone and
immediately and permanently delete the message
and any attachments. Thank you
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 70301 bytes
Desc: not available
More information about the Quagga-dev