Spec URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys.spec SRPM URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys-0.26.0-1.fc41.src.rpm Description: AWS-LC is a general-purpose cryptographic library maintained by the AWS Cryptography team for AWS and their customers. It іs based on code from the Google BoringSSL project and the OpenSSL project. Fedora Account System Username: decathorpe
Copr build: https://copr.fedorainfracloud.org/coprs/build/8730857 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2350145-rust-aws-lc-sys/fedora-rawhide-x86_64/08730857-rust-aws-lc-sys/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Notes: 1. I have not submitted a koji scratch build, though there are test builds in COPR for all architectures: https://copr.fedorainfracloud.org/coprs/decathorpe/aws-lc-rs/monitor/ 2. I put up a repo on pagure with spec file, patches, and rust2rpm.toml config file for easier review: https://pagure.io/aws-lc-rs/blob/main/f/rust-aws-lc-sys 3. After looking through the source code, I don't think this package contains anything objectionable (wrt. cryptography that Fedora isn't allowed to ship). This needs a double-check. 4. I don't think un-bundling aws-lc (the C / C++ library) is feasible. aws-lc doesn't provide a stable ABI, and the bindings in aws-lc-sys are tightly coupled to a specific version (the version that is bundled). The -devel subpackage (with contains the source code) has appropriate "Provides: bundled(aws-lc) = 1.46.0". 5. There are some pre-compiled object files included (used only in Windows), but they are stripped in %prep regardless. 6. The patches should be appropriately documented in the spec file, but here's a summary: - Patch 0001: Unconditionally regenerate Rust bindings for aws-lc at build time (requires rustfmt). - Patch 0002: Set CMake mode for building the bundled aws-lc to RelWithDebInfo unconditionally. - Patch 0003: Enable re-generating some generated assembly code from scratch (requires Perl). - Patch 0004: Hack to work around a "bug" in the cmake crate which would otherwise cause build failures. - Patch 0005: Set CMake minimum_required_version to 3.15 (instead of 3.0) to avoid failures with CMake 4. 7. This has not yet gone through the review wrt/ Crypto Policies: https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/#_new_crypto_libraries Note that I'm not sure whether this requirement even applies here (or *can* be applied). In the default configuration, aws-lc-sys provides only APIs that are roughly equivalent to libcrypto from OpenSSL, not those from libssl. As I understand it, that amounts to low-level cryptography primitives, and not protocol implementations like TLS. I'm not sure applying a crypto policy in these low-level APIs would make sense - or would even be possible. To me, it seems like crypto policy enforcement would need to be handled in higher level code, like actual TLS protocol implementations. This crate (aws-lc-sys) does provide a feature ("ssl") to optionally also provide symbols equivalent to libssl, however, this feature is unused in the "safe" Rust bindings in aws-lc-rs. So, to be safe, it would even be possible to unconditionally disable this feature in the package.
It seems that if both Go and Perl are not found in the CMake build, it uses the pre-generated sources [1]. I wonder if it can be relaxed to not check the go lang, though I don't see `GENERATE_CODE_ROOT` without a check for PERL_EXECUTABLE, so probably it's fine. Best to rm `generated-src` just in case. [1]: https://github.com/aws/aws-lc/blob/5cdc082bd86ffb4539e53444d7806a128ace77f3/CMakeLists.txt#L169-L171
Good find. It should be fine to drop that logic and the files in /generated-src. At least according to the docs, the golang dependency is only for generating FIPS related sources, and we don't use this here.
Turns out this is not as easy as I thought - not all code for generating those sources in included in published crates, so they can't fully be regenerated from scratch offline. This is a problem: https://docs.fedoraproject.org/en-US/packaging-guidelines/what-can-be-packaged/#pregenerated-code I'm not yet sure which files would all need to be included either in published crates or as extra source files.
It turns out that only *one* .go file is missing from the published crates. Adding that, the packaging now re-generates *all* pre-generated code (both Rust code with bindgen, and aws-lc code with Perl and Go). Updated files are available behind the same URLs, and changes can be reviewed here: https://pagure.io/aws-lc-rs/c/bee3491 Updated builds in COPR: https://copr.fedorainfracloud.org/coprs/decathorpe/aws-lc-rs/monitor/
Additionally to the crypto policies issues, this crate is statically building a copy of the C library, not just providing bindings, this is probably not ok for Fedora policy. It would be better if usptream could give an option to build gainst one of the crypto team sanctioned crypto libraries, for example using rust-openssl.
> this crate is statically building a copy of the C library, not just providing bindings, this is probably not ok for Fedora policy. This is definitely not *preferred* from a Packaging Guidelines point of view, but it's not *forbidden*. > It would be better if usptream could give an option to build gainst one of the crypto team sanctioned crypto libraries, for example using rust-openssl. FWIW, I agree. I will elaborate in the post to the crypto-team mailing list that is mandatory for package reviews like this. But TL;DR is that as far as I can tell, this is the least bad option we have for Fedora packaging. (I am working to replace "rust-ring" with "rust-aws-lc-rs" - aligning with the changed defaults in rustls upstream, and moving from a project with arguably spotty maintenance to one backed by Amazon / AWS. If you think this package is bad, don't look at rust-ring. ;))
Spec URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys.spec SRPM URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys-0.27.1-1.fc42.src.rpm - Update to latest version (0.27.1) with aws-lc 1.48.5. - Drop some downstream patches / workarounds that are no longer necessary due to upstream fixes. - Filed an issue upstream about the missing source file when attempting to re-generate all generated code. Successful COPR build for all architectures: https://copr.fedorainfracloud.org/coprs/decathorpe/aws-lc-rs/monitor/
Spec URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys.spec SRPM URL: https://decathorpe.fedorapeople.org/rust-aws-lc-sys-0.28.0-1.fc42.src.rpm - Update to latest version (0.28.0) with aws-lc 1.49.0. - Rebased downstream patches. - Dropped additional source file that is now included in published crates. Successful COPR build for all architectures: https://copr.fedorainfracloud.org/coprs/decathorpe/aws-lc-rs/build/8865637/
Created attachment 2083634 [details] The .spec file difference from Copr build 8730857 to 8865678
Copr build: https://copr.fedorainfracloud.org/coprs/build/8865678 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2350145-rust-aws-lc-sys/fedora-rawhide-x86_64/08865678-rust-aws-lc-sys/fedora-review/review.txt Found issues: - No gcc, gcc-c++ or clang found in BuildRequires Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/C_and_C++/ Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.