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.

Sign up for GitHub

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

stunnel service: allow specifying listening ip #99003

Merged
merged 2 commits into from Nov 4, 2020
Merged

stunnel service: allow specifying listening ip #99003

merged 2 commits into from Nov 4, 2020

Conversation

martinetd
Copy link
Member

@martinetd martinetd commented Sep 28, 2020 •
edited

Motivation for this change
  • server side accept in the config syntax is actually [host:]port, it is necessary to specify the listening ip for e.g. ipv6 where one would set :::port as a string.
  • examples incorrectly had 'enable' set, the option is not defined and reproducing would error out
Things done
  • 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.

@martinetd martinetd changed the title stunnel service: fix servers example stunnel service: allow specifying listening ip Sep 28, 2020
@martinetd
Copy link
Member Author

sneaked in an actual change, so updated title/original message and Ccing maintainer: Cc @lschuermann

thanks!

@lschuermann lschuermann self-requested a review October 10, 2020 09:10
@lschuermann
Copy link
Member

Thanks for the contribution, sorry for the delay! The change looks fine to me, works and is in line with the documentation:

accept = [HOST:]PORT
   accept connections on specified address
   If no host specified, defaults to all IPv4 addresses for the local host.
   To listen on all IPv6 addresses use:
       accept = :::PORT

Maybe we want to also document this behavior? It's quite unconventional to not put IPv6 addresses in square brackets (which will break). Otherwise, this looks fine.

@martinetd
stunnel service: fix servers example
05eef80 
examples incorrectly had 'enable' set, the option is not defined
and reproducing would error out
@martinetd
Copy link
Member Author

Maybe we want to also document this behavior? It's quite unconventional to not put IPv6 addresses in square brackets (which will break). Otherwise, this looks fine.

Good point, I've updated the description to the following (and rebased to master while I was at it)

On which [host:]port stunnel should listen for incoming TLS connections.
Note that unlike other softwares stunnel ipv6 address need no brackets,
so to listen on all IPv6 addresses on port 1234 one would use ':::1234'.

Thanks!

@martinetd
stunnel: make servers accept more lenient
1fb2990 
stunnel config's accept syntax is [host:]port -- this is required to e.g. listen on ipv6
where one would set :::port
@lschuermann lschuermann self-requested a review November 2, 2020 09:52
martinetd
martinetd commented Nov 2, 2020
View reviewed changes
Copy link
Member Author

@martinetd martinetd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Sorry, hadn't noticed because it looked good here in vim -- you'd think by 2020 editors could automatically expand tabs based on context... :/
Force pushed what amounts to your suggestion, thanks for insisting!

@lschuermann
Copy link
Member

Pinging @JohnAZoidberg as a maintainer.

JohnAZoidberg
JohnAZoidberg approved these changes Nov 4, 2020
View reviewed changes
Copy link
Member

@JohnAZoidberg JohnAZoidberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

you'd think by 2020 editors could automatically expand tabs based on context... :/

We don't use tabs at all ;)

You can use :set expandtab to expand the tabs always to the same amount.

@JohnAZoidberg JohnAZoidberg merged commit d4905b1 into NixOS:master Nov 4, 2020
@martinetd
Copy link
Member Author

We don't use tabs at all ;)

You can use :set expandtab to expand the tabs always to the same amount.

I know, I really need to take 5 minutes to properly add per project/directory settings... :)

Anyway, thanks for merging!

@martinetd martinetd deleted the stunnel-doc branch March 14, 2022 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JohnAZoidberg JohnAZoidberg JohnAZoidberg approved these changes

@lschuermann lschuermann lschuermann approved these changes

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@martinetd @lschuermann @JohnAZoidberg