OpenDoas icon indicating copy to clipboard operation
OpenDoas copied to clipboard

Add support for including configuration from multiple files

Open jirutka opened this issue 3 years ago • 17 comments

Can you please add support for including rules from files in a directory (e.g. /etc/doas.d/*)? This is basically essential feature for using doas as a sudo replacement in packages.

jirutka avatar Mar 13 '21 23:03 jirutka

I definitely like snippet-style configuration when you're dealing with a complex configuration, but doas's is dead simple. Mine's literally one line long. I guess my question would be what the specific use case you're needing here: are you thinking of having packages drop in files granting wheel permission to use them, or what? I just don't see what difference it makes how the user goes about configuring it in this case.

escondida avatar Mar 14 '21 02:03 escondida

but doas's is dead simple. Mine's literally one line long.

That doesn’t matter. The point is to be able to add/remove rules just by adding/removing files.

are you thinking of having packages drop in files granting wheel permission to use them, or what?

Yes; for example: repmgr, nagios-openrc.

jirutka avatar Mar 14 '21 13:03 jirutka

This would be nice to have. I can write a patch for it if it would be helpful.

kaniini avatar Jul 23 '21 10:07 kaniini

Also chiming in with @jirutka : when you are talking big infrastructures and you manage everything with a configuration management system, editing files is a no-go. Adding/removing files OTOH is dead simple. Kind of fitting to doas, doesn't it?

telmich avatar Jul 23 '21 11:07 telmich

Yes, that would be awesome!

Adding support for “include” into OpenDoas to allow modular configuration as sudo does, so package or configuration management can add/remove rules simply by adding/removing a file into/from /etc/doas.d/, would allow me to finally replace sudo with doas.

jirutka avatar Jul 23 '21 11:07 jirutka

@telmich, yeah, but not just big infrastructure, I would say every infrastructure. BTW, I use Git + CI + APK packages as a configuration management system (apk-deploy-tool), so those two use cases I described are actually the same for me, that’s why I forgot to mention CM before.

jirutka avatar Jul 23 '21 11:07 jirutka

@duncaen the Alpine TSC has voted to replace sudo with opendoas. I can send a patch to implement /etc/doas.d support, or we can use an update-doas-conf wrapper in Alpine to handle our needs. Which would you prefer?

kaniini avatar Aug 03 '21 16:08 kaniini

I'm not sure, what do you prefer?

My concerns are mostly about diverting from upstream, which I tried to avoid.

There are some implementation details that need to be considered, like that the last applicable rule is the rule that makes the final decision, this could get a bit confusing over multiple files. I don't think this is a problem but needs to be considered when writing rules. The configuration checking mechanism (-C flag) currently also works with one file which would require a breaking change or simply would not work as expected when there are more rules loaded later from a separate file in normal operation.

Duncaen avatar Aug 03 '21 17:08 Duncaen

Based on your point with -C, I think layering configuration management on top makes more sense.

kaniini avatar Aug 03 '21 17:08 kaniini

I guess we could get away without a breaking change to -C, if we would say configuration can be either a single /etc/doas.conf file or if that does not exists the /etc/doas.d directory, then the -C flag would just check against either the supplied file or the supplied directory, without requiring a separate flag to change both paths at once and without having to define if /etc/doas.conf is loaded before or after /etc/doas.d.

Duncaen avatar Aug 03 '21 17:08 Duncaen

I'm not sure, what do you prefer?

I’d definitely prefer implementing it in doas. Using a script to generate merged config is just a poor workaround I’d prefer to avoid, especially for such security sensitive thing.

jirutka avatar Aug 03 '21 18:08 jirutka

I would rather keep it simple. The question of what -C does in this context is one to think about carefully. Since multiple snippets are allowed, and the order is non-deterministic, whether or not a given snippet is "valid" is hard to determine.

The more I think about it, configuration management is really the way to go here.

kaniini avatar Aug 03 '21 18:08 kaniini

This is not some novel and unexplored feature; many software implements it, including sudo. And the typical solution is very simple – just iterate the files in the /etc/doas.d directory in alphabetical order (C ordering). IIRC there’s a function in libc that returns list of directory entries in this very order.

BTW, you would have the same issue with update-doas-conf – decide the order of the files.

jirutka avatar Aug 03 '21 18:08 jirutka

Obviously the order of parsing can be made deterministic. The problem is that -C takes a path to a config file.

Say you want to do doas -C /etc/doas.d/foo.conf, but it needs /etc/doas.d/bar.conf to be loaded first in order to be valid.

The doas -C check will fail, even though in practice the configuration will be fine. The configuration management approach proposed by Natanael does not have that problem as far as doas is concerned, since it would only deal with /etc/doas.conf.

kaniini avatar Aug 03 '21 19:08 kaniini

Well, that’s easy, just don’t support doas -C /etc/doas.d/foo.conf, but only doas -C /etc/doas.conf which will automatically validate also /etc/doas.d/*, i.e. it will behave exactly the same as running doas in normal mode.

jirutka avatar Aug 03 '21 19:08 jirutka

BTW, If we use update-doas-conf approach (I still hope not), then it would be correct to move the generated doas.conf into /usr/lib/doas/ or /lib/doas/ because it will not be a configuration file supposed to be edited by the users anymore, but a data file.

jirutka avatar Aug 03 '21 19:08 jirutka

I think @duncaen's proposal that -C checks either a file, or a directory tree is the best way to go. I'll get the PR done today.

kaniini avatar Aug 04 '21 09:08 kaniini