parse the 'hash' field after 'key' in IDX:branch block #97
Conversation
This PR is adding the ability to read the `hash` after the `key`.
Nothing is being done with that `hash` for now. The immediate goal
is to have a functional `extract_files` operation.
Quoting the kernel's `ubifs-media.h` header file:
In an authenticated UBIFS we have the hash of the referenced node
after @key. This can't be added to the struct type definition
because @key is a dynamically sized element already.
The size of key+hash is computed dynamically by dividing the remaining
length of the node by the number of children. A possible improvement
could be to grab the actual hash algorithm (stored somewhere?) and
verify that the hash length is the valid one.
A future improvement could be to have an `--check-authentication`
optional argument to the UBIFS operation what would verify that the
UBIFS image has not been tampered with.
|
@arnaudviala apologies but I'm only seeing your PR. I'll have a look soon. |
| brs = UBIFS_BRANCH_SZ | ||
| # Dynamic detection of `brs` (branch size), whether a hash is present after | ||
| # the key in authenticated UBIFS. Not checking the len of the hash. | ||
| brs = int((len(buf) - UBIFS_IDX_NODE_SZ) / self.child_cnt) |
There was a problem hiding this comment.
The int casting is hiding low-level details. Do you want to round up or round down ? It would be cleaner with math.floor or math.ceil, depending on implementation details. You can also use // instead of / if you want to round down.
There was a problem hiding this comment.
I think // is what I was trying to achieve. In theory, if the UBIFS image is correctly formatted, the division by self.child_cnt has no remainder. My int( ... ) was a clumsy attempt to get rid of the .0 in the result of the division. (Because brs needs to be an int, not a float.)
I'll use // instead. I'll also rewrite the comment, because I'm having a hard time to understand myself after a few months.
| for key in fields: | ||
| if key == 'key': | ||
| setattr(self, key, parse_key(fields[key])) | ||
| else: | ||
| setattr(self, key, fields[key]) | ||
|
|
||
| if len(buf) > (UBIFS_BRANCH_SZ): |
There was a problem hiding this comment.
Why is UBIFS_BRANCH_SZ between parentheses ?
There was a problem hiding this comment.
For no reason. I forgot to remove them. (Maybe in a previous iteration, this was (UBIFS_BRANCH_SZ + something) ?
I'll remove those unneeded parentheses.
This PR is adding the ability to read the
hashafter thekey. Nothing is being done with thathashfor now. The immediate goal is to have a functionalextract_filesoperation on ubifs volume with authentication.Quoting the kernel's
ubifs-media.hheader file:The size of key+hash is computed dynamically by dividing the remaining length of the node by the number of children. A possible improvement could be to grab the actual hash algorithm (stored somewhere?) and verify that the hash length is the valid one.
A future improvement could be to have an
--check-authenticationoptional argument to the UBIFS operation what would verify that the UBIFS image has not been tampered with.