On Fri, Jun 16, 2023 at 7:55 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Fri, Jun 16, 2023 at 8:24 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> >
> > On Fri, Jun 16, 2023 at 08:07:43AM +0300, Amir Goldstein wrote:
> > > > I would really like to get the overlay.verity xattr support in
> > > > upstream pretty soon, because without it I can't release a stable
> > > > version of the composefs userspace code. I don't want to release
> > > > something that is using an xattr format that is not guaranteed to be
> > > > stable.
> > > >
> > >
> > > Alex,
> > >
> > > Pondering about this last sentence.
> > >
> > > The overlay.verity xattr format is already in its 3rd revision since
> > > the beginning of development.
> > >
> > > When it was bare digest, it might have made sense to have no
> > > header that describes the format.
> > >
> > > When the algo byte was added, that was already a very big hint
> > > that a proper header was in order.
> > >
> > > Now that you had to change the meaning of the byte, it is very hard
> > > to argue that the xattr format is guaranteed to be stable - in the sense
> > > that it can never change again.
> > >
> > > Please add a minimal header to the overlay.verity xattr format,
> > > similar to ovl_fb, that will allow composefs/overlayfs to be
> > > maintained as the separate projects that they are.
> > >
> > > Something like this?
> > >
> > > /* On-disk format for "verity" xattr */
> > > struct ovl_verity {
> > > u8 version; /* 0 */
> > > u8 len; /* size of this header + size of digest */
> > > u8 flags;
> > > u8 algo; /* digest algo */
> > > u8 digest[];
> > > };
> > >
> > > I realize that digest size may be inferred from xattr size,
> > > but it is better to state the stored digest size explicitly and verify
> > > that it matches expected xattr size.
> >
> > If it was up to me I think I'd keep it even more "minimal" just do:
> >
> > struct ovl_verity {
> > u8 version; /* 0 */
> > u8 algo; /* digest algo */
> > u8 digest[];
> > };
> >
> > It's up to you, though.
>
> Please keep len and flags.
> Nothing to be gained from removing them.
Something is gained by removing them: smaller images.
I have a Centos 9 developer rootfs here to test with. The basic
composefs image size for this with the current format (1 byte for algo)
is 11M. Adding one more byte (version) adds 88k (0.8%) to it and adding 3
bytes (version+flags+len) adds 240k (2.1%).
These are not huge costs, but they are not really giving any huge advantages
either. I mean, the digest length can be computed already, both from
the xattr size
and the algo type. And flag-like features could be encoded in the
version byte, or,
if really needed, a version 2 header could add a flags field.
I don't believe this format will actually need to change much.
However, I do agree
with the general requirement for *some* ability to move forward with
this format,
so I'm gonna go with a single version byte.
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Alexander Larsson Red Hat, Inc
alexl@xxxxxxxxxx alexander.larsson@xxxxxxxxx
