ossec / ossec-hids

OSSEC is an Open Source Host-based Intrusion Detection System that performs log analysis, file integrity checking, policy monitoring, rootkit detection, real-time alerting and active response.
http://www.ossec.net
Other
4.44k stars 1.04k forks source link

Fix overwriting an agent counter with sender counter during updating keys #2064

Closed staskysel closed 1 year ago

staskysel commented 1 year ago

Hi. I'm a newbie here, so please forgive me, if I fail to follow some kind of "code of conduct".

In send_msg(), CreateSecMSG() uses (for persisting sender counter) keys structure, which can be modified by main thread calling OS_UpdateKeys(). Thus main thread can interfere with manager thread calling CreateSecMSG().

To fix this, CreateSecMSG() must be called after obtaining sendmsg_mutex lock.

Otherwise under certain circumstances, quite common when adding new agents, an existing agent's counter is overwritten by sender counter e.g. in the following scenario:

Main thread (HandleSecure()):

  1. Receives a message from an initialized (installed) agent, that is not configured in OSSEC server (yet)
  2. Calls check_keyupdate()
  3. If keys file was modified, calls OS_UpdateKeys()
  4. Calls OS_FreeKeys()
  5. Sets keys.keysize = 0
  6. Waits for 1 second with sleep(1), thus transferring control to the main thread and increasing probability of the following.

Manager thread:

  1. Calls CreateSecMSG() (e.g. from send_msg() called from send_file_toagent() called from read_controlmsg() called from wait_for_msgs())
  2. Calls StoreSenderCounter(), which stores sender counter to keys->keyentries[keys->keysize]->fp
  3. Since keys->keysize is 0, instead of writing to "queue/rids/sender_counter" file, the counter is written to "queue/rids/NNN", where NNN is the lowest agent ID.
  4. This effectively disconnects the agent with the lowest ID, as new messages from it are not accepted, because counter doesn't match.
atomicturtle commented 1 year ago

Looks good to me, Im running test to try and duplicate the scenario you described right now so I can get a before and after verification. Sorry about the delay!

atomicturtle commented 1 year ago

Looks good here, and you're right I had to get something like 120 agents registering at once to hit this one