[Bug 2054480] Re: [MIR] nbd-client
Thank you all, with that I sum up the details here. - MIR ack with a few requirements - Security Ack - From here 1. add the tests you mentioned to the package 2. continue to !try! isolation (2058040) 3. ready for promotion in 24.10 4. SRU the tests to 24.04 5. we consider this even ok to promote for 24.04.1 Subscribing Dave to continue on that ** Changed in: nbd (Ubuntu) Assignee: (unassigned) => Dave Jones (waveform) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
I gave the nbd-client.c file a very quick read and it looked moderately well-written to me. It feels like it's got nearly three decades of history to it -- solid, been around a while, and maybe you'd do things different if you were doing it again, but it exists today and solves problems, today. So, ACK. :) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
cpaelzer> how about this - sarnold spends 20 minutes, and gives a shallow security review based on this being in main in the past kind of already cpaelzer> if that outcome is good, get it into 24.04 mclemenceau_> thanks cpaelzer sarnold, I'm ok with the plan w.r.t netboot with a preference for addition in 24.04 :) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
With this in light (but we have the wider "everything that is in main for a very long time in ubuntu, even being security reviewed and got multiple uploads), I would agree that -server could have another security/fresh look. Do you think it’s something the security team has the capacity to look? Otherwise, we may not want to special case this case, as the problem is really linked to the pre-existing packages in main (even GNOME for instance in general, didn’t get a security review… and even if it did, GNOME has nothing looking like the one released in 2004). -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
Thanks Wouter It appears nbd-client existed in main at some point http://old- releases.ubuntu.com/ubuntu/pool/main/n/nbd/ (thanks Seth). Between this MIR and tree's LP#2056099 I am concerned that Security is being bypassed as NN approaches. That's not to say anything is wrong with how nbd-client uses ioctl, but we haven't looked. Security is not asking to review this for NN, just flagging for MIR Team discussion. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
Not to the best of my knowledge, no. It was moved to main very early on; I think it might've been 15 years ago. Perhaps the MIR prices didn't exist yet back then? I wouldn't know, not having heard of the whole thing until I saw this one pass by The client's job is to configure an NBD device in the kernel. It does that through the APIs that the kernel provides. There are two of these in existence; one is an older ioctl-based API, the other is a more recent netlink-based one. As these are the only days you can configure NBD device nodes, it makes sense that the are a lot of ioctl calls... -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
Was -server code ever reviewed by a MIR? The client contains many ioctl calls. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
> I really appreciated the detailed bug description. However, some > elements were missing on the MIR template rules and checks, like the > output of lintian pedantic running as per > https://github.com/canonical/ubuntu-mir. Also, some information are > not exact, you state: "* The package installs no services, timers, > or recurring jobs". However, nbd-client installs > /lib/systemd/system/nbd@.service (not nbd-client which is symlinked > to /dev/null). I have then had to recheck and find the informations > myself. Please try to follow the template thoughtfully next time so > that the burden on the MIR team is lighter, and we can process MIR > bugs quicker. :) Sorry about missing that -- the vast majority of my testing had been in the netboot scenario and in those cases that service isn't used (because no instances are enabled; it's all handled by the initrd). Hence I'd seen the systemd server units derived from init.d (more on that below) but completely missed the instanced client unit. > I see that we have a really trivial autopkgtests as of now (I read > the discussion above about it too). I think this one covers the real > basics of the packages but it’s not comprehensive enough. Since the > package was introduced in main, the MIR rules have changed and now > required better testing story than when it was promoted. This > include autopkgtests too. If non trivial autopkgtests is not > feasable, we have then to fallback to a manual test plan, that is > documented, updated, and run with any upload of the package. This > will though not prevent from reverse dependency regression, meaning > that the test plan has to be run manually at regular intervals too. > As you can see, autopkgtest is a way more future-proof looking in > term of maintainance :) Agreed. I've had a crack at enhancing the autopkgtests, and I'm reasonably pleased with the results. Quoting Wouter: > A proper DEP-8 test would at least set up nbd-server, connect an NBD > device, write to it and read from it again. In a branch up on salsa [1] I've now enhanced the existing regression test (nbdtab-ipv4-address) to do just this; it's now a "read-write" test that checks operations on a file-system match between local access and nbd access (I'd have liked to add this as a separate autopkgtest but it turned out that isolation between tests is not as simple as I'd thought; LP: #2058040). > Additionally, the initramfs his that are shipped would preferably be > tested too, which requires a netboot setup. > > This is not trivial to do in a DEP-8 context, and while I have a > plan that could work, I have not yet had the time to implement this. > If anyone is willing to help with this as a prerequisite to moving > this to Ubuntu main, I'm happy to have a chat or something to > discuss the details. This is absolutely something I really really > really want to do, but the complexity of the problem is significant. While this wasn't trivial, this wasn't as hard as I was expecting either, but this is thanks in large part to my being familiar with the sterling efforts of others in sbuild's autopkgtests. These use the new debvm tools to construct and run VMs. What's most impressive is that the use of unshare means we don't even need root, so this test can run even on salsa's infrastructure without machine isolation! Anyway, the result's in the initrd-boot test [1]. Back to Didier: > - We do run nbd-client as a root process for each nbd device block > found via a systemd service. However, I see this one has no systemd > confinement set. It would be great to have as much restriction on > the systemd unit itself to limit the attack surface. We run nbd-client as root for each *enabled* nbd block device (the instanced nbd@ service isn't run implicitly for each block device found, even if they're defined in nbdtab), i.e. instances need to be manually enabled by the administrator. But I do agree that it would be nice to take advantage of systemd's confinement capabilities, both on the client side and the server side. I'll see what I can come up with, aided by the new autopkgtests and propose what I've got so far upstream. [1]: https://salsa.debian.org/waveform/nbd/-/tree/lintian/debian/tests?ref_type=heads -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2054480] Re: [MIR] nbd-client
It's really nice to have upstream and the debian maintainer looking at this issue too, thanks for joining the conversation and your recent release! :) Review for Source Package: nbd (already in main) with focused on promoting nbd-client [Summary] MIR team ACK under the constraint to resolve the list below. This does not need a security review as -server package was already promoted previously and the client doesn’t have a large attack surface. List of specific binary packages to be promoted to main in addition to exising one: nbd-client Notes: I mainly focused my review on the -client binary package. However, we always use this opportunity to rereview the other parts of the code and packaging. Packages are modified, upstream source code too, so the blanket "this is already in main" doesn't prevent us to do another round of review (ideally, that would be done also outside of the MIR cycle), and I used that opportunity for this. Also, the MIR requirements evolves (hopefully in a good direction) with time, and this is the perfect opportunity to get the required TODO being fixed! For instance, I see the recent upload migrated to source 3.0 (quilt) format and this is excellent! I really appreciated the detailed bug description. However, some elements were missing on the MIR template rules and checks, like the output of lintian pedantic running as per https://github.com/canonical/ubuntu-mir. Also, some information are not exact, you state: "* The package installs no services, timers, or recurring jobs". However, nbd-client installs /lib/systemd/system/nbd@.service (not nbd-client which is symlinked to /dev/null). I have then had to recheck and find the informations myself. Please try to follow the template thoughtfully next time so that the burden on the MIR team is lighter, and we can process MIR bugs quicker. :) Required TODOs: - I see that we have a really trivial autopkgtests as of now (I read the discussion above about it too). I think this one covers the real basics of the packages but it’s not comprehensive enough. Since the package was introduced in main, the MIR rules have changed and now required better testing story than when it was promoted. This include autopkgtests too. If non trivial autopkgtests is not feasable, we have then to fallback to a manual test plan, that is documented, updated, and run with any upload of the package. This will though not prevent from reverse dependency regression, meaning that the test plan has to be run manually at regular intervals too. As you can see, autopkgtest is a way more future-proof looking in term of maintainance :) - We do run nbd-client as a root process for each nbd device block found via a systemd service. However, I see this one has no systemd confinement set. It would be great to have as much restriction on the systemd unit itself to limit the attack surface. [Rationale, Duplication and Ownership] The rationale given in the report seems valid and useful for Ubuntu [Dependencies] OK: - no other Dependencies to MIR due to this - nbd checked with `check-mir` - all dependencies can be found in `seeded-in-ubuntu` (already in main) - none of the (potentially auto-generated) dependencies (Depends and Recommends) that are present after build are not in main - no -dev/-debug/-doc packages that need exclusion - No dependencies in main that are only superficially tested requiring more tests now. [Embedded sources and static linking] OK: - no embedded source present - no static linking - does not have unexpected Built-Using entries OK: - not a go package, no extra constraints to consider in that regard - not a rust package, no extra constraints to consider in that regard - Does not include vendored code [Security] OK: - history of CVEs does not look concerning - does not run a daemon as root - does not use webkit1,2 - does not use lib*v8 directly - does not parse data formats (files [images, video, audio, xml, json, asn.1], network packets, structures, ...) from an untrusted source. - does not expose any external endpoint (port/socket/... or similar) - does not process arbitrary web content - does not use centralized online accounts - does not integrate arbitrary javascript into the desktop - does not deal with system authentication (eg, pam), etc) - does not deal with security attestation (secure boot, tpm, signatures) - does not deal with cryptography (en-/decryption, certificates, signing, ...) - this makes appropriate (for its exposure) use of established risk mitigation features (dropping permissions, using temporary environments, restricted users/groups, seccomp, systemd isolation features, apparmor, ...) Problems: - does run nbd-client as a root process for each nbd device block found via a systemd service. However, I see this one has no systemd confinement set. It would be great to have as much restriction on the systemd unit itself to limit the attack surface. [Common blockers] OK: - does not FTBFS
[Bug 2054480] Re: [MIR] nbd-client
** Changed in: nbd (Ubuntu) Assignee: (unassigned) => Didier Roche-Tolomelli (didrocks) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2054480 Title: [MIR] nbd-client To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/2054480/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
