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.33k stars 1.02k forks source link

Weak key generation in manage_agents #2082

Open mcpherrinm opened 1 year ago

mcpherrinm commented 1 year ago

I was reviewing ossec-hids as a potential FIM solution. In manage_agents, I noticed that the agent keys are generated as:

            /* Random 1: Time took to write the agent information
             * Random 2: Time took to choose the action
             * Random 3: All of this + time + pid
             * Random 4: Md5 all of this + the name, key and IP
             * Random 5: Final key
             */

            snprintf(str1, STR_SIZE, "%d%s%d", (int)(time3 - time2), name, (int)rand1);
            snprintf(str2, STR_SIZE, "%d%s%s%d", (int)(time2 - time1), ip, id, (int)rand2);

            OS_MD5_Str(str1, md1);
            OS_MD5_Str(str2, md2);

            snprintf(str1, STR_SIZE, "%s%d%d%d", md1, (int)getpid(), (int)random(),
                     (int)time3);
            OS_MD5_Str(str1, md1);

            fprintf(fp, "%s %s %s %s%s\n", id, name, c_ip.ip, md1, md2);
            fclose(fp);

These seem relatively weak sources of random compared to using a CSPRNG, which is concerning. I don't see any dedicated security contact for the project so I'm opening this issue.

Do you consider this a security problem? Are there any mitigating factors I'm unaware of?

cpu commented 1 year ago

See also https://github.com/ossec/ossec-hids/issues/714

atomicturtle commented 1 year ago

Could it be better? Yes, but I dont see this as an issue in this case since we're using a hash of the value (rather than the value) that results in a 128-bit key.

g3ntry commented 1 year ago

It's a bit frayed at the edges. It's probably more important than it used to be.

cpu commented 1 year ago

I dont see this as an issue in this case since we're using a hash of the value (rather than the value) that results in a 128-bit key.

You are not getting a key with 128 bits of entropy just because the digest is 128 bits. Running a weak source of entropy through a hash function does not address the underlying problem.