"-c" cmdline option ignored in several executables

[drawks]

It looks like several (all?) of the binaries generated when building this project have the same usage statement:

-V          Version and license message
-h          This help message
-d          Execute in debug mode
-t          Test configuration
-f          Run in foreground
-u <user>   Run as 'user'
-g <group>  Run as 'group'
-c <config> Read the 'config' file
-D <dir>    Chroot to 'dir'

But I've variously seen that the "-c" argument is ignored:

$ sudo /var/lib/ossec/bin/ossec-execd -d -t -c /another_location/ossec.conf 
  2014/05/15 13:24:34 ossec-execd(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

or even fully unimplemented:

$ sudo /var/lib/ossec/bin/ossec-agentd -d -t -c /another_location/ossec.conf 
    /var/lib/ossec/bin/ossec-agentd: invalid option -- 'c'
   2014/05/15 13:24:23 ossec-agentd: DEBUG: Starting ...
   2014/05/15 13:24:23 ossec-config(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

Combined with #24 it becomes virtually impossible to install this software to any location aside from the defaults from the "installer".

[awiddersheim]

I can confirm for atleast ossec-execd that the configuration option doesn't get used at all. Also, it looks like the -D <dir> Chroot to 'dir' has no use as well from what I can tell.

The configuration option in ossec-execd should be fixed and the chroot option should be removed. I guess in the interest in saving time/code they same --help output is used for every binary despite not every binary needing the same options. The reuse of code is kind of ironic when talking about OSSEC from what I've seen. Too bad it was done poorly. Anyway...

I haven't looked at ossec-agentd yet but one thing to be careful of is that some (not all) of the OSSEC binaries chroot themselves as the help indicates. This mean if you installed OSSEC to /var/lib/ossec but you try to read in a configuration file in /etc/ossec/ for example it won't and will never really work. The configuration would need to be placed in /var/lib/etc/ossec.conf and you'd specify something like /etc/ossec.conf when doing the -C option. That is unless you tell the binary to chroot itself elsewhere using the -D option but that will likely break things and things get crazy fast.

Atleast that is my understanding. Despite being able to specify different configuration locations it might not get you very far and your mileage may vary.

That said, even though the "installer" is pretty hokey you can still install to different locations. The issue presented in #24 only really affects some scripts in "contrib". Right now that issue only lists two scripts. Fixing those would require a simple sed. The rest should just work and paths aren't as "hard coded" as that issue might make them out to be.

The configuration/help might need some updating to better describe things here. I'll work on fixing things when time permits.

[mstarks01]

util.sh is fixed in my repo

[drawks]

@awiddersheim Thanks for the reply, I'm aware of the chroot stuff you mentioned, but thanks for pointing it out for maybe others who didn't realize. Unfortunately if the "-c" option doesn't work the path is effectively hardcoded. I've spent the past few days packaging ossec for my production environment and in all the installer/make machinery I don't see any way to change the DEFAULTDIR without manually passing additional arguments into the makefile like:

-DDEFAULTDIR=\"\/some\/other\/path\"

The bug in ossec-execd seems pretty obvious to me, but I can't submit back a fix without doing a whole legal code release dance; so hopefully someone else will see the error in execd.c and correct them :)

82    char *cfg = DEFAULTARPATH;
83    char *xmlcfg = DEFAULTCPATH;

snip

117                 cfg = optarg;

snip

142     /* Reading config */
143     if((c = ExecdConfig(xmlcfg)) < 0)

A cursory glance at main.c of ossec-agentd shows that there isn't even a "c" switch defined in the case statement for parsing args.

At any rate I think any reasonable person would consider it a bug for the usage statements to not match the actual available flags and/or advertised flag behaviors.

edit: I added and removed a complaint here based on a misread of code... fwiw

[awiddersheim]

I think we are on the same page about what is broken and what needs to get fixed. I've seen the issues in ossec-execd which shouldn't be difficult to fix. I'd imagine fixing ossec-agentd won't be that hard either but I haven't looked yet. I (or someone else) will hopefully get around to fixing the issues you brought up shortly.

Thanks for the feedback.

[ddpbsd]

On Fri, May 16, 2014 at 12:51 PM, Dave Rawks [email protected] wrote:

@awiddersheim Thanks for the reply, I'm aware of the chroot stuff you mentioned, but thanks for pointing it out for maybe others who didn't realize. Unfortunately if the "-c" option doesn't work the path is effectively hardcoded. I've spent the past few days packaging ossec for my production environment and in all the installer/make machinery I don't see any way to change the DEFAULTDIR without manually passing additional arguments into the makefile like:
-DDEFAULTDIR="/some/other/path"

The bug in ossec-execd seems pretty obvious to me, but I can't submit back a fix without doing a whole legal code release dance; so hopefully someone else will see the error on lines 82-83 of execd.c and correct them :)

It's not obvious to me. Setting the group and a comment?

A cursory glance at main.c of ossec-agentd shows that there isn't even a "c" switch defined in the case statement for parsing args. Additionally the "test config" flag for that program simply exits without doing anything....

123 /* Exit if test config */
124 if(test_config)
125 exit(0);

At any rate I think any reasonable person would consider it a bug for the usage statements to not match the actual available flags and/or advertised flag behaviors.

Reply to this email directly or view it on GitHub.

[ddpbsd]

On Thu, May 15, 2014 at 4:27 PM, Dave Rawks [email protected] wrote:

It looks like several (all?) of the binaries generated when building this project have the same usage statement:

-V Version and license message
-h This help message
-d Execute in debug mode
-t Test configuration
-f Run in foreground
-u Run as 'user'
-g Run as 'group'
-c Read the 'config' file
-D

Chroot to 'dir'

But I've variously seen that the "-c" argument is ignored:

$ sudo /var/lib/ossec/bin/ossec-execd -d -t -c /var/lib/ossec/etc/ossec.conf
2014/05/15 13:24:34 ossec-execd(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

I do not get this error when I use "-c /etc/ossec.conf"
I didn't check the code, but does the config get read before or after chroot?

or even fully unimplemented:

$ sudo /var/lib/ossec/bin/ossec-agentd -d -t -c /var/lib/ossec/etc/ossec.conf
/var/lib/ossec/bin/ossec-agentd: invalid option -- 'c'
2014/05/15 13:24:23 ossec-agentd: DEBUG: Starting ...
2014/05/15 13:24:23 ossec-config(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

Combined with #24 it becomes virtually impossible to install this software to any location aside from the defaults from the "installer".

Reply to this email directly or view it on GitHub.

[awiddersheim]

I don't think ossec-execd chroots. You have to look at his error carefully. It is kind of tricky about what is happening and I haven't spent too much time on it yet but I think I know what is happening.

$ sudo /var/lib/ossec/bin/ossec-execd -d -t -c /var/lib/ossec/etc/ossec.conf
   2014/05/15 13:24:34 ossec-execd(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

He is specifying /var/lib/ossec/etc/ossec.conf as the config file he wants to use on the command line but the error ossec-execd is coming back with is saying it can't read /var/ossec/etc/ossec.conf. Note the completely different location.

My guess is /var/ossec is where the installer put things and where it thinks ossec.conf should be but he's moved things.

I'd imagine if you removed/renamed the ossec.conf you would get the same/similar error @drawks is getting.

[ddpbsd]

On Fri, May 16, 2014 at 1:41 PM, awiddersheim [email protected] wrote:

I don't think ossec-execd chroots. You have to look at his error carefully. It is kind of tricky about what is happening and I haven't spent too much time on it yet but I think I know what is happening.

You're right, it doesn't.

$ sudo /var/lib/ossec/bin/ossec-execd -d -t -c /var/lib/ossec/etc/ossec.conf
2014/05/15 13:24:34 ossec-execd(1226): ERROR: Error reading XML file '/var/ossec/etc/ossec.conf': XMLERR: File '/var/ossec/etc/ossec.conf' not found. (line 0).

He is specifying /var/lib/ossec/etc/ossec.conf as the config file he wants to use on the command line but the error ossec-execd is coming with is saying it can't read /var/ossec/etc/ossec.conf. Not the completely different location.

Gotcha. Missed the silly situation. I'm still unable to recreate the
problem. I recompiled ossec to use a different location, and tried
pointing ossec-execd to a variety of ossec.confs (real and imaginary)
scattered throughout my /var. None gave me any errors.

My guess is /var/ossec is where the installer put things and where it thinks ossec.conf should be but he's moved things.

I'd imagine if you removed/renamed the ossec.conf you would get the same/similar error @drawks is getting.

Nope.

Reply to this email directly or view it on GitHub.

[awiddersheim]

Try removing/renaming your default ossec.conf and it should produce the error he is seeing. The -C option is actually completely ignored in this case. The error he is getting is actually the result of the default ossec.conf in the installation location being missing as well.

So for example if I installed OSSEC to /opt/ossec and ran ossec-execd with the -C option specifying a configuration file at /etc/ossec.conf nothing should happen.

If I would remove the ossec.conf in /opt/ossec/etc/ossec.conf and do anything it should give that error I think.

Hopefully that makes sense. This is all a bit tricky and I haven't fully tried it all yet either but I think that is what is happening.

[ddpbsd]

On Fri, May 16, 2014 at 2:04 PM, awiddersheim [email protected] wrote:

Try removing/renaming your default ossec.conf and it should produce the error he is seeing. The -C option is actually completely ignored in this case. The error he is getting is actually the result of the default ossec.conf in the installation location being missing as well.

So for example if I installed OSSEC to /opt/ossec and ran ossec-execd with the -C option specifying a configuration file at /etc/ossec.conf nothing should happen.

If I would remove the ossec.conf in /opt/ossec/etc/ossec.conf and do anything it should give that error I think.

Hopefully that makes sense. This is all a bit tricky and I haven't fully tried it all yet either but I think that is what is happening.

Ok, I had to remove several ossec.confs to get this to work. Thanks
for the help.
It should obviously get fixed, but wow you really have to go out of
your way to find this issue. I can't imagine how this would matter in
the real world.

Reply to this email directly or view it on GitHub.

[drawks]

It matters in the real world whenever you want to use the flags as they are described to point to a different config than the default compiled in location... I understand that there is some presumption that everyone (or near everyone) will just use the installer scripts, but in the real world I think it is reasonable that the usage statement matches reality.

The "real world" is full of use cases... I hit this bug less than 24 hours after being asked to help prepare ossec for our environment.

[ddpbsd]

On Fri, May 16, 2014 at 2:29 PM, Dave Rawks [email protected] wrote:

It matters in the real world whenever you want to use the flags as they are described to point to a different config than the default compiled in location... I understand that there is some presumption that everyone (or near everyone) will just use the installer scripts, but in the real world I think it is reasonable that the usage statement matches reality.

The "real world" is full of use cases... I hit this bug less than 24 hours after being asked to help prepare ossec for our environment.

And that's great, I hope you find a bunch more bugs.

Reply to this email directly or view it on GitHub.

[jrossi]

This open is also related #24 ossec a fair amount of locations that get compiled in and cannot be changed with a recompile. Some tools this effects more then others but this is definitely an issue.

[awiddersheim]

I created some small fixes for this issue only for ossec-execd and ossec-agentd. I wouldn't be surprised if some of the other OSSEC binaries are also affected by this issue but I didn't go through them all yet.

I want to see what peoples feelings are about this stop gap fix and if people agree that this is an acceptable solution to hold things over until a more permanent one is created. Once that is done I'll go through the rest of the binaries and fix any problems in them as well.

Some main things I'd like people to look at are as follows:

1. Is allowing the user to change the location of the configuration file any type of security risk that anyone can think of?
2. It doesn't look like ossec-execd does a chroot at any time. Should it?
3. In ossec-agentd I found that both user and group privilege separation happens. In ossec-execd only group privilege separation happens. Is this correct?
4. In ossec-execd I made the un-implemented options like -D return and error saying as such. Previously, if specified correctly, this would have just done nothing and been silently ignored. This sets up the potential for this to break pre-existing installations who might have for whatever reason specified this option. Is this acceptable to everyone?

The same help statement gets printed for every OSSEC binary despite not every binary using or requiring the same arguments. A good example is '-D' which is used to specify an alternate chroot location but not every OSSEC binary seems to chroot.

I can see where the method used now might have saved some time and code replication but since not all binaries are made equal you end up with misinformation.

There is probably a decent amount of work/testing involved with fixing this properly. The main thing is coming to a shared consensus on what the permanent solution should be. Whether that is making sure every binary does the exact same things so all command line arguments are valid or moving the help from shared.h into every binary... I don't know.

Feel free to discuss.

[jrossi]

On May 25, 2014, at 12:51 PM, "awiddersheim" [email protected] wrote:

I created some small fixes for this issue only for ossec-execd and ossec-agentd. I wouldn't be surprised if some of the other OSSEC binaries are also affected by this issue but I didn't go through them all yet.

Woot this is great and really needed.

I want to see what peoples feelings are about this stop gap fix and if people agree that this is an acceptable solution to hold things over until a more permanent one is created. Once that is done I'll go through the rest of the binaries and fix any problems in them as well.

Some main things I'd like people to look at are as follows:

Is allowing the user to change the location of the configuration file any type of security risk that anyone can think of?

No security issue unless we have setuid programs. But is a great feature; as it will make things like testing even easier. (No need to install to test changes)

It doesn't look like ossec-execd does a chroot at any time. Should it?

No - it's goal is to change the host environment. Chroot making that harder.

In ossec-agentd I found that both user and group privilege separation happens. In ossec-execd only group privilege separation happens. Is this correct?

Yes - dropping group permissions is just good practice with doing fork/exec a lot, which is what ossec-execd.

In ossec-execd I made the un-implemented options like -D return and error saying as such. Previously, if specified correctly, this would have just done nothing and been silently ignored. This sets up the potential for this to break pre-existing installations who might have for whatever reason specified this option. Is this acceptable to everyone?
We should have the binary print the help messages that make sense for it. The current method is a short cut. It should most likely be refactored.
The same help statement gets printed for every OSSEC binary despite not every binary using or requiring the same arguments. A good example is '-D' which is used to specify an alternate chroot location but not every OSSEC binary seems to chroot.

I can see where the method used now might have saved some time and code replication but since not all binaries are made equal you end up with misinformation.

We should have standard functions for each app to set options that they accept and then print only them. But we don't need one help page to rule them all. You have pointed out all the reasons why ;)

There is probably a decent amount of work/testing involved with fixing this properly. The main thing is coming to a shared consensus on what the permanent solution should be. Whether that is making sure every binary does the exact same things so all command line arguments are valid or moving the help from shared.h into every binary... I don't know.

Every common doing the same thing does not make sense, but making sure they each one uses flags in the same way should be a goal. This way we don't make things more confusing over time (again).

Feel free to discuss.

—
Reply to this email directly or view it on GitHub.

[mstarks01]

On 05/25/2014 11:51 AM, awiddersheim wrote:

1. Is allowing the user to change the location of the configuration
   file any type of security risk that anyone can think of?

I think as long as it's under the chroot dir, it's fine. If it's not,
well then it I think it's more likely that either poor permissions could
come into play or things like directory traversal attacks become more
likely. Maybe the potential for an attacker to use a non-fully qualified
file reference by replacing it with his own farther up in the path. I
don't know.

1. In |ossec-execd| I made the un-implemented options like |-D| return
   and error saying as such. Previously, if specified correctly, this
   would have just done nothing and been silently ignored. This sets up
   the potential for this to break pre-existing installations who might
   have for whatever reason specified this option. Is this acceptable
   to everyone?

I would say it should do what it says, so fixing this would be correct.

There is probably a decent amount of work/testing involved with fixing
this properly. The main thing is coming to a shared consensus on what
the permanent solution should be. Whether that is making sure every
binary does the exact same things so all command line arguments are
valid or moving the help from |shared.h| into every binary... I don't know.

It doesn't seem to make sense to make these options shared, except for
the few that truly are and will always be.

[awiddersheim]

Perhaps I'll just get rid of the shared help and go through them all creating a proper help for each since that seems like the general consensus on what should happen. If I find anything common between all of them I'll consider getting that shared but it might not be worth it.

I would say it should do what it says, so fixing this would be correct.

The problem is ossec-execd doesn't chroot. This option has no application for ossec-execd so it can't really be fixed. We just need to remove the shared help.

Yes - dropping group permissions is just good practice with doing fork/exec a lot, which is what ossec-execd.

Right, but it only drops group privileges but not user privileges. Note the disparity between ossec-execd and ossec-agentd below:

https://github.com/ossec/ossec-hids/blob/master/src/os_execd/execd.c#L134
https://github.com/ossec/ossec-hids/blob/master/src/client-agent/agentd.c#L51

I guess only doing the user stuff is necessary when doing a chroot which ossec-execd doesn't do?

[jrossi]

On May 25, 2014, at 1:40 PM, "Michael Starks" [email protected] wrote:

On 05/25/2014 11:51 AM, awiddersheim wrote:

1. Is allowing the user to change the location of the configuration
   file any type of security risk that anyone can think of?

I think as long as it's under the chroot dir, it's fine. If it's not,
well then it I think it's more likely that either poor permissions could
come into play or things like directory traversal attacks become more
likely. Maybe the potential for an attacker to use a non-fully qualified
file reference by replacing it with his own farther up in the path. I
don't know.

No it does not add any security. To chroot you need root privs and we don't have any setuid apps in ossec. (Good thing) So if you run it as non root user this is a hugely useful feature as you can test config with out changing production.

1. In |ossec-execd| I made the un-implemented options like |-D| return
   and error saying as such. Previously, if specified correctly, this
   would have just done nothing and been silently ignored. This sets up
   the potential for this to break pre-existing installations who might
   have for whatever reason specified this option. Is this acceptable
   to everyone?

I would say it should do what it says, so fixing this would be correct.

No this is incorrect chroot with ossec-execd does not make sense. Every single use of active response I know of requires root access to full system.

There is probably a decent amount of work/testing involved with fixing
this properly. The main thing is coming to a shared consensus on what
the permanent solution should be. Whether that is making sure every
binary does the exact same things so all command line arguments are
valid or moving the help from |shared.h| into every binary... I don't know.

It doesn't seem to make sense to make these options shared, except for
the few that truly are and will always be.
—
Reply to this email directly or view it on GitHub.

[jrossi]

On May 25, 2014, at 2:15 PM, "awiddersheim" [email protected] wrote:

Perhaps I'll just get rid of the shared help and go through them all creating a proper help for each since that seems like the general consensus on what should happen. If I find anything common between all of them I'll consider getting that shared but it might not be worth it.

I would say it should do what it says, so fixing this would be correct.

The problem is ossec-execd doesn't chroot. This option has no application for ossec-execd so it can't really be fixed. We just need to remove the shared help.

Correct

Yes - dropping group permissions is just good practice with doing fork/exec a lot, which is what ossec-execd.

Right, but it only drops group privileges but not user privileges. Note the disparity between ossec-execd and ossec-agentd below:

https://github.com/ossec/ossec-hids/blob/master/src/os_execd/execd.c#L134
https://github.com/ossec/ossec-hids/blob/master/src/client-agent/agentd.c#L51

I guess only doing the user stuff is necessary when doing a chroot which ossec-execd doesn't do?

Chroot and other functions require root syscalls and as such must be started as root. Once chroot and other controlled syscalls are completed most of ossec drops root access and groups. But for some programs this does not make sense. Ossec-execd is one of them as it makes changes to the host OS to protect it. All the following require root access: iptables, tcp wrappers, hosts file, and many many more.
—
Reply to this email directly or view it on GitHub.

[drawks]

@jrossi I'd argue that it could easily make sense to have execd run as a non-root user. Active response is essentially just exec'ing commands as configured and depending on the user's environment the commands that are called could have their own methods of priviledge escalation (sudo inside shell script for example) or could even be crafted setuid programs. Since I don't see any mechanism by which the configured actions of active response can be run in the context of another user or group it seems a bit ham fisted for execd to always run as root and always run it's child processes as root.

As such it would seem, to me at least, a worthwhile effort to have execd be able to drop root at runtime if configured to do so AND also to enable it, if root isn't dropped, to exec child processes as different uid/gid as configured via active response command options.

[jrossi]

@drawks Ham fisted - yeah I will agree with that. But, then again I would say the same about sudo setup is also ham fisted. I feel that ossec-execd is feed from the another application that has already dropped privs so we are Basically we are re-arranging chairs. No gains and no loss.

Chroot on the other hand is something that does NOT make sense even with sudo. Breaking out of your own chroot to update hosts is crazy and not something I would like to codify and leave around.

[mstarks01]

+1 @drawks. While I agree that most current uses of active response might as well use root because of what they do, AR is a blank slate of opportunities and it makes sense to use the least privilege possible for the task.

[jrossi]

@drawks @mstarks01 great - look forward to what you guys come up with. It is a hard problem and something that I think was handle correctly and simply with ossec-execd, but "I think" only goes so far so look forward to what i can learn in this area about how to handle this.

[awiddersheim]

Please open another issue for this other issue if anyone feels it important enough so we can keep the original issue on topic. Didn't mean to start everyone in a different direction. Apologies.

[drawks]

@awiddersheim you are absolutely right, sorry for piling on unrelated stuff in this issue. I've created #215 to specifically add a feature request/suggestion.

[awiddersheim]

Perfect. Thanks. It is no problem I somewhat brought it on myself.

Will hopefully tackle fixing command line arguments and some path hardcoding this weekend now that we have a consensus on what should be done.

[awiddersheim]

Been like 12 or 13 weekends since I said I would get to this. Been pretty busy with other things. Made some headway though. Any feedback thus far might be good. Planning on doing a bunch more though. Unfortunately, for a change like this, it is easier to make just a larger PR than just smaller ones I think.

[awiddersheim]

Note that this PR #270 only really fixes the fact that the options are incorrect for some of the programs. It doesn't really address the fact that OSSEC isn't really flexible with where/how you install it which was also brought up in this issue.

For example, in many programs you can specify an alternate location for chrooting or the configuration file to read but some programs will also try to read the OSSEC internal_options files. The path/name of these files are hard coded and failure to read them will result in the program dying. No option exists today to manipulate that.

Another example is the client_keys files. These are hardcoded as well and can't really be changed outside of compile time.

I for one have little need to make OSSEC that flexible. By that I mean I don't plan on needing to change the name/location of the client_keys file or the internal_options files on the fly. Being able to set OSSEC's location at compile time, the -D chrooting option and -c configuration option are all I need to run OSSEC in production so I doubt I'll be doing any code to expand this functionality until I do have a need.

For those that don't know how to easily configure OSSEC's installation location at compile time doing the following should be all you need to do:

echo "CEXTRA=-DDEFAULTDIR=\\\"$location\\\"" > Config.OS
make all

Changing anything else is pretty hit or miss.

[awiddersheim]

With the merger of #270 I'm going to close this as that should have fixed the issue where the help generated did not always show the correct options each binary was capable of.

Another issue was spawned from this in #215 and that can be handled there. I also created a new issue for hard coded paths in #278.