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

Fix alignment when listing jails with more than one IP address #691

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

vrachnis
Copy link
Contributor

When a VNET jail has more than IP address configured on its primary interface, invoking bastille list -a will now display all addresses vertically aligned. This is to address a misalignment issue where the fields following the ip addresses were no longer in the same line as the jail name.

For instance:

 JID     State  IP Address       Published Ports  Hostname  Release          Path
 foo     Up     10.10.10.10
10.10.10.11   -                foo              14.0-RELEASE-p5  /usr/local/bastille/jails/foo/root
 hello1  Up     10.10.10.13      -                hello1                 14.0-RELEASE-p5  /usr/local/bastille/jails/hello1/root
 hello   Up     10.10.10.12      -                hello                14.0-RELEASE-p5  /usr/local/bastille/jails/hello/root
 test    Up     10.10.10.14      -                test               14.0-RELEASE-p5  /usr/local/bastille/jails/test/root

With this change, all additional IPs for a given jail are aligned vertically, and the fields following the IP (ports, hostname, release, path) are all in the same line as the jail name:

 JID     State  IP Address       Published Ports  Hostname  Release          Path
 foo     Up     10.10.10.10      -                foo              14.0-RELEASE-p5  /usr/local/bastille/jails/foo/root
                10.10.10.11
 hello1  Up     10.10.10.13      -                hello1                 14.0-RELEASE-p5  /usr/local/bastille/jails/hello1/root
 hello   Up     10.10.10.12      -                hello                14.0-RELEASE-p5  /usr/local/bastille/jails/hello/root
 test    Up     10.10.10.14      -                test               14.0-RELEASE-p5  /usr/local/bastille/jails/test/root

Considerations

Grouping of IPs

Initially I attempted to use line-drawing characters to visualize the fact that all addresses belong to the same jail:

 JID     State  IP Address       Published Ports  Hostname  Release          Path
 foo     Up     ┬ 10.10.10.10    -                foo              14.0-RELEASE-p5  /usr/local/bastille/jails/foo/root
                └ 10.10.10.11
 hello1  Up     10.10.10.13      -                hello1                 14.0-RELEASE-p5  /usr/local/bastille/jails/hello1/root
 hello   Up     10.10.10.12      -                hello                14.0-RELEASE-p5  /usr/local/bastille/jails/hello/root
 test    Up     10.10.10.14      -                test               14.0-RELEASE-p5  /usr/local/bastille/jails/test/root

While the result was making it clear to the user where the extra line comes from, the implementation became unnecessarily complex. More importantly, it meant that if anyone parses the output of bastille list -a in their script, would have to account for the fact that the "primary" IP address would be in either the third or fourth field of the line.

This highlights the fact that with this, the first and second/third/etc addresses will still be on different fields. The first address will be on field number 3, while the following addresses will be on field number 1. I could potentially modify the output to put some dummy characters in the first two columns, although I'm not sure that I like the result. Let me know if something like the following is preferable.

 JID     State  IP Address       Published Ports  Hostname  Release          Path
 foo     Up     10.10.10.10      -                foo              14.0-RELEASE-p5  /usr/local/bastille/jails/foo/root
 └─────  ─────  10.10.10.11
 hello1  Up     10.10.10.13      -                hello1                 14.0-RELEASE-p5  /usr/local/bastille/jails/hello1/root
 hello   Up     10.10.10.12      -                hello                14.0-RELEASE-p5  /usr/local/bastille/jails/hello/root
 test    Up     10.10.10.14      -                test               14.0-RELEASE-p5  /usr/local/bastille/jails/test/root

Dependencies

No new dependencies were introduced.

When a VNET jail has more than IP address configured on its primary interface, invoking "bastille list -a" will now display all addresses vertically aligned.
@yaazkal yaazkal added the enhancement New feature or request label Jul 8, 2024
@yaazkal
Copy link
Collaborator

yaazkal commented Jul 14, 2024

Thanks, I personally prefer the space instead of the extra characters for the first two columns. Now, does this support printing the published ports corresponding to each IP? does this support different hostname per IP?

Regards

@bmac2
Copy link
Collaborator

bmac2 commented Dec 17, 2024

two questions. How do you add a second ip to a jail?? I want to test this one. AND the code here is just the spaces and not the extra characters,right? @vrachnis @yaazkal

@bmac2
Copy link
Collaborator

bmac2 commented Dec 26, 2024

@vrachnis I really need the steps you went through to add a second ip to a vnet jail. I have googled, read the jail.conf manual and can not figure out how to set a second ip to a vnet jail.

@bmac2
Copy link
Collaborator

bmac2 commented Dec 26, 2024

when I run your list with a normal jail with multiple ip
addresses, it does not look like yours. Here is what I see:

root@fbsd:/usr/local/share/bastille # bastille list -a
JID State IP Address Published Ports Hostname Release Path
jail1 Up 192.168.1.155 - jail1 14.1-RELEASE
jail2 Down 192.168.1.156 - jail2 14.1-RELEASE
jail3 Up 192.168.1.152,10.0.0.152 - jail3 14.1-RELEASE
jail4 Up 192.168.1.159 - jail4 14.1-RELEASE
jail5 Down 192.168.1.160 - jail5 14.1-RELEASE
root@fbsd:/usr/local/share/bastille #

