| Message ID | 20220808230833.16275-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hello, > > This small patch series adds support for openssl as an alternative to > gnutls to verify the signature of IPA modules. > > Compared to v1, I have reorganized the series to move the most > controversial part - making the dependency on crypto libraries optional > - on top, in patch 4/4 (previously 1/5 and 2/5, squashed together) to > allow discussions to continue without blocking the other patches. Patch > 4/4, which add libcrypto support, has been modified to not use APIs that > are deprecated in OpenSSL 3.0, to support Fedora 36. > > The dependency on crypto libraries was optional, before we realized that > missing IPA protocol serialization made it effectively required in > practice. Serialization is now there, so module signature support can be > made optional again. This could possibly cause issues for some users who > may not notice the missing dependency and wonder why IPA modules run > isolated (although that should be a fully supported configuration). > > To address this, I've documented module signing as recommended in > README.md (patch 4/4), and emit a warning at meson setup time when the > dependencies are not found. We however all know how often both > documentation and warnings are overlooked. If anyone thinks this is a > bad idea, I can drop (or modify) patch 4/4. > > For the rest of the series, please see individual patches. > > Eric, would you be able to test this on Fedora 36 to check if it fixes > the compilation issues you've reported ? Yes, although I notice it found libcrypto, although I don't see -DHAVE_CRYPTO in the compile line or anything like that, although that could be my misunderstanding of the build scripts. > > Laurent Pinchart (4): > libcamera: meson: Use dependency() to find gnutls > libcamera: pub_key: Gracefully handle failures to load public key > libcamera: pub_key: Support openssl as an alternative to gnutls > libcamera: Make IPA module signing recommended instead of mandatory > > README.rst | 7 ++-- > include/libcamera/internal/pub_key.h | 8 +++-- > src/libcamera/ipa_manager.cpp | 3 ++ > src/libcamera/meson.build | 16 +++++++-- > src/libcamera/pub_key.cpp | 50 +++++++++++++++++++++++++--- > src/meson.build | 3 +- > 6 files changed, 75 insertions(+), 12 deletions(-) > > > base-commit: fe8941d7d61bd22ed66e5b5615e931c68fdf9bfa > -- > Regards, > > Laurent Pinchart >
On Tue, 9 Aug 2022 at 11:40, Eric Curtin <ecurtin@redhat.com> wrote: > > On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hello, > > > > This small patch series adds support for openssl as an alternative to > > gnutls to verify the signature of IPA modules. > > > > Compared to v1, I have reorganized the series to move the most > > controversial part - making the dependency on crypto libraries optional > > - on top, in patch 4/4 (previously 1/5 and 2/5, squashed together) to > > allow discussions to continue without blocking the other patches. Patch > > 4/4, which add libcrypto support, has been modified to not use APIs that > > are deprecated in OpenSSL 3.0, to support Fedora 36. > > > > The dependency on crypto libraries was optional, before we realized that > > missing IPA protocol serialization made it effectively required in > > practice. Serialization is now there, so module signature support can be > > made optional again. This could possibly cause issues for some users who > > may not notice the missing dependency and wonder why IPA modules run > > isolated (although that should be a fully supported configuration). > > > > To address this, I've documented module signing as recommended in > > README.md (patch 4/4), and emit a warning at meson setup time when the > > dependencies are not found. We however all know how often both > > documentation and warnings are overlooked. If anyone thinks this is a > > bad idea, I can drop (or modify) patch 4/4. > > > > For the rest of the series, please see individual patches. > > > > Eric, would you be able to test this on Fedora 36 to check if it fixes > > the compilation issues you've reported ? > > Yes, although I notice it found libcrypto, although I don't see > -DHAVE_CRYPTO in the compile line or anything like that, although that > could be my misunderstanding of the build scripts. Should have clarified, it builds on Fedora 36. > > > > > > > Laurent Pinchart (4): > > libcamera: meson: Use dependency() to find gnutls > > libcamera: pub_key: Gracefully handle failures to load public key > > libcamera: pub_key: Support openssl as an alternative to gnutls > > libcamera: Make IPA module signing recommended instead of mandatory > > > > README.rst | 7 ++-- > > include/libcamera/internal/pub_key.h | 8 +++-- > > src/libcamera/ipa_manager.cpp | 3 ++ > > src/libcamera/meson.build | 16 +++++++-- > > src/libcamera/pub_key.cpp | 50 +++++++++++++++++++++++++--- > > src/meson.build | 3 +- > > 6 files changed, 75 insertions(+), 12 deletions(-) > > > > > > base-commit: fe8941d7d61bd22ed66e5b5615e931c68fdf9bfa > > -- > > Regards, > > > > Laurent Pinchart > >
Hi Eric, On Tue, Aug 09, 2022 at 11:40:50AM +0100, Eric Curtin wrote: > On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart wrote: > > > > Hello, > > > > This small patch series adds support for openssl as an alternative to > > gnutls to verify the signature of IPA modules. > > > > Compared to v1, I have reorganized the series to move the most > > controversial part - making the dependency on crypto libraries optional > > - on top, in patch 4/4 (previously 1/5 and 2/5, squashed together) to > > allow discussions to continue without blocking the other patches. Patch > > 4/4, which add libcrypto support, has been modified to not use APIs that > > are deprecated in OpenSSL 3.0, to support Fedora 36. > > > > The dependency on crypto libraries was optional, before we realized that > > missing IPA protocol serialization made it effectively required in > > practice. Serialization is now there, so module signature support can be > > made optional again. This could possibly cause issues for some users who > > may not notice the missing dependency and wonder why IPA modules run > > isolated (although that should be a fully supported configuration). > > > > To address this, I've documented module signing as recommended in > > README.md (patch 4/4), and emit a warning at meson setup time when the > > dependencies are not found. We however all know how often both > > documentation and warnings are overlooked. If anyone thinks this is a > > bad idea, I can drop (or modify) patch 4/4. > > > > For the rest of the series, please see individual patches. > > > > Eric, would you be able to test this on Fedora 36 to check if it fixes > > the compilation issues you've reported ? > > Yes, Nice to know it now works :-) Can I add your Tested-by ? Reviews are also always appreciated if you have time. > although I notice it found libcrypto, although I don't see > -DHAVE_CRYPTO in the compile line or anything like that, although that > could be my misunderstanding of the build scripts. It's added to the auto-generated config.h in the build directory. > > Laurent Pinchart (4): > > libcamera: meson: Use dependency() to find gnutls > > libcamera: pub_key: Gracefully handle failures to load public key > > libcamera: pub_key: Support openssl as an alternative to gnutls > > libcamera: Make IPA module signing recommended instead of mandatory > > > > README.rst | 7 ++-- > > include/libcamera/internal/pub_key.h | 8 +++-- > > src/libcamera/ipa_manager.cpp | 3 ++ > > src/libcamera/meson.build | 16 +++++++-- > > src/libcamera/pub_key.cpp | 50 +++++++++++++++++++++++++--- > > src/meson.build | 3 +- > > 6 files changed, 75 insertions(+), 12 deletions(-) > > > > > > base-commit: fe8941d7d61bd22ed66e5b5615e931c68fdf9bfa
On Tue, 9 Aug 2022 at 11:43, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Eric, > > On Tue, Aug 09, 2022 at 11:40:50AM +0100, Eric Curtin wrote: > > On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart wrote: > > > > > > Hello, > > > > > > This small patch series adds support for openssl as an alternative to > > > gnutls to verify the signature of IPA modules. > > > > > > Compared to v1, I have reorganized the series to move the most > > > controversial part - making the dependency on crypto libraries optional > > > - on top, in patch 4/4 (previously 1/5 and 2/5, squashed together) to > > > allow discussions to continue without blocking the other patches. Patch > > > 4/4, which add libcrypto support, has been modified to not use APIs that > > > are deprecated in OpenSSL 3.0, to support Fedora 36. > > > > > > The dependency on crypto libraries was optional, before we realized that > > > missing IPA protocol serialization made it effectively required in > > > practice. Serialization is now there, so module signature support can be > > > made optional again. This could possibly cause issues for some users who > > > may not notice the missing dependency and wonder why IPA modules run > > > isolated (although that should be a fully supported configuration). > > > > > > To address this, I've documented module signing as recommended in > > > README.md (patch 4/4), and emit a warning at meson setup time when the > > > dependencies are not found. We however all know how often both > > > documentation and warnings are overlooked. If anyone thinks this is a > > > bad idea, I can drop (or modify) patch 4/4. > > > > > > For the rest of the series, please see individual patches. > > > > > > Eric, would you be able to test this on Fedora 36 to check if it fixes > > > the compilation issues you've reported ? > > > > Yes, > > Nice to know it now works :-) Can I add your Tested-by ? Reviews are > also always appreciated if you have time. > > > although I notice it found libcrypto, although I don't see > > -DHAVE_CRYPTO in the compile line or anything like that, although that > > could be my misunderstanding of the build scripts. > > It's added to the auto-generated config.h in the build directory. Yes I see now thanks. > > > > Laurent Pinchart (4): > > > libcamera: meson: Use dependency() to find gnutls > > > libcamera: pub_key: Gracefully handle failures to load public key > > > libcamera: pub_key: Support openssl as an alternative to gnutls > > > libcamera: Make IPA module signing recommended instead of mandatory > > > > > > README.rst | 7 ++-- > > > include/libcamera/internal/pub_key.h | 8 +++-- > > > src/libcamera/ipa_manager.cpp | 3 ++ > > > src/libcamera/meson.build | 16 +++++++-- > > > src/libcamera/pub_key.cpp | 50 +++++++++++++++++++++++++--- > > > src/meson.build | 3 +- > > > 6 files changed, 75 insertions(+), 12 deletions(-) > > > > > > > > > base-commit: fe8941d7d61bd22ed66e5b5615e931c68fdf9bfa > > -- > Regards, > > Laurent Pinchart >
On Tue, 9 Aug 2022 at 11:44, Eric Curtin <ecurtin@redhat.com> wrote: > > On Tue, 9 Aug 2022 at 11:43, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > > > Hi Eric, > > > > On Tue, Aug 09, 2022 at 11:40:50AM +0100, Eric Curtin wrote: > > > On Tue, 9 Aug 2022 at 00:08, Laurent Pinchart wrote: > > > > > > > > Hello, > > > > > > > > This small patch series adds support for openssl as an alternative to > > > > gnutls to verify the signature of IPA modules. > > > > > > > > Compared to v1, I have reorganized the series to move the most > > > > controversial part - making the dependency on crypto libraries optional > > > > - on top, in patch 4/4 (previously 1/5 and 2/5, squashed together) to > > > > allow discussions to continue without blocking the other patches. Patch > > > > 4/4, which add libcrypto support, has been modified to not use APIs that > > > > are deprecated in OpenSSL 3.0, to support Fedora 36. > > > > > > > > The dependency on crypto libraries was optional, before we realized that > > > > missing IPA protocol serialization made it effectively required in > > > > practice. Serialization is now there, so module signature support can be > > > > made optional again. This could possibly cause issues for some users who > > > > may not notice the missing dependency and wonder why IPA modules run > > > > isolated (although that should be a fully supported configuration). > > > > > > > > To address this, I've documented module signing as recommended in > > > > README.md (patch 4/4), and emit a warning at meson setup time when the > > > > dependencies are not found. We however all know how often both > > > > documentation and warnings are overlooked. If anyone thinks this is a > > > > bad idea, I can drop (or modify) patch 4/4. > > > > > > > > For the rest of the series, please see individual patches. > > > > > > > > Eric, would you be able to test this on Fedora 36 to check if it fixes > > > > the compilation issues you've reported ? > > > > > > Yes, > > > > Nice to know it now works :-) Can I add your Tested-by ? Reviews are > > also always appreciated if you have time. > > > > > although I notice it found libcrypto, although I don't see > > > -DHAVE_CRYPTO in the compile line or anything like that, although that > > > could be my misunderstanding of the build scripts. > > > > It's added to the auto-generated config.h in the build directory. > > Yes I see now thanks. Tested-by: Eric Curtin <ecurtin@redhat.com> > > > > > > > Laurent Pinchart (4): > > > > libcamera: meson: Use dependency() to find gnutls > > > > libcamera: pub_key: Gracefully handle failures to load public key > > > > libcamera: pub_key: Support openssl as an alternative to gnutls > > > > libcamera: Make IPA module signing recommended instead of mandatory > > > > > > > > README.rst | 7 ++-- > > > > include/libcamera/internal/pub_key.h | 8 +++-- > > > > src/libcamera/ipa_manager.cpp | 3 ++ > > > > src/libcamera/meson.build | 16 +++++++-- > > > > src/libcamera/pub_key.cpp | 50 +++++++++++++++++++++++++--- > > > > src/meson.build | 3 +- > > > > 6 files changed, 75 insertions(+), 12 deletions(-) > > > > > > > > > > > > base-commit: fe8941d7d61bd22ed66e5b5615e931c68fdf9bfa > > > > -- > > Regards, > > > > Laurent Pinchart > >