[Bug 2257948] New: Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

Bug ID: 2257948
           Summary: Review Request: bpfman - EBPF Program Manager
           Product: Fedora
           Version: rawhide
          Hardware: All
                OS: Linux
            Status: NEW
         Component: Package Review
          Severity: medium
          Priority: medium
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]
  Target Milestone: ---
    Classification: Fedora

Spec URL: https://mikel.olasagasti.info/tmp/fedora/bpfman.spec
SRPM URL:
https://mikel.olasagasti.info/tmp/fedora/bpfman-0.4.0-0.1.20240110gitc6d1da2...
Description: bpfman operates as an eBPF manager, focusing on simplifying the
deployment and administration of eBPF programs.
Fedora Account System Username: mikelo2

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

Mikel Olasagasti Uranga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Doc Type|---                         |If docs needed, set a value

--- Comment #1 from Mikel Olasagasti Uranga  ---
Note: As this is a bundled spec, I uploaded the vendor tarball to a public
server to help automatic fedora-review process. Spec has commented the Source
entry that should be used and also the steps to generate it.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #2 from Fedora Review Service  ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6885806
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...

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.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]

--- Comment #3 from Fabio Valentini  ---
Package looks good in general, but the way the crate dependencies are handled
is not good right now.

1. In general, using vendored sources is strongly discouraged (in general, and
also for Rust packages). I have looked at the contents of the vendor tarball,
and it looks like almost all dependencies are already packaged for Fedora. Do
you have a list of crate dependencies that are missing from Fedora? If it is
not too long, I would *very much* prefer to not use vendored dependencies here
(except for the git snapshot of aya that is apparently necessary). I (and other
Rust SIG members) can help with packaging and / or reviewing missing
dependencies.

2. *If* it turns out that using *all* vendored dependencies is indeed necessary
(see Rust Packaging Guidelines for replacing Git snapshots with path-based
dependencies), you will need to do at least *some* auditing of the contents of
the vendor tarball.

I can tell just by looking at the list of vendored crates that there are is
least some content that is not acceptable for redistributing in Fedora (most
importantly, cryptography algorithm implementations that are not legally
allowed in Fedora in the fiat-crypto crate).

You can also use automation to generate the list of bundled crates with the
`%cargo_vendor_manifest` macro and adding that file as a `%license` file, there
is no need to list the bundled crates manually anymore. In fact, you are
already using that, so there is no need to list "Provides: bundled(crate(...))"
at all, since it is redundant with the Provides generator.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #4 from Mikel Olasagasti Uranga  ---
Thanks for reviewing Fabio!

I tried %cargo_generate_buildrequires against bpfman workspace only and the
following deps would be missing:

Problem 1: nothing provides requested (crate(aya) >= 0.11.0 with crate(aya) <
0.12.0~)
 Problem 2: nothing provides requested (crate(comfy-table) >= 7.1.0 with
crate(comfy-table) < 8.0.0~)
 Problem 3: nothing provides requested (crate(comfy-table/tty) >= 7.1.0 with
crate(comfy-table/tty) < 8.0.0~)
 Problem 4: nothing provides requested (crate(netlink-packet-route) >= 0.17.1
with crate(netlink-packet-route) < 0.18.0~)
 Problem 5: nothing provides requested (crate(oci-distribution) >= 0.9.0 with
crate(oci-distribution) < 0.10.0~)
 Problem 6: nothing provides requested (crate(oci-distribution/rustls-tls) >=
0.9.0 with crate(oci-distribution/rustls-tls) < 0.10.0~)
 Problem 7: nothing provides requested (crate(oci-distribution/trust-dns) >=
0.9.0 with crate(oci-distribution/trust-dns) < 0.10.0~)
 Problem 8: nothing provides requested (crate(prost) >= 0.12.3 with
crate(prost) < 0.13.0~)
 Problem 9: nothing provides requested (crate(prost-types) >= 0.12.3 with
crate(prost-types) < 0.13.0~)
 Problem 10: nothing provides requested (crate(prost/prost-derive) >= 0.12.3
with crate(prost/prost-derive) < 0.13.0~)
 Problem 11: nothing provides requested (crate(prost/std) >= 0.12.3 with
crate(prost/std) < 0.13.0~)
 Problem 12: nothing provides requested (crate(rtnetlink) >= 0.13.1 with
crate(rtnetlink) < 0.14.0~)
 Problem 13: nothing provides requested (crate(rtnetlink/tokio_socket) >=
0.13.1 with crate(rtnetlink/tokio_socket) < 0.14.0~)
 Problem 14: nothing provides requested (crate(sigstore) >= 0.7.2 with
crate(sigstore) < 0.8.0~)
 Problem 15: nothing provides requested (crate(sigstore/cached-client) >= 0.7.2
with crate(sigstore/cached-client) < 0.8.0~)
 Problem 16: nothing provides requested (crate(sigstore/cosign-rustls-tls) >=
0.7.2 with crate(sigstore/cosign-rustls-tls) < 0.8.0~)
 Problem 17: nothing provides requested (crate(sigstore/tuf) >= 0.7.2 with
crate(sigstore/tuf) < 0.8.0~)
 Problem 18: nothing provides requested (crate(sled) >= 0.34.7 with crate(sled)
< 0.35.0~)
 Problem 19: nothing provides requested (crate(systemd-journal-logger) >= 1.0.0
with crate(systemd-journal-logger) < 2.0.0~)
 Problem 20: nothing provides requested (crate(tonic) >= 0.10.2 with
crate(tonic) < 0.11.0~)
 Problem 21: nothing provides requested (crate(tonic/codegen) >= 0.10.2 with
crate(tonic/codegen) < 0.11.0~)
 Problem 22: nothing provides requested (crate(tonic/prost) >= 0.10.2 with
crate(tonic/prost) < 0.11.0~)
 Problem 23: nothing provides requested (crate(tonic/transport) >= 0.10.2 with
crate(tonic/transport) < 0.11.0~)

I can try to build them to see how feasible having bpfman unbundled would be,
but things like oci-distribution at least in go-sig it's a huge challenge.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #5 from Fabio Valentini  ---
Taking a look at the missing dependencies:

- aya: appears to require a git snapshot according to the comments in the spec
file, cannot be un-bundled
- comfy-table: packaged for Fedora, just too old (v4 vs. v7)
- netlink-packet-route: packaged for Fedora, but too new (v0.18 vs. v0.17)
- oci-distribution: not packaged for Fedora (does not look too bad to add it)
- prost / prost-types: packaged for Fedora, but too old (v0.12 vs v0.10)
- rtnetlink: packaged for Fedora, but too new (v0.14 vs v0.13)
- sigstore: not packaged for Fedora (does not look too bad to add it)
- sled: not packaged for Fedora (looks like we already have all its
dependencies though)
- systemd-journal-logger: not packaged for Fedora (we already have all its
dependencies)
- tonic: not packaged for Fedora (I *think* we have all its dependencies)

====================

To sum it up, these are missing:

- oci-distribution
- sigstore
- sled
- systemd-journal-logger
- tonic

These are too new in Fedora, you might be able to patch bpfman to use the new
versions or request that this happens upstream:

- netlink-packet-route
- rtnetlink

These are too old in Fedora, I should be able to update them soon in Fedora if
necessary:

- comfy-table
- prost / prost-types

So that would mean (estimated) ~10-15 new packages for missing dependencies,
plus updates for some existing packages.

====================

If you want to get bpfman into Fedora quickly, I do not want to block on
building against vendored dependencies as it is done right now.
However, in that case, I would like to work on getting the missing pieces in so
it could stop using vendored dependences at some point in the future.

What do you think?

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #6 from Mikel Olasagasti Uranga  ---
I like your idea and if we can have bpfman vendored now we can work during
February to have all deps in place.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #7 from Fabio Valentini  ---
That sounds good to me, however you will still need to verify that the vendor
tarball contains only permissible content: see point 2 in my previous comment
https://bugzilla.redhat.com/show_bug.cgi?id=2257948#c3

This will likely require at least some amount of modifying and / or patching
the sources of some vendored dependencies, similar to what we do in the Fedora
package for the fiat-crypto crate. I have not done a complete audit of the
vendor tarball though, so there is likely other stuff that would need to be
cleaned up for legal reasons.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #8 from Daniel Mellado  ---
(In reply to Fabio Valentini from comment #5)
> netlink-packet-route
> rtnetlink

Updated bpfman to match these dependencies, so this wouldn't be an issue
anymore

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #9 from Daniel Mellado  ---
(In reply to Fabio Valentini from comment #7)
> That sounds good to me, however you will still need to verify that the
> vendor tarball contains only permissible content: see point 2 in my previous
> comment https://bugzilla.redhat.com/show_bug.cgi?id=2257948#c3
>
> This will likely require at least some amount of modifying and / or patching
> the sources of some vendored dependencies, similar to what we do in the
> Fedora package for the fiat-crypto crate. I have not done a complete audit
> of the vendor tarball though, so there is likely other stuff that would need
> to be cleaned up for legal reasons.

Thanks, that make sense, we'll take a look at that crate but, are there any
specific guidelines for this audit to be done?

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #10 from Fabio Valentini  ---
> Thanks, that make sense, we'll take a look at that crate but, are there any specific guidelines for this audit to be done?

In principle, the same source audit that should be done for every package
review:

- Are there any pre-built binaries that are used during the build process? If
yes, can they be replaced and / or rebuilt from source?
- Are there any contents that are under licenses that would prohibit it from
being distributed by Fedora?
- Are there any implementations of cryptographic algorithms that Fedora is not
allowed to distribute?
- etc.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #11 from Daniel Mellado  ---
Hi @fab, I've checked mainly re: implementations of algorithms with some
dedicated folks (thanks @cclang) and it should be fine.

Can we move forward with the bundled version?

We'll still commit to provide the unbundled and switch to it but as you can see
we're waiting on some packages and iterating on a few others and the list keeps
growing. I'd like to unblock this while keeping the work, is there any issue
from your side? Thanks! Pointing to specifics we're waiting on:

# Updated on bpfman

- netlink-packet-route: packaged for Fedora, but too new (v0.18 vs. v0.17) ->
updated in bpfman, should be ok now
- rtnetlink: packaged for Fedora, but too new (v0.14 vs v0.13)

# Waiting to be updated on Fedora
- comfy-table: packaged for Fedora, just too old (v4 vs. v7)

- aya: cannot be un-bundled, trying to get a new release so we can package it
instead of git
- sled: https://bugzilla.redhat.com/show_bug.cgi?id=2259675
  PR open to sled -> https://github.com/spacejam/sled/pull/1500 updates
rand_distr to 0.4.3
  PR open to sled -> https://github.com/spacejam/sled/pull/1501 updates
zerocopyu to 0.7

# These brings a lot of dependencies, with several dependency tree levels and
somehow blocked. Can provide exact details if you'd like to discuss.

- sigstore: not packaged for Fedora
- oci-distribution: not packaged for Fedora (does not look too bad to add it)
- systemd-journal-logger: not packaged for Fedora

# We discussed about these ones and you mentioned that they'll be tackled later
from rust-sig
- prost / prost-types: packaged for Fedora, but too old (v0.12 vs v0.10)
- tonic: not packaged for Fedora (I *think* we have all its dependencies)

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #12 from Daniel Mellado  ---
As a follow up, taking for example sigstore gets us to:

A few of those are in review and I already provided  rpm and upstream fixes for
a few of those such as
 Problem 1: nothing provides requested (crate(cached/async) >= 0.46.0 with
crate(cached/async) < 0.47.0~)   - In Review
https://bugzilla.redhat.com/show_bug.cgi?id=2259225
 Problem 2: nothing provides requested (crate(cached/default) >= 0.46.0 with
crate(cached/default) < 0.47.0~) - In Review
https://bugzilla.redhat.com/show_bug.cgi?id=2259225
 Problem 3: nothing provides requested (crate(crypto_secretbox/default) >=
0.1.1 with crate(crypto_secretbox/default) < 0.2.0~) - In Review
https://bugzilla.redhat.com/show_bug.cgi?id=2259330

Pending or with different versions in Fedora 
 Problem 4: nothing provides requested (crate(docker_credential/default) >=
1.1.0 with crate(docker_credential/default) < 2.0.0~)

Initial build, pending from some update
https://bugzilla.redhat.com/show_bug.cgi?id=1981119 
 Problem 5: nothing provides requested (crate(ed25519-dalek/default) >=
2.0.0~rc.2 with crate(ed25519-dalek/default) < 3.0.0~)
 Problem 6: nothing provides requested (crate(ed25519-dalek/pkcs8) >=
2.0.0~rc.2 with crate(ed25519-dalek/pkcs8) < 3.0.0~)
 Problem 7: nothing provides requested (crate(ed25519-dalek/rand_core) >=
2.0.0~rc.2 with crate(ed25519-dalek/rand_core) < 3.0.0~)
 Problem 8: nothing provides requested (crate(ed25519/alloc) >= 2.2.1 with
crate(ed25519/alloc) < 3.0.0~)
 Problem 9: nothing provides requested (crate(ed25519/default) >= 2.2.1 with
crate(ed25519/default) < 3.0.0~)

Not in Fedora
 Problem 10: nothing provides requested (crate(oci-distribution) >= 0.10.0 with
crate(oci-distribution) < 0.11.0~)
 Problem 11: nothing provides requested (crate(oci-distribution/native-tls) >=
0.10.0 with crate(oci-distribution/native-tls) < 0.11.0~)
 Problem 12: nothing provides requested (crate(olpc-cjson/default) >= 0.1.0
with crate(olpc-cjson/default) < 0.2.0~)
 Problem 13: nothing provides requested (crate(openidconnect) >= 3.0.0 with
crate(openidconnect) < 4.0.0~)
 Problem 14: nothing provides requested (crate(openidconnect/native-tls) >=
3.0.0 with crate(openidconnect/native-tls) < 4.0.0~)
 Problem 15: nothing provides requested (crate(openidconnect/reqwest) >= 3.0.0
with crate(openidconnect/reqwest) < 4.0.0~)
 Problem 16: nothing provides requested (crate(p256/default) >= 0.13.2 with
crate(p256/default) < 0.14.0~)

Packaged in Fedora but different version 
https://src.fedoraproject.org/rpms/rust-rsa
 Problem 17: nothing provides requested (crate(rsa/default) >= 0.9.2 with
crate(rsa/default) < 0.10.0~)

Not in Fedora 
 Problem 18: nothing provides requested (crate(rustls-pki-types/default) >=
1.0.0 with crate(rustls-pki-types/default) < 2.0.0~)
 Problem 19: nothing provides requested (crate(rustls-pki-types/std) >= 1.0.0
with crate(rustls-pki-types/std) < 2.0.0~)
 Problem 20: nothing provides requested (crate(rustls-webpki/alloc) >= 0.102.0
with crate(rustls-webpki/alloc) < 0.103.0~)
 Problem 21: nothing provides requested (crate(rustls-webpki/default) >=
0.102.0 with crate(rustls-webpki/default) < 0.103.0~)

Not in Fedora and would need LICENSE files fix
 Problem 22: nothing provides requested (crate(testcontainers/default) >=
0.15.0 with crate(testcontainers/default) < 0.16.0~)

Not in Fedora
 Problem 23: nothing provides requested (crate(tough/default) >= 0.14.0 with
crate(tough/default) < 0.15.0~) 
 Problem 24: nothing provides requested (crate(tough/http) >= 0.14.0 with
crate(tough/http) < 0.15.0~)
 Problem 25: nothing provides requested (crate(x509-cert/default) >= 0.2.2 with
crate(x509-cert/default) < 0.3.0~)
 Problem 26: nothing provides requested (crate(x509-cert/pem) >= 0.2.2 with
crate(x509-cert/pem) < 0.3.0~)
 Problem 27: nothing provides requested (crate(x509-cert/std) >= 0.2.2 with
crate(x509-cert/std) < 0.3.0~)

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #13 from Clemens Lang  ---
I looked through your dependency tree from a cryptographic point of view, and
most of it is fine.

A lot of the cryptography is implemented in rust and thus does not use one of
the implementations we validate according to various standards on RHEL. This
means that, for example, a system running in FIPS mode or a
CommonCriteria-certified system should not make security assumptions about
bpfman (i.e., whatever signatures it verifies should not be considered
trusted). That's not an issue for Fedora, though.

As for specific questionable algorithms that are in your vendored source tree:

- You already identified p434 in the fiat-crypto crate. See [1,2,3] for
reference, background discussion and patches. Maybe you can not bundle
fiat-crypto and use the version from Fedora?
- cruve25519-dalek contains an implementation of the Ristretto group [4,5]. I'm
not aware of prior legal review of that group, so maybe we shouldn't include
it.

I'm also somewhat generally concerned about side-channel vulnerabilities in
those cryptographic implementations. We test the common implementations on RHEL
and Fedora, but not those Rust ones. We already know that some of those crates
are vulnerable to side-channel attacks [6,7].

My recommendation would thus be to drop support for the p343 and Ristretto
curves, and tread carefully when it comes to some of the other libraries with
respect to side-channel issues.

[1]:
https://lists.fedoraproject.org/archives/list/legal%40lists.fedoraproject.or...
[2]:
https://src.fedoraproject.org/rpms/rust-fiat-crypto/blob/rawhide/f/gen_clean...
[3]: https://fedoraproject.org/wiki/Legal:ECC
[4]:
https://github.com/dalek-cryptography/curve25519-dalek/blob/curve25519-4.1.1...
[5]: https://ristretto.group/ristretto.html
[6]: https://github.com/RustCrypto/RSA/issues/19
[7]:
https://github.com/RustCrypto/RSA/blob/master/src/algorithms/pkcs1v15.rs#L59...

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #14 from Fabio Valentini  ---
I'm working through my backlog and will come back to this review ASAP.

Regarding the previous comment:
> Maybe you can not bundle fiat-crypto and use the version from Fedora?

The way cargo treats dependencies is more or less an all-or-nothing approach
when it comes to vendored dependencies. Either you use all dependencies from
the system registry, or you use all dependencies from the vendor directory. In
most cases, it's not possible to do something in the middle. So I'm pretty sure
using vendored dependencies for most things and using the system dependencies
for *some* things is not feasible.

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

Daniel Mellado  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?(mikel@olasagasti.
                   |                            |info)

--- Comment #15 from Daniel Mellado  ---
(In reply to Mikel Olasagasti Uranga from comment #0)
> Spec URL: https://mikel.olasagasti.info/tmp/fedora/bpfman.spec
> SRPM URL:
> https://mikel.olasagasti.info/tmp/fedora/bpfman-0.4.0-0.1.20240110gitc6d1da2.
> fc39.src.rpm
> Description: bpfman operates as an eBPF manager, focusing on simplifying the
> deployment and administration of eBPF programs.
> Fedora Account System Username: mikelo2

I've already reached out to you, but we'd need to get this updated. If you
don't have the bandwidth to do so, would you mind dropping the issue so I
either take over or submit a new one? There's been (and will be) a few changes
in the deps, updating a few and moving from sigstore to native-tls. Thanks

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

--- Comment #16 from Daniel Mellado  ---
superseded by https://bugzilla.redhat.com/show_bug.cgi?id=2269411 in order to
be able to take this over from Mikel

[Bug 2257948] Review Request: bpfman - EBPF Program Manager

https://bugzilla.redhat.com/show_bug.cgi?id=2257948

Fabio Valentini  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |DUPLICATE
              Flags|needinfo?(mikel@olasagasti. |
                   |info)                       |
        Last Closed|                            |2024-03-14 13:28:05

--- Comment #17 from Fabio Valentini  ---
Updated and resubmitted due to lack of response.

*** This bug has been marked as a duplicate of bug 2269411 ***