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

nixos/dnscrypt-wrapper: fix rotate script failing to restart the service #33444

Merged
merged 1 commit into from Jan 14, 2018

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jan 5, 2018

Motivation for this change

Sorry it took me a while to notice this.
There is an issue with ca54a86: when the script runs as a normal user systemctl restart dnscrypt-wrapper fails with "Interactive authentication required." so dnscrypt-wrapper it's never restarted.
There a two possible solutions I can think of: running it as root (as it was before) and fixing the permission after or adding a polkit rule. I'd prefer not running as root since it's not really needed.

Things done
  • Tested the rotate script in a VM

cc @makefu

@makefu
Copy link
Contributor

makefu commented Jan 5, 2018

@rnhmjoj you are right, i just checked my logs and it seems the key was rotated correctly but the service was never restarted.
I will try to test the PR.

@vcunat
Copy link
Member

vcunat commented Jan 7, 2018

Are you aware of #33540?

@makefu
Copy link
Contributor

makefu commented Jan 7, 2018

@vcunat well, as of right now there is no real alternative as unbound+dns-tls does not validate the source.

@vcunat
Copy link
Member

vcunat commented Jan 7, 2018

I'm not sure about the possibilities in Unbound; Knot-resolver has a few ways of validating, but this thread isn't a good place and it's not released at this moment yet.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 14, 2018

So?

@makefu
Copy link
Contributor

makefu commented Jan 14, 2018

@rnhmjoj worked for me! The #33540 issue was resolved by another party is taking over maintance of dnscrypt-proxy so i assume there is no need to remove it.

@joachifm joachifm merged commit b6c696c into NixOS:master Jan 14, 2018
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 14, 2018

@joachifm Can you backport to 17.09?

@joachifm
Copy link
Contributor

@rnhmjoj done

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 15, 2018

Thank you

@rnhmjoj rnhmjoj deleted the dnscrypt-wrapper branch January 22, 2018 19:05
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

5 participants