Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
1.99k stars 571 forks source link

[dev.icinga.com #6143] Escape macros in perfdata files #1521

Closed icinga-migration closed 10 years ago

icinga-migration commented 10 years ago

This issue has been migrated from Redmine: https://dev.icinga.com/issues/6143

Created by mfriedrich on 2014-05-07 16:26:25 +00:00

Assignee: gbeutner Status: Rejected (closed on 2014-05-26 10:03:14 +00:00) Target Version: (none) Last Update: 2014-05-26 10:03:14 +00:00 (in Redmine)

Icinga Version: 0.0.10

$service.output$ and other nasty stuff which may contain shell meta characters must be properly escaped.

Attachments

icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-11 14:08:26 +00:00

Output via Classic UI

Passing a pipe character unescaped breaks the output/perfdata parser similar to 1.x

Output

;.$3`,.\|<>&/%

Perfdata

foo=1234

[2014-05-11 16:02:44 +0200]  information/compat: Executing external command: [1399816964] PROCESS_SERVICE_CHECK_RESULT;2742-server;2742-macro-test;2;;.$3`,.\|<>&/%|foo=1234
[2014-05-11 16:02:44 +0200]  debug/db_ido: add external command history
[2014-05-11 16:02:44 +0200]  information/icinga: Processing passive check result for service '2742-macro-test'
[2014-05-11 16:02:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_externalcommands (command_args, command_name, command_type, entry_time, instance_id) VALUES ('2742-server;2742-macro-test;2;.$3`,.\\|<>&/%|foo=1234', 'PROCESS_SERVICE_CHECK_RESULT', '30', FROM_UNIXTIME(1399816964), 2)
[2014-05-11 16:02:44 +0200]  debug/db_ido: add log entry history for '2742-server!2742-macro-test'
[2014-05-11 16:02:44 +0200]  debug/db_ido: add service check history for '2742-server!2742-macro-test'
[2014-05-11 16:02:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_logentries (entry_time, entry_time_usec, instance_id, logentry_data, logentry_time, logentry_type, object_id) VALUES (FROM_UNIXTIME(1399816964), '65471', 2, 'SERVICE ALERT: 2742-server;2742-macro-test;CRITICAL;SOFT;3;.$3`,.\\', FROM_UNIXTIME(1399816964), '65536', 899)
[2014-05-11 16:02:44 +0200]  debug/db_ido: Endpoint node: 'imagine' status update for '2742-server!2742-macro-test'
[2014-05-11 16:02:44 +0200]  debug/db_ido: add state change history for '2742-server!2742-macro-test'
[2014-05-11 16:02:44 +0200]  debug/db_ido_mysql: Query: UPDATE icinga_servicestatus SET acknowledgement_type = '0',  active_checks_enabled = '0',  check_command = '2742-macro-command',  check_source = 'imagine',  check_type = '1',  current_check_attempt = '3',  current_notification_number = '0',  current_state = '2',  event_handler = '',  event_handler_enabled = '0',  execution_time = '0',  flap_detection_enabled = '1',  has_been_checked = '1',  instance_id = 2,  is_flapping = '1',  is_reachable = '0',  last_check = FROM_UNIXTIME(1399816964),  last_hard_state_change = FROM_UNIXTIME(1399816114),  last_state_change = FROM_UNIXTIME(1399816964),  last_time_critical = FROM_UNIXTIME(1399816964),  last_time_ok = FROM_UNIXTIME(1399816114),  last_time_warning = FROM_UNIXTIME(1399816346),  latency = '0',  long_output = '',  max_check_attempts = '3',  modified_service_attributes = '2',  next_check = FROM_UNIXTIME(1399817001),  normal_check_interval = '5',  notifications_enabled = '1',  output = '.$3`,.\\',  passive_checks_enabled = '1',  percent_state_change = '100',  perfdata = '<>&/%|foo=1234',  problem_has_been_acknowledged = '0',  process_performance_data = '1',  retry_check_interval = '1',  scheduled_downtime_depth = '0',  service_object_id = 899,  should_be_scheduled = '0',  state_type = '0',  status_update_time = FROM_UNIXTIME(1399816964) WHERE service_object_id = 899
[2014-05-11 16:02:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_statehistory (check_source, current_check_attempt, instance_id, last_hard_state, last_state, long_output, max_check_attempts, object_id, output, state, state_change, state_time, state_time_usec, state_type) VALUES ('imagine', '3', 2, '0', '1', '', '3', 899, '.$3`,.\\', '2', '1', FROM_UNIXTIME(1399816964), '67146', '0')
[2014-05-11 16:02:44 +0200]  debug/db_ido_mysql: Query: UPDATE icinga_servicestatus SET next_check = FROM_UNIXTIME(1399817264) WHERE instance_id = 2 AND service_object_id = 899

DATATYPE::SERVICEPERFDATA       TIMET::1399816964       HOSTNAME::2742-server   SERVICEDESC::2742-macro-test    SERVICEPERFDATA::<>&/%|foo=1234 SERVICECHECKCOMMAND::2742-macro-command HOSTSTATE::DOWN HOSTSTATETYPE::HARD     SERVICESTATE::CRITICAL  SERVICESTATETYPE::SOFT

Using a proper example with a backslash, the macro is properly escaped.

[2014-05-11 16:05:44 +0200]  information/compat: Executing external command: [1399817144] PROCESS_SERVICE_CHECK_RESULT;2742-server;2742-macro-test;1;;.$3`,.\<>&/%|foo=4567
[2014-05-11 16:05:44 +0200]  debug/db_ido: add external command history
[2014-05-11 16:05:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_externalcommands (command_args, command_name, command_type, entry_time, instance_id) VALUES ('2742-server;2742-macro-test;1;.$3`,.\\<>&/%|foo=4567', 'PROCESS_SERVICE_CHECK_RESULT', '30', FROM_UNIXTIME(1399817144), 2)
[2014-05-11 16:05:44 +0200]  information/icinga: Processing passive check result for service '2742-macro-test'
[2014-05-11 16:05:44 +0200]  debug/db_ido: add log entry history for '2742-server!2742-macro-test'
[2014-05-11 16:05:44 +0200]  debug/db_ido: add service check history for '2742-server!2742-macro-test'
[2014-05-11 16:05:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_logentries (entry_time, entry_time_usec, instance_id, logentry_data, logentry_time, logentry_type, object_id) VALUES (FROM_UNIXTIME(1399817144), '894253', 2, 'SERVICE ALERT: 2742-server;2742-macro-test;WARNING;HARD;1;.$3`,.\\<>&/%', FROM_UNIXTIME(1399817144), '32768', 899)
[2014-05-11 16:05:44 +0200]  debug/db_ido: Endpoint node: 'imagine' status update for '2742-server!2742-macro-test'
[2014-05-11 16:05:44 +0200]  debug/db_ido: add state change history for '2742-server!2742-macro-test'
[2014-05-11 16:05:44 +0200]  debug/db_ido_mysql: Query: UPDATE icinga_servicestatus SET acknowledgement_type = '0',  active_checks_enabled = '0',  check_command = '2742-macro-command',  check_source = 'imagine',  check_type = '1',  current_check_attempt = '1',  current_notification_number = '0',  current_state = '1',  event_handler = '',  event_handler_enabled = '0',  execution_time = '0',  flap_detection_enabled = '1',  has_been_checked = '1',  instance_id = 2,  is_flapping = '1',  is_reachable = '0',  last_check = FROM_UNIXTIME(1399817144),  last_hard_state_change = FROM_UNIXTIME(1399817144),  last_state_change = FROM_UNIXTIME(1399817144),  last_time_critical = FROM_UNIXTIME(1399816964),  last_time_ok = FROM_UNIXTIME(1399816114),  last_time_warning = FROM_UNIXTIME(1399817144),  latency = '0',  long_output = '',  max_check_attempts = '3',  modified_service_attributes = '2',  next_check = FROM_UNIXTIME(1399817264),  normal_check_interval = '5',  notifications_enabled = '1',  output = '.$3`,.\\<>&/%',  passive_checks_enabled = '1',  percent_state_change = '100',  perfdata = 'foo=4567',  problem_has_been_acknowledged = '0',  process_performance_data = '1',  retry_check_interval = '1',  scheduled_downtime_depth = '0',  service_object_id = 899,  should_be_scheduled = '0',  state_type = '1',  status_update_time = FROM_UNIXTIME(1399817144) WHERE service_object_id = 899
[2014-05-11 16:05:44 +0200]  debug/db_ido_mysql: Query: INSERT INTO icinga_statehistory (check_source, current_check_attempt, instance_id, last_hard_state, last_state, long_output, max_check_attempts, object_id, output, state, state_change, state_time, state_time_usec, state_type) VALUES ('imagine', '1', 2, '1', '2', '', '3', 899, '.$3`,.\\<>&/%', '1', '1', FROM_UNIXTIME(1399817144), '896553', '1')
[2014-05-11 16:05:44 +0200]  debug/db_ido_mysql: Query: UPDATE icinga_servicestatus SET next_check = FROM_UNIXTIME(1399817444) WHERE instance_id = 2 AND service_object_id = 899

DATATYPE::SERVICEPERFDATA       TIMET::1399817144       HOSTNAME::2742-server   SERVICEDESC::2742-macro-test    SERVICEPERFDATA::foo=4567       SERVICECHECKCOMMAND::2742-macro-command HOSTSTATE::DOWN HOSTSTATETYPE::HARD     SERVICESTATE::WARNING   SERVICESTATETYPE::HARD
icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-12 17:12:13 +00:00

icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-21 13:20:31 +00:00

icinga-migration commented 10 years ago

Updated by elippmann on 2014-05-21 14:57:11 +00:00

Why did you reject this bug?

icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-21 15:02:59 +00:00

It's not a bug. A pipe character within an output is invalid per definition. The other characters are properly escaped and work as expected.

icinga-migration commented 10 years ago

Updated by elippmann on 2014-05-21 15:14:47 +00:00

https://nagios-plugins.org/doc/guidelines.html#AEN200 wrote:

'label'=value[UOM];[warn];[crit];[min];[max] Notes:

  1. space separated list of label/value pairs

  2. label can contain any characters except the equals sign or single quote (')

  3. the single quotes for the label are optional. Required if spaces are in the label ...

'This is a pipe|' is perfectly valid.

icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-21 16:51:02 +00:00

The problem is different, and I truly misunderstood it. It's not about the perfdata itself (the Icinga 2 perfdata parser already removes characters it cannot recognize) but rather the $service.output$ which comes unfiltered. And somewhat comment fields used in notification commands

fooo&<<>>.%§/\()[]}{"'_ | bar=1234

results in a modifed template filter using

object PerfdataWriter "perfdata" {
  host_format_template = "DATATYPE::HOSTPERFDATA\tTIMET::$icinga.timet$\tHOSTNAME::$host.name$\tHOSTPERFDATA::$host.perfdata$\tHOSTCHECKCOMMAND::$host.check_command$\tHOSTSTATE::$host.state$\tHOSTSTATETYPE::$host.state_type$\tHOSTOUPUT::$host.output$"
  service_format_template = "DATATYPE::SERVICEPERFDATA\tTIMET::$icinga.timet$\tHOSTNAME::$host.name$\tSERVICEDESC::$service.name$\tSERVICEPERFDATA::$service.perfdata$\tSERVICECHECKCOMMAND::$service.check_command$\tHOSTSTATE::$host.state$\tHOSTSTATETYPE::$host.state_type$\tSERVICESTATE::$service.state$\tSERVICESTATETYPE::$service.state_type$\tSERVICEOUTPUT::$service.output$"

}

which then results in

nbmif /var/spool/icinga2/perfdata # grep 5872-pc *
host-perfdata.1400689876:DATATYPE::HOSTPERFDATA TIMET::1400689857   HOSTNAME::5872-pc   HOSTPERFDATA::bar=1234  HOSTCHECKCOMMAND::hostalive HOSTSTATE::DOWN HOSTSTATETYPE::HARD HOSTOUPUT::fooo&<<>>.%§/\()[]}{"'_

The problem is - security relevance is not given with perfdata files on disk. Rather the format is important, such as the '::' and '\t' separators.

When running commands you may want to filter backticks etc - or properly escape them.

`~$&"|'<>

Ideas on doing that?

icinga-migration commented 10 years ago

Updated by mfriedrich on 2014-05-21 16:51:32 +00:00

icinga-migration commented 10 years ago

Updated by gbeutner on 2014-05-22 06:17:10 +00:00

icinga-migration commented 10 years ago

Updated by gbeutner on 2014-05-22 06:41:02 +00:00

How does PNP4Nagios/inGraph/etc. handle those special characters in perfdata files? What's the problem we're trying to solve in this ticket?

icinga-migration commented 10 years ago

Updated by gbeutner on 2014-05-26 10:03:14 +00:00

According to Eric we shouldn't escape performance data values.