On Fri, Jan 28, 2022 at 03:54:14PM +0100, Claudio Jeker wrote: > On Fri, Jan 28, 2022 at 09:31:26AM +0100, Theo Buehler wrote: > > On Thu, Jan 27, 2022 at 09:38:54AM +0100, Claudio Jeker wrote: > > > On Thu, Jan 27, 2022 at 07:46:32AM +0100, Theo Buehler wrote: > > > > On Wed, Jan 26, 2022 at 04:42:04PM +0100, Claudio Jeker wrote: > > > > > So the RFC is not very clear but in general the idea is that if > > > > > multiple > > > > > MFTs are available the newest one (highest manifest number) should be > > > > > used. > > > > > > > > > > In our case there are two possible MFTs available the previously > > > > > valid on > > > > > and the now downloaded one. So adjust the parser code so that both > > > > > files > > > > > are opened and parsed and the x509 is verified. Checks like the > > > > > thisUpdate/nextUpdate validity and FileAndHash sequence are postponed. > > > > > Compare these two mfts and decide which one should be used. > > > > > Now check everything that was postponed. > > > > > > > > > > When checking the hash of files in the MFT check both locations and > > > > > remember which file was the actual match. It is important that later > > > > > on > > > > > the same file is opened. > > > > > > > > > > The error checking around MFTs had to be adjusted in some places > > > > > since it > > > > > turned out to be too noisy on stale caches. > > > > > > > > > > Please test and report unexpected behaviour. > > > > > > > > This seems to work fine here. I have read the diff and it looks good, > > > > but have not reviewed it thoroughly. Do you consider it ready for that? > > > > > > I have parts I'm not super happy with but have no better idea yet. > > > Mainly the changes to parse_entity() make that function even more complex > > > and error prone. proc_parser_mft_check() is the other function where I'm > > > not sure. So happy for any feedback to improve those bits. > > > > The only suggestion I have is the usual one: factor the complicated bits > > into a function. The diff below changes two things: > > > > 1. Add error checking to mft_compare() > > 2. Factor the RTYPE_MFT handling into parse_load_mft() > > > > I think I like the more symmetric ownership handling of the two files > > better this way. This could probably be improved quite a bit by tweaking > > mft_compare() and by choosing better variable names than foo1 and foo2. > > In a next pass we could polish the other RTYPE_* cases in entity_parse() > > to resemble each other more. > > > > I'm also happy to land your initial diff (ok tb for that) and improve > > things in tree. > > Here is my latest version. I changed mft_compare() to compare the > hexnumbers with a length check and strcmp(). Looking at BN_bn2hex() this > should always work since leading zeros are correctly suppressed.
Agreed. > I also refactored the code out into parse_load_mft() but did the split a > bit different. By doing the proc_parser_mft_post() check in parse_entity() > it simplifies the return code a bit. Yes, that's a lot nicer. > So this is the version I would like to commit and we can then iterate > further in tree. Let's do this. ok
