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

Jump to bottom

nixos/dnscrypt-proxy2: more service hardening #108238

Merged
merged 1 commit into from Feb 8, 2021
Merged

nixos/dnscrypt-proxy2: more service hardening #108238

merged 1 commit into from Feb 8, 2021

Conversation

snicket2100
Copy link
Contributor

Motivation for this change

Increasing system security by adding more hardening hardening the dnscrypt-proxy2 service.

Things done

Added 'ProtectClock' and made the seccomp filter a bit more restrictive.

I have been running it with these settings for a while with zero problems.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@snicket2100
nixos/dnscrypt-proxy2: more service hardening
2bab1a7 
added 'ProtectClock' and made the seccomp filter a bit more restrictive
have been running with these settings for a while with zero problems
@symphorien symphorien merged commit 2630a2d into NixOS:master Feb 8, 2021
@evrim
Copy link
Contributor

evrim commented Feb 13, 2021

Great patch, I am glad that everybody who uses dnscrypt-proxy2 would appreciate this one, especially the ~@sync part. It is a wonder how well it works. Perfect!

@ju1m
Copy link
Contributor

ju1m commented Feb 21, 2021 •
edited

AFAIU @sync is required when setting a (working) cache_file :

sources.public-resolvers = { 
  urls = [ "https://raw.githubusercontent.com/DNSCrypt/dnscrypt-resolvers/master/v3/public-resolvers.md" ];  
  minisign_key = "RWQf6LRCGA9i53mlYecO4IzT51TGPpvWucNSCh1CBM0QTaLn73Y7GFO3";
  cache_file = "/var/lib/dnscrypt-proxy/public-resolvers.md";
};

By default cache_file points to the read-only /nix/store/ effectively disabling the cache.
But when it is set at a writable path, dnscrypt-proxy2.service is killed at startup (likely leaving the system without any working DNS):

dnscrypt-proxy2.service: Main process exited, code=killed, status=31/SYS

Ensuing coredump confirms ~@sync is causing the SIGSYS:

$ sudo coredumpctl debug 4495
[...]
(gdb) bt 2
#0  0x00000000004ba2fb in syscall.Syscall ()
#1  0x00000000004b77c5 in syscall.Fsync ()

@symphorien
Copy link
Member

Can you open a PR removing the sync part then?

@ju1m ju1m mentioned this pull request Feb 21, 2021
Merged
11 tasks
@ju1m
Copy link
Contributor

ju1m commented Feb 21, 2021

@symphorien, done here: #113904

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emantor Emantor Emantor approved these changes

@symphorien symphorien Awaiting requested review from symphorien

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@snicket2100 @evrim @ju1m @symphorien @Emantor