On Tue, Aug 14, 2018 at 07:40:02PM +0200, Toke Høiland-Jørgensen wrote: > > IMHO the draft describes this order: > > > > - parse trailer > > - preparse for PC and index > > - verify authentication (HMAC and PC/index) > > - regular parse and processing > > > > You are doing > > > > - parse trailer > > - verify HMAC > > - regular parse > > - verify PC/index > > - regular processing > > > > I would prefer if we just keep whole packet verification as a separate > > phase like described in the draft and as a separate function called > > from babel_process_packet(), even if that means two iterations through > > TLVs. > > Why? The way it is described in the draft in influenced by the babeld > parser, which will do full processing (including updating internal > state) of TLVs as they are parsed. Since we already have a separate > parsing step, verifying the HMAC between that and the stateful checks > seems like the natural thing to do, rather than introduce a third round?
Well, there are several interconnnected reasons: It is much easier to read and understand such code, which is especially important for security-related code, there is much smaller attack vector - if there is a bug in packet parsing code, it cannot be exploited by unathenticated attacker [*] and it keeps consistent error messages (you do not get packet parsing errors for regular LSAs for packets that are already rejected due to invalid authentication regardless of whether it is HMAC-invalid or PC/index-invalid). -- Elen sila lumenn' omentielvo Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."