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

some quick/dirty support for subjails #1

Closed
wants to merge 1 commit into from

Conversation

egwynn
Copy link

@egwynn egwynn commented Jul 7, 2015

sshjail's underscore-mangling makes it fail to recognize subjail names (which get named like parentjailname.childjailname). This patch tells sshjail to first build a map of mangled -> unmangled jail names, and then replaces self.jname with the unmangled name if it can find it in the map.

@austinhyde
Copy link
Owner

Oh, nice find! I haven't had to work with subjails yet, so I didn't realize this wouldn't work. What it's currently doing is based on my company's usage with explicit hostnames and implicit jail names.

I'm curious if this is the best solution though - maybe instead of .sub(r'\W','_',jaildef) it does something more like .sub(r'[^\w\.]','_', jaildef):

>>> import re
>>> re.sub(r'\W', '_', 'parent-jail.child-jail')
'parent_jail_child_jail'
>>> re.sub(r'[^\w\.]', '_', 'parent-jail.child-jail')
'parent_jail.child_jail'

I'd be much happier if the jail name could be "statically" derived, rather than searching through the whole list of jails, although I suppose that's not so different than what it's doing now with regards to finding the jail path and jid.

As I don't have a setup on hand with some subjails, would you mind posting a couple lines from running jls name host.hostname on the topmost jailhost? Feel free anonymize it a little, I'm mostly just interested in seeing what the naming scheme is between names and hostnames on jails vs subjails. And did you leave one or the other implicit? or did you explicitly define everything?

I would like to get this working for you and merged in though!

@egwynn
Copy link
Author

egwynn commented Jul 8, 2015

Your proposed regex would address the issue for my jails. I have a bunch of top-level jails, and then my web jail has subjails (this makes it easy to have nginx serve static content from within the jails instead of nullfs-ing myself to death). Here are a few of my jails, including subjails.

$ jls name host.hostname
mail_egwynn_com mail.egwynn.com
mysqld_egwynn_com mysqld.egwynn.com
nginx_egwynn_com nginx.egwynn.com
nginx_egwynn_com.projectsnow projectsnow
nginx_egwynn_com.wp_mb_blog wp_mb_blog
nginx_egwynn_com.dj_ag_jab dj_ag_jab
nginx_egwynn_com.dj_pretrial dj_pretrial

If you are interested in trying this yourself, you may need to set jail_<subjailname>_flags="-n <subjailname>" in order to for your subjail to get a proper name. In my case, I was seeing the system just use jid if I didn't set that flag.

EDIT: Oh, and yeah, I set the names/hostnames for all of my jails explicitly, but as you can see, I'm not giving the inner ones fully-qualified hostnames for now. Maybe I will later though.

@austinhyde
Copy link
Owner

Hmm, on second thought, I'm not sure my proposed regex is a better fix after all. If someone were to attempt to use the hostname in the host specification (either jail or subjail), like [email protected], then it would look for jail name mail.egwynn.com.

I would prefer to be able use the hostname as a valid specification, because in my case, the hostname is what we primarily use to identify and refer to jails, rather than the jail name. At the same time, it makes perfect sense that you would want to use the jail name to specify the name.

What if I take your idea of matching the jail from the whole list, but instead get rid of underscore mangling entirely, and first attempt to match on jail name, then hostname, then fail? If I whip this up in a separate PR, and give you the opportunity to review and ensure it works for you, would that be acceptable?

@austinhyde austinhyde mentioned this pull request Jul 8, 2015
@austinhyde
Copy link
Owner

@egwynn I created PR #2 with my proposed solution. Could you review that when you get a chance, and see if that works for you?

@egwynn
Copy link
Author

egwynn commented Jul 8, 2015

Your changes in PR #2 look fine and work great for me. Thanks!

@egwynn egwynn closed this Jul 8, 2015
@austinhyde
Copy link
Owner

Great! That's merged in.

Thanks for bringing this up!

Let me know if you run into any more quirks/difficulties/etc

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

2 participants