Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/avahi handle bogus services gracefully #298069

Merged

Conversation

muggenhor
Copy link
Contributor

Description of changes

This applies avahi/avahi#523, the fix for avahi/avahi#212, where having a single invalid service being published inside a network could DoS the discovery by all avahi clients.

For me this happens with a "SIEMENS HM676G0S6" oven added to my network. This is the same company as mentioned in the original issue (Bosch, a brand which AFAIK is owned by Siemens).

Additionally this collapses the manual editing of the patches done by #269599 into a simple excludes list passed to fetchpatch.

Note that an alternate option is to use the 0.9-rc1 release candidate, but I don't know what the policy for using pre-releases is.

Things done

  • Built on platform(s)
    • x86_64-linux (only built and tested this cherry-picked to nixos-23.11)
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@muggenhor
Copy link
Contributor Author

NOTE: This, together with #269599, should be backported to 23.11.

And #269599 should possibly be merged to 23.05 too (given that oldestSupportedRelease was 2305 at the time that PR was merged).

@evverx
Copy link

evverx commented Mar 23, 2024

Note that an alternate option is to use the 0.9-rc1 release candidate, but I don't know what the policy for using pre-releases

FWIW it's rc because it came with no release notes (the release notes are blocked by avahi/avahi#516). Other than that it's a normal release with a lot of bug fixes and some distros like Arch have already switched to it.

There were some build system changes like avahi/avahi@9350e8e and avahi/avahi@bc116c0 that should probably affect the package though but I have to admit I'm not particularly familiar with NixOS packages so I'm not sure what should be changed. @jtojnar sorry to bother you could you take a look?

@jtojnar
Copy link
Member

jtojnar commented Mar 24, 2024

Note that an alternate option is to use the 0.9-rc1 release candidate, but I don't know what the policy for using pre-releases is.

Yeah, we do not typically use unstable versions unless cherry-picking patches becomes too much of a pain. And backporting such a significant version bumg to a stable NixOS version would require serious evaluation that I do not think is feasible with the resources we have.

There were some build system changes like avahi/avahi@9350e8e and avahi/avahi@bc116c0 that should probably affect the package though but I have to admit I'm not particularly familiar with NixOS packages so I'm not sure what should be changed.

The changes required to build from master are pretty minimal:

Details
--- a/pkgs/development/libraries/avahi/default.nix
+++ b/pkgs/development/libraries/avahi/default.nix
@@ -3,6 +3,7 @@
 , lib
 , stdenv
 , pkg-config
+, autoreconfHook
 , libdaemon
 , dbus
 , libpcap
@@ -10,6 +11,7 @@
 , gettext
 , glib
 , libiconv
+, systemdMinimal
 , libevent
 , nixosTests
 , gtk3Support ? false
@@ -25,67 +27,10 @@ stdenv.mkDerivation rec {
   pname = "avahi${lib.optionalString withLibdnssdCompat "-compat"}";
   version = "0.8";
 
-  src = fetchurl {
-    url = "https://github.com/lathiat/avahi/releases/download/v${version}/avahi-${version}.tar.gz";
-    sha256 = "1npdixwxxn3s9q1f365x9n9rc5xgfz39hxf23faqvlrklgbhj0q6";
-  };
+  src = ~/Projects/avahi;
 
   outputs = [ "out" "dev" "man" ];
 
-  patches = [
-    # CVE-2021-36217 / CVE-2021-3502
-    (fetchpatch {
-      name = "CVE-2021-3502.patch";
-      url = "https://github.com/lathiat/avahi/commit/9d31939e55280a733d930b15ac9e4dda4497680c.patch";
-      sha256 = "sha256-BXWmrLWUvDxKPoIPRFBpMS3T4gijRw0J+rndp6iDybU=";
-    })
-    # CVE-2021-3468
-    (fetchpatch {
-      name = "CVE-2021-3468.patch";
-      url = "https://github.com/lathiat/avahi/commit/447affe29991ee99c6b9732fc5f2c1048a611d3b.patch";
-      sha256 = "sha256-qWaCU1ZkCg2PmijNto7t8E3pYRN/36/9FrG8okd6Gu8=";
-    })
-    (fetchpatch {
-      name = "CVE-2023-1981.patch";
-      url = "https://github.com/lathiat/avahi/commit/a2696da2f2c50ac43b6c4903f72290d5c3fa9f6f.patch";
-      sha256 = "sha256-BEYFGCnQngp+OpiKIY/oaKygX7isAnxJpUPCUvg+efc=";
-    })
-    # CVE-2023-38470
-    # https://github.com/lathiat/avahi/pull/457 merged Sep 19
-    (fetchpatch {
-      name = "CVE-2023-38470.patch";
-      url = "https://github.com/lathiat/avahi/commit/94cb6489114636940ac683515417990b55b5d66c.patch";
-      sha256 = "sha256-Fanh9bvz+uknr5pAmltqijuUAZIG39JR2Lyq5zGKJ58=";
-    })
-    # CVE-2023-38473
-    # https://github.com/lathiat/avahi/pull/486 merged Oct 18
-    (fetchpatch {
-      name = "CVE-2023-38473.patch";
-      url = "https://github.com/lathiat/avahi/commit/b448c9f771bada14ae8de175695a9729f8646797.patch";
-      sha256 = "sha256-/ZVhsBkf70vjDWWG5KXxvGXIpLOZUXdRkn3413iSlnI=";
-    })
-    # CVE-2023-38472
-    # https://github.com/lathiat/avahi/pull/490 merged Oct 19
-    (fetchpatch {
-      name = "CVE-2023-38472.patch";
-      url = "https://github.com/lathiat/avahi/commit/b024ae5749f4aeba03478e6391687c3c9c8dee40.patch";
-      sha256 = "sha256-FjR8fmhevgdxR9JQ5iBLFXK0ILp2OZQ8Oo9IKjefCqk=";
-    })
-    # CVE-2023-38471
-    # https://github.com/lathiat/avahi/pull/494 merged Oct 24
-    (fetchpatch {
-      name = "CVE-2023-38471.patch";
-      url = "https://github.com/lathiat/avahi/commit/894f085f402e023a98cbb6f5a3d117bd88d93b09.patch";
-      sha256 = "sha256-4dG+5ZHDa+A4/CszYS8uXWlpmA89m7/jhbZ7rheMs7U=";
-    })
-    # https://github.com/lathiat/avahi/pull/499 merged Oct 25
-    # (but with the changes to '.github/workflows/smoke-tests.sh removed)
-    ./CVE-2023-38471-2.patch
-    # CVE-2023-38469
-    # https://github.com/lathiat/avahi/pull/500 merged Oct 25
-    # (but with the changes to '.github/workflows/smoke-tests.sh removed)
-    ./CVE-2023-38469.patch
-  ];
 
   depsBuildBuild = [
     pkg-config
@@ -93,6 +38,7 @@ stdenv.mkDerivation rec {
 
   nativeBuildInputs = [
     pkg-config
+    autoreconfHook
     gettext
     glib
   ];
@@ -103,6 +49,7 @@ stdenv.mkDerivation rec {
     glib
     expat
     libiconv
+    systemdMinimal
     libevent
   ] ++ lib.optionals stdenv.isFreeBSD [
     libpcap
@@ -121,6 +68,9 @@ stdenv.mkDerivation rec {
   configureFlags = [
     "--disable-gdbm"
     "--disable-mono"
+    # We do not have xmltoman packaged
+    "--disable-xmltoman"
+    "--disable-manpages"
     # Use non-deprecated path https://github.com/lathiat/avahi/pull/376
     "--with-dbus-sys=${placeholder "out"}/share/dbus-1/system.d"
     (lib.enableFeature gtk3Support "gtk3")
@@ -142,7 +92,7 @@ stdenv.mkDerivation rec {
   installFlags = [
     # Override directories to install into the package.
     # Replace with runstatedir once is merged https://github.com/lathiat/avahi/pull/377
-    "avahi_runtime_dir=${placeholder "out"}/run"
+    "runstatedir=${placeholder "out"}/run"
     "sysconfdir=${placeholder "out"}/etc"
   ];
 

@jtojnar
Copy link
Member

jtojnar commented Mar 24, 2024

Thanks for this PR, looks great.

Could also please also apply the other patches mentioned in #269599 (comment) while at it?

I requested auto-backport of the previous PR: #298573

@evverx
Copy link

evverx commented Mar 24, 2024

we do not typically use unstable versions unless cherry-picking patches becomes too much of a pain. And backporting such a significant version bumg to a stable NixOS version would require serious evaluation that I do not think is feasible with the resources we have

Got it. Thanks!

In terms of evaluation relatively recently I set up the CI upstream where various things are tested with all sorts of tools. It even compiles on 32-bit and big endian machines there. It hasn't been fully set up yet (avahi/avahi#540) and I have to run some things outside of the avahi repository but hopefully it should help to get to the point where it's safe to pull the master branch and roll it out.

(It's all a bit chaotic with moving avahi to another organization, setting things up and so on. Sorry about that!)

…tenance

And grab only the single required commit in case of avahi/avahi#499.
This applies the fix for avahi/avahi#212 where having a single invalid
service being published inside a network could DoS discovery for all
avahi clients.

For me this happened with a "SIEMENS HM676G0S6". AFAIK Bosch (from the
original GitHub issue) is a subsidiary of Siemens.

Fixes: avahi/avahi#212
@muggenhor muggenhor force-pushed the fix/avahi-handle-bogus-services-gracefully branch from 961a2c6 to 8ea6519 Compare March 25, 2024 10:35
Specifically these where recommended by an upstream maintainer in
[this comment]:

* avahi/avahi#480
* avahi/avahi#515
* avahi/avahi#519

[this comment]: NixOS#269599 (comment)
@muggenhor muggenhor force-pushed the fix/avahi-handle-bogus-services-gracefully branch from 8ea6519 to a4e8e24 Compare March 25, 2024 10:39
@jtojnar jtojnar merged commit 02d746d into NixOS:staging Mar 25, 2024
22 of 23 checks passed
@jtojnar
Copy link
Member

jtojnar commented Mar 25, 2024

Great, thanks.

Copy link
Contributor

Successfully created backport PR for staging-23.11:

@muggenhor
Copy link
Contributor Author

Thanks for your review and merging!

@muggenhor muggenhor deleted the fix/avahi-handle-bogus-services-gracefully branch March 25, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants