friissoren
commented
2 years ago Thanks for checking this. I agree that the whole topic of the length of authentication data is somewhat confusing.
Did you see the additional text that was added to Chapter 5.4.5.15 of the latest v1.1 draft? It was added fairly recently as an attempt to further clarify how exactly to interpret the somewhat ambiguous comment added to the Length field in Table 8. You are now quoting an earlier version of the draft v1.1 (I guess the one that was sent for ballot). Please get the latest from Gabriel's OneDrive. It will be sent for a second ballot at some point (hopefully soon).
I have tried to also give an explanation of this here.
The reason the code is checking a uint8 against a value larger than 255 is that the code also allows lowering ODID_AUTH_MAX_PAGES. Since the value of MAX_AUTH_LENGTH depends on ODID_AUTH_MAX_PAGES, there are situations where this check becomes relevant. I saw the warning in the IDE I am using when I wrote the code but since it didn't show up when I compiled, I didn't think of suppressing it. I agree it needs to be fixed. I will add some comments to the PR.
cedric-papon
When compiling with "-Wextra" flag and ODID_AUTH_MAX_PAGES = 16, the compiler warns that we are comparing an uint8_t against MAX_AUTH_LENGTH, in this case with the value 362.
The latest F3411 draft states:
My understanding would be that
up to 11 maximum authentication pages (including page zero), we should check that the Length field doesn't exceed _MAX_AUTHLENGTH
for 12 to 16 maximum pages we should only check that Length is different from 255 if LastPageIndex is greater or equal to 11