Use composefs crate for fsverity digest reading #24
Conversation
Because we require fsverity in the tests, make this easy to configure externally for different test harnesses to use. Signed-off-by: Colin Walters <[email protected]>
Prep for linking to the composefs crate and C shared library to ensure we share code and maintain an incentive to contribute to those. Signed-off-by: Colin Walters <[email protected]>
To increase the motivation to improve that crate and the C library; especially the C library, because we *must* have it anyways because we depend on the binaries and we are just not going to rewrite everything in Rust in the near future. (And if we *did* start a rewrite of the composefs core I think that rewrite should live in that repo, not this one) Signed-off-by: Colin Walters <[email protected]>
| // #define FS_IOC_MEASURE_VERITY _IORW('f', 134, struct fsverity_digest) | ||
| type FsIocMeasureVerity = ioctl::ReadWriteOpcode<b'f', 134, FsVerityDigest<()>>; | ||
|
|
||
| pub fn fs_ioc_measure_verity<F: AsFd, H: FsVerityHashValue>(fd: F) -> Result<H> { |
There was a problem hiding this comment.
That said this code is quite elegant and nice; the generics usage for sha256/sha512 is cool. But OTOH there are no plans to use anything but sha256 for composefs.
What we could do though is move some of this "reimplement composefs core in Rust" into the composefs git repo under the composefs crate under an optional feature flag or something - might be especially useful to "cross test" i.e. verify the C and Rust codepaths do the same thing.
| Ok(digest.digest) | ||
| } | ||
| pub fn fs_ioc_measure_verity<F: AsFd>(fd: F) -> Result<Sha256HashValue> { | ||
| let mut digest = composefs::fsverity::Digest::new(); |
There was a problem hiding this comment.
Also looking at this, perhaps we should drop the Digest wrapper and do what you have here and just have it write into an array. Definitely the compile-time known size is better than what we have in .get() at the moment.
OTOH, and this relates to a larger topic...I feel like in practice we end up converting to hex in many places anyways, so we may as well canonicalize to e.g. this Sha256Digest instead.
|
I'm afraid I don't like this change. I really like the way the ioctl interface in rustix looks... and in general, if we can avoid the trouble of taking the dependency I'd really prefer to do so. Indeed: we use I kinda get some of the higher-level ideas about unifying this repo with the composefs repo a bit more... |
Is there a better reason that the below ⬇️ (That I already addressed via using Fedora)?
Sure but in reality I think what we'll want is integration tests and it's those that can require fsverity. Requiring fsverity for unit tests isn't wrong, but it isn't right either. |
|
I do think we want to help get the composefs C core packaged for Ubuntu and Arch etc. in the near term which would help address the dependency issue. |
Me too, but to be clear this is just starting the ball rolling on deduping things. I don't think it makes sense for this crate to have its own dumpfile code either for example. |
|
We had a quick realtime debate on this and the conclusion was:
Instead we will:
Incidentally in this I think we should actually switch over this crate to being a workspace...if you look at the I think then what is in |
tests: Add support for CFS_TEST_TMPDIR
Because we require fsverity in the tests, make this easy
to configure externally for different test harnesses to use.
Signed-off-by: Colin Walters [email protected]
ci: Switch to using Fedora in a container
Prep for linking to the composefs crate and C shared library
to ensure we share code and maintain an incentive to contribute
to those.
Signed-off-by: Colin Walters [email protected]
Use composefs crate for fsverity digest reading
To increase the motivation to improve that crate and the C library;
especially the C library, because we must have it anyways because
we depend on the binaries and we are just not going to rewrite
everything in Rust in the near future.
(And if we did start a rewrite of the composefs core I think
that rewrite should live in that repo, not this one)
Signed-off-by: Colin Walters [email protected]