@vrachnis
Copy link
Contributor Author

apologies for missing the previous comments here. I just found the notification emails.

I [...] need the steps you went through to add a second ip to a vnet jail

I've added the second IPs using rc.conf, and specifically using the config from carp(4):

ifconfig_e2b_test_name="vnet0"
ifconfig_vnet0="inet 192.168.1.32/24"
ifconfig_vnet0_alias0="inet vhid 1 advskew 100 pass supersecretpass peer 192.168.1.33 alias 192.168.1.34/24"

That said, I just tried and I was able to create a jail with standard aliases (and no carp):

ifconfig_e0b_test1_name="vnet0"
ifconfig_vnet0=" inet 192.168.1.99/24 "
ifconfig_vnet0_alias0=" inet 192.168.1.100/24 "

This generates the following output (using the patch from this PR):

root@rpi4-01:~ # bastille list -a
 JID     State  IP Address  Published Ports  Hostname  Release          Path
 test1   Up     192.168.1.99  -                test1     14.1-RELEASE-p5  /usr/local/bastille/jails/test1/root
                192.168.1.100

Note that those are only applicable to vnet jails. For standard jails, the IPs are defined in jail.conf using commas as you said. I could also port this multiline behaviour to non-vnet output if you'd like me to.

the code here is just the spaces and not the extra characters,right?

Correct. I didn't like the complexity of the logic if I tried to use the extra bracket characters, so I didn't submit that. The only reason I mentioned it is that if the reviewers preferred it, I could bring that in.

does this support printing the published ports corresponding to each IP?

That's a very good question. I've never used bastille rdr before, because I've always run my jails using vnet and bridges. Looking at rdr.sh, I'm not sure that it works with vnet at all considering that the value of JAIL_IP is not populated when vnet is enabled. I'll test that and comment again.

does this support different hostname per IP?

Not really. The hostname logic is still the same; that is to say list.sh pulls the host.hostname value from jail.conf.

@vrachnis
Copy link
Contributor Author

Regarding the rdr elements, I think it's not meant to work with vnet jails using the current implementation. Trying to add a new redirection gives me an error:

# bastille rdr test1 tcp 2222 22 log
pfctl: DIOCGETRULES: Invalid argument
stdin:2: syntax error
pfctl: Syntax error in config file: pf rules not loaded

If I try to see the pf rule that it generated, I get this:

# sh -ex /usr/local/share/bastille/rdr.sh test1 tcp 2222 22 log
[...]
+ grep -qs 'tcp 2222 22 log' /usr/local/bastille/jails/test1/rdr.conf
+ load_rdr_log_rule tcp 2222 22 log
+ proto=tcp
+ host_port=2222
+ jail_port=22
+ shift 3
+ log=log
+ pfctl -a rdr/test1 -Psn
+ pfctl -a rdr/test1 -f-
pfctl: DIOCGETRULES: Invalid argument
+ printf '%s\nrdr pass %s on $%s inet proto %s to port %s -> %s port %s\n' 'ext_if=genet0' log ext_if tcp 2222 '' 22
stdin:2: syntax error
pfctl: Syntax error in config file: pf rules not loaded

With the key line being:

printf '%s\nrdr pass %s on $%s inet proto %s to port %s -> %s port %s\n' 'ext_if=genet0' log ext_if tcp 2222 '' 22

This generates rdr pass log on $ext_if inet proto tcp to port 2222 -> port 22 as the rule.

Looking at pf.conf(5), it seems that the -> $IP part of the rule is not optional:

     rdr-rule       = [ "no" ] "rdr" [ "pass" [ "log" [ "(" logopts ")" ] ] ]
                      [ "on" ifspec ] [ af ]
                      [ protospec ] hosts [ "tag" string ] [ "tagged" string ]
                      [ "->" ( redirhost | "{" redirhost-list "}" )
                      [ portspec ] [ pooltype ] ]
[...]
     redirhost      = address [ "/" mask-bits ]

I believe that because of this VNET check, we end up having an empty JAIL_IP, which then goes on to break this and this rule.

So coming back to the original question:

does this support printing the published ports corresponding to each IP?

No, it does not. That's because rdr doesn't work for vnet jails, and this patch only affects the output when vnet is enabled. When vnet is disabled, the IP list is pulled from jail.cof. As long as there are no new lines in the IP list, the new logic is disabled and uses the existing output format

Thinking out loud (because I don't have a v6 set of addresses from my ISP to test with), the JAIL_IP variable could still be multiline for non-vnet jails if IPv6 and IPv4 addresses are assigned at the same time. That would trigger the if statement to use the new logic, and that wouldn't show the published ports any differently than what we already do. However, considering that rdr.sh adds the same rules for the v4 and the v6 address, that is (probably) not a bad thing and better than having new line characters in the middle of the IP list for the jail?

@bmac2
Copy link
Collaborator

bmac2 commented Dec 27, 2024

tested with vnet jails with multiple ip addresses, works as advertised.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants