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

Fix deletion of duplicate entries in Logcollector #3616

Merged
merged 2 commits into from
Jul 17, 2019

Conversation

albertomn86
Copy link
Contributor

@albertomn86 albertomn86 commented Jul 3, 2019

Related issue
#3051

Description

This PR solves the problem related with the order in the read configuration in ossec-logcollector. The function Remove_Localfile was changing the array positions during the duplicates deletion. This behavior changed the value to keep in the array.

Tests

  • Compilation without warnings in every supported platform
    • Linux
    • Windows
    • MAC OS X
  • Source installation
  • Source upgrade
  • Memory tests
    • Valgrind report for affected components
    • CPU impact
    • RAM usage impact
  • Configuration on demand reports new parameters
  • Review logs syntax and correct language

@albertomn86 albertomn86 requested a review from vikman90 July 3, 2019 14:45
@albertomn86
Copy link
Contributor Author

Test with duplicated localfiles

ossec.conf

  <localfile>
    <log_format>apache</log_format>
    <location>/var/log/apache2/error.log</location>
    <target>test1</target>
  </localfile>

  <localfile>
    <log_format>apache</log_format>
    <location>/var/log/apache2/error.log</location>
    <target>test2</target>
  </localfile>

  <localfile>
    <log_format>apache</log_format>
    <location>/var/log/apache2/error.log</location>
    <target>test3</target>
    <ignore_binaries>yes</ignore_binaries>
  </localfile>

  <localfile>
    <log_format>apache</log_format>
    <location>/var/log/apache2/error.log</location>
    <target>test4</target>
    <ignore_binaries>yes</ignore_binaries>
  </localfile>

API request

# curl -u foo:bar -k -X GET "http://127.0.0.1:55000/agents/000/config/logcollector/localfile?pretty"
{
   "error": 0,
   "data": {
      "localfile": [
         {
            "file": "/var/log/apache2/error.log",
            "logformat": "apache",
            "ignore_binaries": "yes",
            "target": [
               "test4"
            ],
            "only-future-events": "yes"
         }
      ]
   }
}

@albertomn86
Copy link
Contributor Author

Valgrind report

==58146== Memcheck, a memory error detector
==58146== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==58146== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==58146== Command: /var/ossec/bin/ossec-logcollector -fdd
==58146==
==58146==
==58146== HEAP SUMMARY:
==58146==     in use at exit: 54,139 bytes in 44 blocks
==58146==   total heap usage: 5,140 allocs, 5,096 frees, 2,512,997 bytes allocated
==58146==
==58146== 6 bytes in 1 blocks are still reachable in loss record 1 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x11F8EA: Read_Localfile (localfile-config.c:114)
==58146==    by 0x1233E9: read_main_elements (config.c:114)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 7 bytes in 1 blocks are still reachable in loss record 2 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x12076B: Read_Localfile (localfile-config.c:226)
==58146==    by 0x1233E9: read_main_elements (config.c:114)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 16 bytes in 1 blocks are still reachable in loss record 3 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x1A6543: OS_StrBreak (os_regex_strbreak.c:34)
==58146==    by 0x11F892: Read_Localfile (localfile-config.c:111)
==58146==    by 0x1233E9: read_main_elements (config.c:114)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 18 bytes in 3 blocks are still reachable in loss record 4 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x1B43B8: Read_Socket (socket-config.c:69)
==58146==    by 0x123A9D: read_main_elements (config.c:171)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 18 bytes in 3 blocks are still reachable in loss record 5 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x19C0A2: _OSHash_Add (hash_op.c:279)
==58146==    by 0x19BF56: OSHash_Add (hash_op.c:231)
==58146==    by 0x11CC0D: w_msg_hash_queues_add_entry (logcollector.c:1632)
==58146==    by 0x11C628: set_sockets (logcollector.c:1543)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 27 bytes in 1 blocks are still reachable in loss record 6 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x120699: Read_Localfile (localfile-config.c:224)
==58146==    by 0x1233E9: read_main_elements (config.c:114)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 32 bytes in 1 blocks are still reachable in loss record 7 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x11F32D: Read_Localfile (localfile-config.c:70)
==58146==    by 0x1233E9: read_main_elements (config.c:114)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 32 bytes in 1 blocks are still reachable in loss record 8 of 25
==58146==    at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x11C405: set_sockets (logcollector.c:1524)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 63 bytes in 3 blocks are still reachable in loss record 9 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5B1AC99: strdup (strdup.c:42)
==58146==    by 0x1B449E: Read_Socket (socket-config.c:72)
==58146==    by 0x123A9D: read_main_elements (config.c:171)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 88 bytes in 1 blocks are still reachable in loss record 10 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B860: OSHash_Create (hash_op.c:28)
==58146==    by 0x11CA82: w_msg_hash_queues_init (logcollector.c:1614)
==58146==    by 0x113934: main (main.c:120)
==58146==
==58146== 88 bytes in 1 blocks are still reachable in loss record 11 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B860: OSHash_Create (hash_op.c:28)
==58146==    by 0x118A31: LogCollectorStart (logcollector.c:95)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 88 bytes in 1 blocks are still reachable in loss record 12 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B860: OSHash_Create (hash_op.c:28)
==58146==    by 0x118A6D: LogCollectorStart (logcollector.c:101)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 96 bytes in 3 blocks are still reachable in loss record 13 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19C03A: _OSHash_Add (hash_op.c:271)
==58146==    by 0x19BF56: OSHash_Add (hash_op.c:231)
==58146==    by 0x11CC0D: w_msg_hash_queues_add_entry (logcollector.c:1632)
==58146==    by 0x11C628: set_sockets (logcollector.c:1543)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 192 bytes in 1 blocks are still reachable in loss record 14 of 25
==58146==    at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x1B40CD: Read_Socket (socket-config.c:44)
==58146==    by 0x123A9D: read_main_elements (config.c:171)
==58146==    by 0x123E8C: ReadConfig (config.c:245)
==58146==    by 0x1140AF: LogCollectorConfig (config.c:81)
==58146==    by 0x113940: main (main.c:123)
==58146==
==58146== 272 bytes in 1 blocks are possibly lost in loss record 15 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x4013C86: allocate_dtv (dl-tls.c:290)
==58146==    by 0x4013C86: _dl_allocate_tls (dl-tls.c:538)
==58146==    by 0x5870421: allocate_stack (allocatestack.c:597)
==58146==    by 0x5870421: pthread_create@@GLIBC_2.2.5 (pthread_create.c:669)
==58146==    by 0x19B668: CreateThreadJoinable (pthreads_op.c:47)
==58146==    by 0x19B70F: CreateThread (pthreads_op.c:62)
==58146==    by 0x11916C: LogCollectorStart (logcollector.c:301)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 288 bytes in 3 blocks are still reachable in loss record 16 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x11CAEE: w_msg_hash_queues_add_entry (logcollector.c:1627)
==58146==    by 0x11C628: set_sockets (logcollector.c:1543)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 480 bytes in 1 blocks are still reachable in loss record 17 of 25
==58146==    at 0x4C31D2F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x122AE1: Remove_Localfile (localfile-config.c:626)
==58146==    by 0x11C0A4: remove_duplicates (logcollector.c:1480)
==58146==    by 0x118B67: LogCollectorStart (logcollector.c:161)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 528 bytes in 3 blocks are still reachable in loss record 18 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x191F3B: queue_init (queue_op.c:16)
==58146==    by 0x11CB4B: w_msg_hash_queues_add_entry (logcollector.c:1628)
==58146==    by 0x11C628: set_sockets (logcollector.c:1543)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 552 bytes in 1 blocks are still reachable in loss record 19 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x5AFD9CC: __fopen_internal (iofopen.c:69)
==58146==    by 0x11A4DC: handle_file (logcollector.c:798)
==58146==    by 0x11AB5D: set_read (logcollector.c:1037)
==58146==    by 0x119025: LogCollectorStart (logcollector.c:252)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 816 bytes in 3 blocks are possibly lost in loss record 20 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x4013C86: allocate_dtv (dl-tls.c:290)
==58146==    by 0x4013C86: _dl_allocate_tls (dl-tls.c:538)
==58146==    by 0x5870421: allocate_stack (allocatestack.c:597)
==58146==    by 0x5870421: pthread_create@@GLIBC_2.2.5 (pthread_create.c:669)
==58146==    by 0x19B668: CreateThreadJoinable (pthreads_op.c:47)
==58146==    by 0x19B70F: CreateThread (pthreads_op.c:62)
==58146==    by 0x11D4A1: w_create_output_threads (logcollector.c:1776)
==58146==    by 0x1190EC: LogCollectorStart (logcollector.c:290)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 1,088 bytes in 4 blocks are possibly lost in loss record 21 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x4013C86: allocate_dtv (dl-tls.c:290)
==58146==    by 0x4013C86: _dl_allocate_tls (dl-tls.c:538)
==58146==    by 0x5870421: allocate_stack (allocatestack.c:597)
==58146==    by 0x5870421: pthread_create@@GLIBC_2.2.5 (pthread_create.c:669)
==58146==    by 0x19B668: CreateThreadJoinable (pthreads_op.c:47)
==58146==    by 0x19B70F: CreateThread (pthreads_op.c:62)
==58146==    by 0x11E015: w_create_input_threads (logcollector.c:2061)
==58146==    by 0x1190F6: LogCollectorStart (logcollector.c:293)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 8,256 bytes in 1 blocks are still reachable in loss record 22 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B8BF: OSHash_Create (hash_op.c:41)
==58146==    by 0x11CA82: w_msg_hash_queues_init (logcollector.c:1614)
==58146==    by 0x113934: main (main.c:120)
==58146==
==58146== 8,256 bytes in 1 blocks are still reachable in loss record 23 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B8BF: OSHash_Create (hash_op.c:41)
==58146==    by 0x118A31: LogCollectorStart (logcollector.c:95)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 8,256 bytes in 1 blocks are still reachable in loss record 24 of 25
==58146==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x19B8BF: OSHash_Create (hash_op.c:41)
==58146==    by 0x118A6D: LogCollectorStart (logcollector.c:101)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== 24,576 bytes in 3 blocks are still reachable in loss record 25 of 25
==58146==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58146==    by 0x191F98: queue_init (queue_op.c:17)
==58146==    by 0x11CB4B: w_msg_hash_queues_add_entry (logcollector.c:1628)
==58146==    by 0x11C628: set_sockets (logcollector.c:1543)
==58146==    by 0x118AAE: LogCollectorStart (logcollector.c:106)
==58146==    by 0x113CC1: main (main.c:187)
==58146==
==58146== LEAK SUMMARY:
==58146==    definitely lost: 0 bytes in 0 blocks
==58146==    indirectly lost: 0 bytes in 0 blocks
==58146==      possibly lost: 2,176 bytes in 8 blocks
==58146==    still reachable: 51,963 bytes in 36 blocks
==58146==         suppressed: 0 bytes in 0 blocks
==58146==
==58146== For counts of detected and suppressed errors, rerun with: -v
==58146== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)

