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

shorewall: init at 5.2.3.3 #46131

Merged
merged 1 commit into from Dec 30, 2019
Merged

shorewall: init at 5.2.3.3 #46131

merged 1 commit into from Dec 30, 2019

Conversation

ju1m
Copy link
Contributor

@ju1m ju1m commented Sep 6, 2018 •
edited

Motivation for this change

The Shoreline Firewall is a great firewall based upon Linux's Netfilter: "Shorewall is not the easiest to use of the available iptables configuration tools but I believe that it is the most flexible and powerful."

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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)
    NOTE: only tested manually with a very basic setup.
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-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)
  • Fits CONTRIBUTING.md.
    FIXME: maintainers field not set so far.

FIXME

A lot of collision between happen between the share/shorewall from the shorewall package and the share/shorewall from the shorewall6 package.

collision between `/nix/store/hb223l2y13cqcrjp7a45xgs262j86ssb-shorewall-5.2.0.5/share/shorewall/macro.SVN' and `/nix/store/ihcd7r0wr3b4vk0n8cccn37d8l58cw21-shorewall6-5.2.0.5/share/shorewall/macro.SVN'
collision between `/nix/store/hb223l2y13cqcrjp7a45xgs262j86ssb-shorewall-5.2.0.5/share/shorewall/macro.Edonkey' and `/nix/store/ihcd7r0wr3b4vk0n8cccn37d8l58cw21-shorewall6-5.2.0.5/share/shorewall/macro.Edonkey' 
collision between `/nix/store/hb223l2y13cqcrjp7a45xgs262j86ssb-shorewall-5.2.0.5/share/shorewall/action.DNSAmp' and `/nix/store/ihcd7r0wr3b4vk0n8cccn37d8l58cw21-shorewall6-5.2.0.5/share/shorewall/action.DNSAmp'
collision between `/nix/store/hb223l2y13cqcrjp7a45xgs262j86ssb-shorewall-5.2.0.5/share/shorewall/action.A_REJECT' and `/nix/store/ihcd7r0wr3b4vk0n8cccn37d8l58cw21-shorewall6-5.2.0.5/share/shorewall/action.A_REJECT'
collision between `/nix/store/hb223l2y13cqcrjp7a45xgs262j86ssb-shorewall-5.2.0.5/share/shorewall/macro.Git' and `/nix/store/ihcd7r0wr3b4vk0n8cccn37d8l58cw21-shorewall6-5.2.0.5/share/shorewall/macro.Git'
.
.
.

No idea if this is that bad, nor how to fix this. Maybe there is a cleaner way to let shorewall reuse shorewall-core, and likewise to let shorewall6 reuse both of them. Currently the installPhases do cp -r -s ${shorewall-core} $out or cp -r -s ${shorewall}/share/shorewall $out/share/.

@matthewbauer
Copy link
Member

@GrahamcOfBorg build shorewall

@GrahamcOfBorg

This comment has been minimized.

Sign in to view
@GrahamcOfBorg

This comment has been minimized.

Sign in to view
@GrahamcOfBorg

This comment has been minimized.

Sign in to view
Mic92
Mic92 reviewed Nov 15, 2018
View reviewed changes
pkgs/top-level/all-packages.nix Outdated
shorewall = callPackage ../tools/networking/shorewall {
# For the `logger' command, we can use either `utillinux' or
# GNU Inetutils. The latter is more portable.
logger = if stdenv.isLinux then utillinux else inetutils;
Copy link
Member

Choose a reason for hiding this comment

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

Since this package depends on iptables it will not run on any other platform.

@ryantm
Copy link
Member

ryantm commented Mar 9, 2019

@GrahamcOfBorg build shorewall

ryantm
ryantm requested changes Mar 9, 2019
View reviewed changes
Copy link
Member

@ryantm ryantm left a comment

Choose a reason for hiding this comment

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

Address other comments, please.

@ju1m ju1m requested a review from infinisil as a code owner June 17, 2019 19:48
@infinisil
Copy link
Member

Why are there two versions for ipv4/ipv6? I'd think these can be combined into a single package

@ju1m
Copy link
Contributor Author

ju1m commented Jul 23, 2019

Why are there two versions for ipv4/ipv6? I'd think these can be combined into a single package
Just because it's done like this upstream and in Debian, so I tried to follow the same path. But merging the three packages (core, IPv4 and IPv6) would likely simplify the installation (eg. remove the cp -r -s hack).

@infinisil
Copy link
Member

Yeah unless there's a good reason I'd like to have just a single derivation for them. I can't make any sense of them being different

@ju1m ju1m changed the title shorewall: init at 5.2.0.5 shorewall: init at 5.2.3.3 Jul 26, 2019
@azazel75
Copy link
Contributor

Hi, nice work here! I would like to use this, what is keeping it back?

@ryantm
Copy link
Member

ryantm commented Dec 23, 2019

@GrahamcOfBorg build shorewall

@ryantm
shorewall: init at 5.2.3.3
56a73df 
nixos/shorewall: init
@ryantm
Copy link
Member

ryantm commented Dec 30, 2019

@GrahamcOfBorg build shorewall

@ryantm
Copy link
Member

ryantm commented Dec 30, 2019

I squashed the commits, fixed up a reference to IPv4 and changed it to use pname instead of baseName

@ryantm
Copy link
Member

ryantm commented Dec 30, 2019

aarch64-linux ofborg build was successful, I built locally on x86_64 NixOS and it won't need to build on darwin, so I'm going to merge this.

@ryantm ryantm merged commit 3a644e3 into NixOS:master Dec 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@JohnAZoidberg JohnAZoidberg JohnAZoidberg left review comments

@c0bw3b c0bw3b c0bw3b left review comments

@ryantm ryantm ryantm approved these changes

@infinisil infinisil Awaiting requested review from infinisil

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

Successfully merging this pull request may close these issues.

None yet
10 participants
@ju1m @matthewbauer @GrahamcOfBorg @ryantm @infinisil @azazel75 @Mic92 @JohnAZoidberg @c0bw3b @Janik-Haag