| Message ID | 20200211000037.189180-1-Jes.Sorensen@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Split fsverity-utils into a shared library | expand |
Hi Jes, On Mon, Feb 10, 2020 at 07:00:30PM -0500, Jes Sorensen wrote: > From: Jes Sorensen <jsorensen@fb.com> > > Hi, > > I am looking at what it will take to add support for fsverity > signatures to rpm, similar to how rpm supports IMA signatures. > > In order to do so, it makes sense to split the fsverity util into a > shared library and the command line tool, so the core functions can be > used from other applciations. Alternatively I will have to copy over a > good chunk of the code into rpm, which makes it nasty to support long > term. > > This is a first stab at doing that, and I'd like to get some feedback > on the approach. > > I basically split it into four functions: > > fsverity_cmd_gen_digest(): Build the digest, but do not sign it > fsverity_cmd_sign(): Sign the digest structure > fsverity_cmd_measure(): Measure a file, basically 'fsverity measure' > fsverity_cmd_enable(): Enable verity on a file, basically 'fsverity enable' > > If we can agree on the approach, then I am happy to deal with the full > libtoolification etc. > Before we do all this work, can you take a step back and explain the use case so that we can be sure it's really worthwhile? fsverity_cmd_enable() and fsverity_cmd_measure() would just be trivial wrappers around the FS_IOC_ENABLE_VERITY and FS_IOC_MEASURE_VERITY ioctls, so they don't need a library. [Aside: I'd suggest calling these fsverity_enable() and fsverity_measure(), and leaving "cmd" for the command-line wrappers.] That leaves signing as the only real point of the library. But do you actually need to be able to *sign* the files via the rpm binary, or do you just need to be able to install already-created signatures? I.e., can the signatures instead just be created with 'fsverity sign' when building the RPMs? Separately, before you start building something around fs-verity's builtin signature verification support, have you also considered adding support for fs-verity to IMA? I.e., using the fs-verity hashing mechanism with the IMA signature mechanism. The IMA maintainer has been expressed interested in that. If rpm already supports IMA signatures, maybe that way would be a better fit? - Eric
On 2/11/20 2:22 PM, Eric Biggers wrote: > Hi Jes, > > On Mon, Feb 10, 2020 at 07:00:30PM -0500, Jes Sorensen wrote: >> From: Jes Sorensen <jsorensen@fb.com> >> If we can agree on the approach, then I am happy to deal with the full >> libtoolification etc. > > Before we do all this work, can you take a step back and explain the use case so > that we can be sure it's really worthwhile? > > fsverity_cmd_enable() and fsverity_cmd_measure() would just be trivial wrappers > around the FS_IOC_ENABLE_VERITY and FS_IOC_MEASURE_VERITY ioctls, so they don't > need a library. [Aside: I'd suggest calling these fsverity_enable() and > fsverity_measure(), and leaving "cmd" for the command-line wrappers.] > > That leaves signing as the only real point of the library. But do you actually > need to be able to *sign* the files via the rpm binary, or do you just need to > be able to install already-created signatures? I.e., can the signatures instead > just be created with 'fsverity sign' when building the RPMs? So I basically want to be able to carry verity signatures in RPM as RPM internal data, similar to how it supports IMA signatures. I want to be able to install those without relying on post-install scripts and signature files being distributed as actual files that gets installed, just to have to remove them. This is how IMA support is integrated into RPM as well. Right now the RPM approach for signatures involves two steps, a build digest phase, and a sign the digest phase. The reason I included enable and measure was for completeness. I don't care wildly about those. > Separately, before you start building something around fs-verity's builtin > signature verification support, have you also considered adding support for > fs-verity to IMA? I.e., using the fs-verity hashing mechanism with the IMA > signature mechanism. The IMA maintainer has been expressed interested in that. > If rpm already supports IMA signatures, maybe that way would be a better fit? I looked at IMA and it is overly complex. It is not obvious to me how you would get around that without the full complexity of IMA? The beauty of fsverity's approach is the simplicity of relying on just the kernel keyring for validation of the signature. If you have explicit suggestions, I am certainly happy to look at it. Thanks, Jes
On Tue, Feb 11, 2020 at 05:09:22PM -0500, Jes Sorensen wrote: > On 2/11/20 2:22 PM, Eric Biggers wrote: > > Hi Jes, > > > > On Mon, Feb 10, 2020 at 07:00:30PM -0500, Jes Sorensen wrote: > >> From: Jes Sorensen <jsorensen@fb.com> > >> If we can agree on the approach, then I am happy to deal with the full > >> libtoolification etc. > > > > Before we do all this work, can you take a step back and explain the use case so > > that we can be sure it's really worthwhile? > > > > fsverity_cmd_enable() and fsverity_cmd_measure() would just be trivial wrappers > > around the FS_IOC_ENABLE_VERITY and FS_IOC_MEASURE_VERITY ioctls, so they don't > > need a library. [Aside: I'd suggest calling these fsverity_enable() and > > fsverity_measure(), and leaving "cmd" for the command-line wrappers.] > > > > That leaves signing as the only real point of the library. But do you actually > > need to be able to *sign* the files via the rpm binary, or do you just need to > > be able to install already-created signatures? I.e., can the signatures instead > > just be created with 'fsverity sign' when building the RPMs? > > So I basically want to be able to carry verity signatures in RPM as RPM > internal data, similar to how it supports IMA signatures. I want to be > able to install those without relying on post-install scripts and > signature files being distributed as actual files that gets installed, > just to have to remove them. This is how IMA support is integrated into > RPM as well. > > Right now the RPM approach for signatures involves two steps, a build > digest phase, and a sign the digest phase. > > The reason I included enable and measure was for completeness. I don't > care wildly about those. So the signing happens when the RPM is built, not when it's installed? Are you sure you actually need a library and not just 'fsverity sign' called from a build script? > > > Separately, before you start building something around fs-verity's builtin > > signature verification support, have you also considered adding support for > > fs-verity to IMA? I.e., using the fs-verity hashing mechanism with the IMA > > signature mechanism. The IMA maintainer has been expressed interested in that. > > If rpm already supports IMA signatures, maybe that way would be a better fit? > > I looked at IMA and it is overly complex. It is not obvious to me how > you would get around that without the full complexity of IMA? The beauty > of fsverity's approach is the simplicity of relying on just the kernel > keyring for validation of the signature. If you have explicit > suggestions, I am certainly happy to look at it. fs-verity's builtin signature verification feature is simple, but does it actually do what you need? Note that unlike IMA, it doesn't provide an in-kernel policy about which files have to have signatures and which don't. I.e., to get any authenticity guarantee, before using any files that are supposed to be protected by fs-verity, userspace has to manually check whether the fs-verity bit is actually set. Is that part of your design? - Eric
On 2/11/20 6:14 PM, Eric Biggers wrote: > On Tue, Feb 11, 2020 at 05:09:22PM -0500, Jes Sorensen wrote: >> On 2/11/20 2:22 PM, Eric Biggers wrote: >>> Hi Jes, >> So I basically want to be able to carry verity signatures in RPM as RPM >> internal data, similar to how it supports IMA signatures. I want to be >> able to install those without relying on post-install scripts and >> signature files being distributed as actual files that gets installed, >> just to have to remove them. This is how IMA support is integrated into >> RPM as well. >> >> Right now the RPM approach for signatures involves two steps, a build >> digest phase, and a sign the digest phase. >> >> The reason I included enable and measure was for completeness. I don't >> care wildly about those. > > So the signing happens when the RPM is built, not when it's installed? Are you > sure you actually need a library and not just 'fsverity sign' called from a > build script? So the way RPM is handling these is to calculate the digest in one place, and sign it in another. Basically the signing is a second step, post build, using the rpmsign command. Shelling out is not a good fit for this model. >>> Separately, before you start building something around fs-verity's builtin >>> signature verification support, have you also considered adding support for >>> fs-verity to IMA? I.e., using the fs-verity hashing mechanism with the IMA >>> signature mechanism. The IMA maintainer has been expressed interested in that. >>> If rpm already supports IMA signatures, maybe that way would be a better fit? >> >> I looked at IMA and it is overly complex. It is not obvious to me how >> you would get around that without the full complexity of IMA? The beauty >> of fsverity's approach is the simplicity of relying on just the kernel >> keyring for validation of the signature. If you have explicit >> suggestions, I am certainly happy to look at it. > > fs-verity's builtin signature verification feature is simple, but does it > actually do what you need? Note that unlike IMA, it doesn't provide an > in-kernel policy about which files have to have signatures and which don't. > I.e., to get any authenticity guarantee, before using any files that are > supposed to be protected by fs-verity, userspace has to manually check whether > the fs-verity bit is actually set. Is that part of your design? Totally aware of this, and it fits the model I am looking at. Jes
Hi Jes, On Tue, Feb 11, 2020 at 06:35:45PM -0500, Jes Sorensen wrote: > On 2/11/20 6:14 PM, Eric Biggers wrote: > > On Tue, Feb 11, 2020 at 05:09:22PM -0500, Jes Sorensen wrote: > >> On 2/11/20 2:22 PM, Eric Biggers wrote: > >>> Hi Jes, > >> So I basically want to be able to carry verity signatures in RPM as RPM > >> internal data, similar to how it supports IMA signatures. I want to be > >> able to install those without relying on post-install scripts and > >> signature files being distributed as actual files that gets installed, > >> just to have to remove them. This is how IMA support is integrated into > >> RPM as well. > >> > >> Right now the RPM approach for signatures involves two steps, a build > >> digest phase, and a sign the digest phase. > >> > >> The reason I included enable and measure was for completeness. I don't > >> care wildly about those. > > > > So the signing happens when the RPM is built, not when it's installed? Are you > > sure you actually need a library and not just 'fsverity sign' called from a > > build script? > > So the way RPM is handling these is to calculate the digest in one > place, and sign it in another. Basically the signing is a second step, > post build, using the rpmsign command. Shelling out is not a good fit > for this model. > > >>> Separately, before you start building something around fs-verity's builtin > >>> signature verification support, have you also considered adding support for > >>> fs-verity to IMA? I.e., using the fs-verity hashing mechanism with the IMA > >>> signature mechanism. The IMA maintainer has been expressed interested in that. > >>> If rpm already supports IMA signatures, maybe that way would be a better fit? > >> > >> I looked at IMA and it is overly complex. It is not obvious to me how > >> you would get around that without the full complexity of IMA? The beauty > >> of fsverity's approach is the simplicity of relying on just the kernel > >> keyring for validation of the signature. If you have explicit > >> suggestions, I am certainly happy to look at it. > > > > fs-verity's builtin signature verification feature is simple, but does it > > actually do what you need? Note that unlike IMA, it doesn't provide an > > in-kernel policy about which files have to have signatures and which don't. > > I.e., to get any authenticity guarantee, before using any files that are > > supposed to be protected by fs-verity, userspace has to manually check whether > > the fs-verity bit is actually set. Is that part of your design? > > Totally aware of this, and it fits the model I am looking at. > Well, this might be a legitimate use case then. We need to define the library interface as simply as possible, though, so that we can maintain this code in the future without breaking users. I suggest starting with something along the lines of: #ifndef _LIBFSVERITY_H #define _LIBFSVERITY_H #include <stddef.h> #include <stdint.h> #define FS_VERITY_HASH_ALG_SHA256 1 #define FS_VERITY_HASH_ALG_SHA512 2 struct libfsverity_merkle_tree_params { uint32_t version; uint32_t hash_algorithm; uint32_t block_size; uint32_t salt_size; const uint8_t *salt; size_t reserved[11]; }; struct libfsverity_digest { uint16_t digest_algorithm; uint16_t digest_size; uint8_t digest[]; }; struct libfsverity_signature_params { const char *keyfile; const char *certfile; size_t reserved[11]; }; int libfsverity_compute_digest(int fd, const struct libfsverity_merkle_tree_params *params, struct libfsverity_digest **digest_ret); int libfsverity_sign_digest(const struct libfsverity_digest *digest, const struct libfsverity_signature_params *sig_params, void **sig_ret, size_t *sig_size_ret); #endif /* _LIBFSVERITY_H */ I.e.: - The stuff in util.h and hash_algs.h isn't exposed to library users. - Then names of all library functions and structs are appropriately prefixed and avoid collisions with the kernel header. - Only signing functionality is included. - There are reserved fields, so we can add more parameters later. Before committing to any stable API, it would also be helpful to see the RPM patches to see what it actually does. We'd also need to follow shared library best practices like compiling with -fvisibility=hidden and marking the API functions explicitly with __attribute__((visibility("default"))), and setting the 'soname' like -Wl,-soname=libfsverity.so.0. Also, is the GPLv2+ license okay for the use case? - Eric
Hi Eric, On 2/14/20 3:35 PM, Eric Biggers wrote: > Well, this might be a legitimate use case then. We need to define the library > interface as simply as possible, though, so that we can maintain this code in > the future without breaking users. I suggest starting with something along the > lines of: > > #ifndef _LIBFSVERITY_H > #define _LIBFSVERITY_H > > #include <stddef.h> > #include <stdint.h> > > #define FS_VERITY_HASH_ALG_SHA256 1 > #define FS_VERITY_HASH_ALG_SHA512 2 > > struct libfsverity_merkle_tree_params { > uint32_t version; > uint32_t hash_algorithm; > uint32_t block_size; > uint32_t salt_size; > const uint8_t *salt; > size_t reserved[11]; > }; > > struct libfsverity_digest { > uint16_t digest_algorithm; > uint16_t digest_size; > uint8_t digest[]; > }; > > struct libfsverity_signature_params { > const char *keyfile; > const char *certfile; > size_t reserved[11]; > }; This looks reasonable to me - I would do the reserved fields as void * or uint32_t, but that is a detail. > int libfsverity_compute_digest(int fd, > const struct libfsverity_merkle_tree_params *params, > struct libfsverity_digest **digest_ret); > > int libfsverity_sign_digest(const struct libfsverity_digest *digest, > const struct libfsverity_signature_params *sig_params, > void **sig_ret, size_t *sig_size_ret); > > #endif /* _LIBFSVERITY_H */ Looks good too, I deliberately named the functions as fsverity, but happy to prepend them with 'lib'. Didn't want to have a clash with 'sign_hash' as a function is actually named in a related library. > I.e.: > > - The stuff in util.h and hash_algs.h isn't exposed to library users. > - Then names of all library functions and structs are appropriately prefixed > and avoid collisions with the kernel header. > - Only signing functionality is included. > - There are reserved fields, so we can add more parameters later. I was debating whether to expect the library to do the open or have the caller be responsible for that. Given we have to play the song and dance with the signing key and certificate filenames, it's a little quirky, but we're passing those to libopenssl so no way to really get around it. > Before committing to any stable API, it would also be helpful to see the RPM > patches to see what it actually does. Absolutely, I wanted to have us agree on the strategy first before taking it to the next step. I'll take a stab at this. > We'd also need to follow shared library best practices like compiling with > -fvisibility=hidden and marking the API functions explicitly with > __attribute__((visibility("default"))), and setting the 'soname' like > -Wl,-soname=libfsverity.so.0. > > Also, is the GPLv2+ license okay for the use case? Personally I only care about linking it into rpm, which is GPL v2, so from my perspective, that is sufficient. I am also fine making it LGPL, but given it's your code I am stealing, I cannot make that call. Cheers, Jes
On Wed, Feb 19, 2020 at 06:49:07PM -0500, Jes Sorensen wrote: > > We'd also need to follow shared library best practices like compiling with > > -fvisibility=hidden and marking the API functions explicitly with > > __attribute__((visibility("default"))), and setting the 'soname' like > > -Wl,-soname=libfsverity.so.0. > > > > Also, is the GPLv2+ license okay for the use case? > > Personally I only care about linking it into rpm, which is GPL v2, so > from my perspective, that is sufficient. I am also fine making it LGPL, > but given it's your code I am stealing, I cannot make that call. > Hi Jes, I'd like to revisit this, as I'm concerned about future use cases where software under other licenses (e.g. LGPL, MIT, or Apache 2.0) might want to use libfsverity -- especially if libfsverity grows more functionality. Also, fsverity-utils links to OpenSSL, which some people (e.g. Debian) consider to be incompatible with GPLv2. We think the MIT license (https://opensource.org/licenses/MIT) would offer the most flexibility. Are you okay with changing the license of fsverity-utils to MIT? If so, I'll send a patch and you can give an Acked-by on it. Thanks! - Eric
On 7/30/20 1:52 PM, Eric Biggers wrote: > On Wed, Feb 19, 2020 at 06:49:07PM -0500, Jes Sorensen wrote: >>> We'd also need to follow shared library best practices like compiling with >>> -fvisibility=hidden and marking the API functions explicitly with >>> __attribute__((visibility("default"))), and setting the 'soname' like >>> -Wl,-soname=libfsverity.so.0. >>> >>> Also, is the GPLv2+ license okay for the use case? >> >> Personally I only care about linking it into rpm, which is GPL v2, so >> from my perspective, that is sufficient. I am also fine making it LGPL, >> but given it's your code I am stealing, I cannot make that call. >> > > Hi Jes, I'd like to revisit this, as I'm concerned about future use cases where > software under other licenses (e.g. LGPL, MIT, or Apache 2.0) might want to use > libfsverity -- especially if libfsverity grows more functionality. > > Also, fsverity-utils links to OpenSSL, which some people (e.g. Debian) consider > to be incompatible with GPLv2. > > We think the MIT license (https://opensource.org/licenses/MIT) would offer the > most flexibility. Are you okay with changing the license of fsverity-utils to > MIT? If so, I'll send a patch and you can give an Acked-by on it. > > Thanks! > > - Eric Hi Eric, I went back through my patches to make sure I didn't reuse code from other GPL projects. I don't see anything that looks like it was reused except from fsverity-utils itself, so it should be fine. I think it's fair to relax the license so other projects can link to it. I would prefer we use the LGPL rather than the MIT license though? CC'ing Chris Mason as well, since he has the auth to ack it on behalf of the company. Cheers, Jes
On 31 Jul 2020, at 13:40, Jes Sorensen wrote: > On 7/30/20 1:52 PM, Eric Biggers wrote: >> On Wed, Feb 19, 2020 at 06:49:07PM -0500, Jes Sorensen wrote: >>>> We'd also need to follow shared library best practices like >>>> compiling with >>>> -fvisibility=hidden and marking the API functions explicitly with >>>> __attribute__((visibility("default"))), and setting the 'soname' >>>> like >>>> -Wl,-soname=libfsverity.so.0. >>>> >>>> Also, is the GPLv2+ license okay for the use case? >>> >>> Personally I only care about linking it into rpm, which is GPL v2, >>> so >>> from my perspective, that is sufficient. I am also fine making it >>> LGPL, >>> but given it's your code I am stealing, I cannot make that call. >>> >> >> Hi Jes, I'd like to revisit this, as I'm concerned about future use >> cases where >> software under other licenses (e.g. LGPL, MIT, or Apache 2.0) might >> want to use >> libfsverity -- especially if libfsverity grows more functionality. >> >> Also, fsverity-utils links to OpenSSL, which some people (e.g. >> Debian) consider >> to be incompatible with GPLv2. >> >> We think the MIT license would offer the >> most flexibility. Are you okay with changing the license of >> fsverity-utils to >> MIT? If so, I'll send a patch and you can give an Acked-by on it. >> >> Thanks! >> >> - Eric > > Hi Eric, > > I went back through my patches to make sure I didn't reuse code from > other GPL projects. I don't see anything that looks like it was reused > except from fsverity-utils itself, so it should be fine. > > I think it's fair to relax the license so other projects can link to > it. > I would prefer we use the LGPL rather than the MIT license though? > > CC'ing Chris Mason as well, since he has the auth to ack it on behalf > of > the company. MIT, BSD, LGPL are Signed-off-by: Chris Mason <clm@fb.com> We’re flexible, the goal is just to fit into the rest of fsverity overall. -chris
On Fri, Jul 31, 2020 at 01:47:36PM -0400, Chris Mason wrote: > On 31 Jul 2020, at 13:40, Jes Sorensen wrote: > > > On 7/30/20 1:52 PM, Eric Biggers wrote: > > > On Wed, Feb 19, 2020 at 06:49:07PM -0500, Jes Sorensen wrote: > > > > > We'd also need to follow shared library best practices like > > > > > compiling with > > > > > -fvisibility=hidden and marking the API functions explicitly with > > > > > __attribute__((visibility("default"))), and setting the > > > > > 'soname' like > > > > > -Wl,-soname=libfsverity.so.0. > > > > > > > > > > Also, is the GPLv2+ license okay for the use case? > > > > > > > > Personally I only care about linking it into rpm, which is GPL > > > > v2, so > > > > from my perspective, that is sufficient. I am also fine making > > > > it LGPL, > > > > but given it's your code I am stealing, I cannot make that call. > > > > > > > > > > Hi Jes, I'd like to revisit this, as I'm concerned about future use > > > cases where > > > software under other licenses (e.g. LGPL, MIT, or Apache 2.0) might > > > want to use > > > libfsverity -- especially if libfsverity grows more functionality. > > > > > > Also, fsverity-utils links to OpenSSL, which some people (e.g. > > > Debian) consider > > > to be incompatible with GPLv2. > > > > > > We think the MIT license would offer the > > > most flexibility. Are you okay with changing the license of > > > fsverity-utils to > > > MIT? If so, I'll send a patch and you can give an Acked-by on it. > > > > > > Thanks! > > > > > > - Eric > > > > Hi Eric, > > > > I went back through my patches to make sure I didn't reuse code from > > other GPL projects. I don't see anything that looks like it was reused > > except from fsverity-utils itself, so it should be fine. > > > > I think it's fair to relax the license so other projects can link to it. > > I would prefer we use the LGPL rather than the MIT license though? > > > > CC'ing Chris Mason as well, since he has the auth to ack it on behalf of > > the company. > > MIT, BSD, LGPL are Signed-off-by: Chris Mason <clm@fb.com> > > We’re flexible, the goal is just to fit into the rest of fsverity overall. > > -chris Thanks Chris and Jes. At least on Google's side, a permissive license generally makes things easier for people -- even though in practice we'll be upstreaming all changes anyway. Since fsverity-utils is only a small project and is unlikely to be customized much by people (as it's closely tied to the upstream kernel support), for now I'd rather not create problems for users or cause duplication of effort. If it were a larger project, or something people would be more likely to customize, the case for LGPL would be stronger IMO. There are also OpenSSL linking exceptions out there even for the LGPL (!), so I'm not sure everyone agrees that one isn't needed... I'd like to avoid wasting time on any such issues and just write code :-) Note that we can always choose to move to LGPL later, but LGPL => MIT won't be possible (since in line with kernel community norms, for fsverity-utils we're only requiring the DCO, not a CLA). I think we shouldn't go down a one-way street too early. I've send out a patch to change the license. Can you two explicitly give Acked-by on the patch itself? Thanks! - Eric
From: Jes Sorensen <jsorensen@fb.com> Hi, I am looking at what it will take to add support for fsverity signatures to rpm, similar to how rpm supports IMA signatures. In order to do so, it makes sense to split the fsverity util into a shared library and the command line tool, so the core functions can be used from other applciations. Alternatively I will have to copy over a good chunk of the code into rpm, which makes it nasty to support long term. This is a first stab at doing that, and I'd like to get some feedback on the approach. I basically split it into four functions: fsverity_cmd_gen_digest(): Build the digest, but do not sign it fsverity_cmd_sign(): Sign the digest structure fsverity_cmd_measure(): Measure a file, basically 'fsverity measure' fsverity_cmd_enable(): Enable verity on a file, basically 'fsverity enable' If we can agree on the approach, then I am happy to deal with the full libtoolification etc. Jes Jes Sorensen (7): Build basic shared library Restructure fsverity_cmd_sign for shared libraries Make fsverity_cmd_measure() a library function Make fsverity_cmd_enable a library call() Rename commands.h to fsverity.h Move cmdline helper functions to fsverity.c cmd_sign: fsverity_cmd_sign() into two functions Makefile | 18 ++- cmd_enable.c | 133 +------------------ cmd_measure.c | 51 ++------ cmd_sign.c | 168 ++++++------------------ commands.h | 24 ---- fsverity.c | 345 +++++++++++++++++++++++++++++++++++++++++++++++--- fsverity.h | 38 ++++++ util.c | 13 ++ 8 files changed, 446 insertions(+), 344 deletions(-) delete mode 100644 commands.h create mode 100644 fsverity.h