@albertomn86
Copy link
Contributor Author

ossec.conf

  <localfile>
    <location>C:\test\file1</location>
    <log_format>iis</log_format>
  </localfile>
  
  <localfile>
    <location>C:\test\file1</location>
    <log_format>json</log_format>
  </localfile>
  
  <localfile>
    <location>C:\test\file1</location>
    <log_format>syslog</log_format>
  </localfile>

API request

# curl -u foo:bar -k -X GET "http://127.0.0.1:55000/agents/001/config/logcollector/localfile?pretty"
{
   "error": 0,
   "data": {
      "localfile": [
         {
            "file": "C:\\test\\file1",
            "logformat": "syslog",
            "ignore_binaries": "no",
            "target": [
               "agent"
            ],
            "frequency": 2,
            "only-future-events": "yes"
         }
      ]
   }
}

@vikman90 vikman90 self-assigned this Jul 16, 2019
@vikman90 vikman90 changed the title Fix duplicated entries deletion in logcollector Fix deletion of duplicate entries in Logcollector Jul 16, 2019
@vikman90 vikman90 added type/bug Something isn't working module/logcollector labels Jul 16, 2019
Copy link
Member

@vikman90 vikman90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vikman90 vikman90 requested a review from snaow July 16, 2019 15:02
@vikman90 vikman90 merged commit fb2a056 into 3.9 Jul 17, 2019
@vikman90 vikman90 deleted the 3.9-logcollector-config-3051 branch July 17, 2019 14:38
@vikman90 vikman90 requested a review from bah07 July 24, 2019 14:48
Copy link
Contributor

@bah07 bah07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/logcollector type/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants