-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
nixos/espanso: fix wayland problems due to missing capabilities #328890
base: master
Are you sure you want to change the base?
Changes from all commits
16ad84c
08c7064
6ebf110
4b56fd0
4e3840d
a8ac9e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
{ | ||
config, | ||
lib, | ||
pkgs, | ||
... | ||
}: | ||
|
||
let | ||
cfg = config.programs.espanso.capdacoverride; | ||
in | ||
{ | ||
meta = { | ||
maintainers = with lib.maintainers; [ pitkling ]; | ||
}; | ||
|
||
options = { | ||
programs.espanso.capdacoverride = { | ||
enable = (lib.mkEnableOption "espanso-wayland overlay with DAC_OVERRIDE capability") // { | ||
description = '' | ||
Creates an espanso binary with the DAC_OVERRIDE capability (via `security.wrappers`) and overlays `pkgs.espanso-wayland` such that self-forks call the capability-enabled binary. | ||
Required for `pkgs.espanso-wayland` to work correctly if not run with root privileges. | ||
''; | ||
}; | ||
|
||
package = lib.mkOption { | ||
type = lib.types.package // { | ||
check = package: lib.types.package.check package && (builtins.elem "wayland" package.buildFeatures); | ||
description = | ||
lib.types.package.description | ||
+ " for espanso with wayland support (`package.builtFeatures` must contain `\"wayland\"`)"; | ||
}; | ||
default = pkgs._espanso-wayland-orig; | ||
defaultText = "pkgs.espanso-wayland (before applying the overlay)"; | ||
description = "The espanso-wayland package used as the base to generate the capability-enabled package."; | ||
}; | ||
}; | ||
}; | ||
|
||
config = lib.mkIf cfg.enable { | ||
nixpkgs.overlays = [ | ||
(final: prev: { | ||
_espanso-wayland-orig = prev.espanso-wayland; | ||
espanso-wayland = pkgs.callPackage ./espanso-capdacoverride.nix { | ||
capDacOverrideWrapperDir = "${config.security.wrapperDir}"; | ||
espanso = cfg.package; | ||
}; | ||
}) | ||
]; | ||
|
||
security.wrappers."espanso-wayland" = { | ||
source = lib.getExe pkgs.espanso-wayland; | ||
capabilities = "cap_dac_override+p"; | ||
owner = "root"; | ||
group = "root"; | ||
}; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
{ | ||
autoPatchelfHook, | ||
capDacOverrideWrapperDir, | ||
espanso, | ||
patchelfUnstable, # have to use patchelfUnstable to support --rename-dynamic-symbols | ||
stdenv, | ||
}: | ||
let | ||
inherit (espanso) version; | ||
pname = "${espanso.pname}-capdacoverride"; | ||
|
||
wrapperLibName = "wrapper-lib.so"; | ||
|
||
# On Wayland, Espanso requires the DAC_OVERRIDE capability. One can create a wrapper binary with this | ||
# capability using the `config.security.wrappers.<name>` framework. However, this is not enough: the | ||
# capability is required by a worker process of Espanso created by forking `/proc/self/exe`, which points | ||
# to the executable **without** the DAC_OVERRIDE capability. Thus, we inject a wrapper library into Espanso | ||
# that redirects requests to `/proc/self/exe` to the binary with the proper capabilities. | ||
wrapperLib = stdenv.mkDerivation { | ||
name = "${pname}-${version}-wrapper-lib"; | ||
|
||
dontUnpack = true; | ||
|
||
postPatch = '' | ||
substitute ${./wrapper-lib.c} lib.c --subst-var-by to "${capDacOverrideWrapperDir}/espanso-wayland" | ||
''; | ||
|
||
buildPhase = '' | ||
runHook preBuild | ||
cc -fPIC -shared lib.c -o ${wrapperLibName} | ||
pitkling marked this conversation as resolved.
Show resolved
Hide resolved
|
||
runHook postBuild | ||
''; | ||
|
||
installPhase = '' | ||
runHook preInstall | ||
install -D -t $out/lib ${wrapperLibName} | ||
runHook postInstall | ||
''; | ||
}; | ||
in | ||
espanso.overrideAttrs (previousAttrs: { | ||
inherit pname; | ||
|
||
buildInputs = previousAttrs.buildInputs ++ [ wrapperLib ]; | ||
|
||
nativeBuildInputs = previousAttrs.nativeBuildInputs ++ [ | ||
autoPatchelfHook | ||
patchelfUnstable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need unstable here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we do. The wrapper library works by renaming the |
||
]; | ||
|
||
postInstall = | ||
'' | ||
echo readlink readlink_wrapper > readlink_name_map | ||
patchelf \ | ||
--rename-dynamic-symbols readlink_name_map \ | ||
--add-needed ${wrapperLibName} \ | ||
"$out/bin/espanso" | ||
'' | ||
+ previousAttrs.postInstall; | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
#include <stdio.h> | ||
#include <string.h> | ||
#include <unistd.h> | ||
|
||
static const char from[] = "/proc/self/exe"; | ||
static const char to[] = "@to@"; | ||
|
||
ssize_t readlink_wrapper(const char *restrict path, char *restrict buf, size_t bufsize) { | ||
if (strcmp(path, from) == 0) { | ||
printf("readlink_wrapper.c: Resolving readlink call to '%s' to '%s'\n", from, to); | ||
size_t to_length = strlen(to); | ||
if (to_length > bufsize) { | ||
to_length = bufsize; | ||
} | ||
memcpy(buf, to, to_length); | ||
return to_length; | ||
} else { | ||
return readlink(path, buf, bufsize); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also just patch the code to point to our correct path which IMO sounds a lot less troublesome than all of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, that's an alternative I thought about in the beginning. My main motivation for going this route was that it seems lower maintenance wrt upstream changes. With the wrapper library, no matter which of the Espanso subcommands perform a fork (now or sometime in the future), it is always caught and redirected. Patching the code would require manual updates.
I should also mention that, ideally, such an automatic fork redirect or something similar would be handled internally by
security.wrappers
. The reason that's currently not working has to do with the way how Espanso yields the capabilities early and relies on file capabilities to regain them when required (see this comment).