Bug 1679196

Summary: pcs should not allow {}\n\r characters in corosync.conf values
Product: Red Hat Enterprise Linux 8 Reporter: Tomas Jelinek <tojeline>
Component: pcsAssignee: Tomas Jelinek <tojeline>
Status: CLOSED ERRATA QA Contact: cluster-qe <cluster-qe>
Severity: unspecified Docs Contact:
Priority: high    
Version: 8.0CC: cfeist, cluster-maint, cluster-qe, idevat, jpokorny, mlisik, mmazoure, omular, tojeline
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: pcs-0.10.2-1.el8 Doc Type: Bug Fix
Doc Text:
Cause: User puts special characters into corosync options or values and uses --force to override pcs validation of those. Consequence: Pcs prints a warning noting the validation has been overridden and updates corosync.conf. Corosync cannot start / reload its config. Fix: Make pcs validate that the entered corosync options and values only contain safe characters and do not break corosync.conf. Inform the user if it's not the case. Result: The user cannot make corosync.conf unreadable for corosync by entering special characters in pcs commands.
 Story Points: ---
Clone Of: 1515193 Environment:
Last Closed: 2019-11-05 20:39:40 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1682129    
Bug Blocks:    
Attachments:
Description Flags
proposed fix + tests none

Description Tomas Jelinek 2019-02-20 15:09:03 UTC 
+++ This bug was initially created as a clone of Bug #1515193 +++

From bz1389209 comment 33:
$ pcs quorum device update model \
  "host=$(printf \
    "localhost\n}\nheuristics {\nexec_bar: /usr/bin/sh -c 'echo reboot>>/root/.profile||:'\n")"

Need to check with corosync parser which characters should be disallowed.

--- Additional comment from Jan Pokorný [poki] on 2017-11-20 15:55:19 CET ---

This is rather urgent for reasons in [bug 1389209 comment 35].

And rather than being matter of corosync parser, it's a general
deficiency in checking the inputs (possibly from less-privileged
sources, depending on the exact use case) to compose the resulting
corosync.conf from, as currently the precooked configuration
snippets may be injected in "plain unlimited string"-evaluated
instances.

--- Additional comment from Tomas Jelinek on 2017-11-20 16:25:37 CET ---

- { anywhere in a line means start of a section -> disallowed
- } anywhere in a line means end of a section -> disallowed
- \n and \r starts a new line which could be used to set its own key-value or section -> disallowed
- : is allowed, as only the first : in a line matters
- # is allowed, as the # only matters when it is the first character in a line (not considering whitespace)
- there is no escaping available
Comment 1 Tomas Jelinek 2019-02-20 15:09:47 UTC 
Corosync parser has been updated in corosync3 so we need to recheck it.
Comment 3 Tomas Jelinek 2019-05-17 13:08:23 UTC 
Changes in corosync parser have no effect on the set of disallowed characters.
Comment 4 Tomas Jelinek 2019-06-11 12:44:04 UTC 
Created attachment 1579347 [details]
proposed fix + tests

With this fix, pcs does not allow characters breaking corosync.conf structure to be put into corosync.conf not even if --force is used.

Forbidden characters in options' values: } { \r \n
Allowed characters in options' names: a-z A-Z 0-9 _ / -

Example:
[root@rh80-node1:~]# pcs quorum device add model net host=rh80-node3 algorithm=lms "port=12345}" "op#tion=value{" --force
Warning: invalid quorum device model option 'op#tion', allowed options are: 'algorithm', 'connect_timeout', 'force_ip_version', 'host', 'port', 'tie_breaker'
Warning: '12345}' is not a valid port value, use a port number (1..65535)
Error: invalid quorum device model option 'op#tion', quorum device model options may contain a-z A-Z 0-9 /_- characters only
Error: port cannot contain {}\n\r characters
Error: op#tion cannot contain {}\n\r characters
Error: Errors have occurred, therefore pcs is unable to continue
[root@rh80-node1:~]# echo $?
1
Comment 5 Ivan Devat 2019-06-14 07:35:44 UTC 
After Fix:

[ant8 ~] $ rpm -q pcs
pcs-0.10.2-1.el8.x86_64
[ant8 ~] $ pcs quorum device add model net host=rh80-node3 algorithm=lms "port=12345}" "op#tion=value{" --force
Warning: invalid quorum device model option 'op#tion', allowed options are: 'algorithm', 'connect_timeout', 'force_ip_version', 'host', 'port', 'tie_breaker'
Warning: '12345}' is not a valid port value, use a port number (1..65535)
Error: invalid quorum device model option 'op#tion', quorum device model options may contain a-z A-Z 0-9 /_- characters only
Error: port cannot contain {}\n\r characters
Error: op#tion cannot contain {}\n\r characters
Error: Errors have occurred, therefore pcs is unable to continue
Comment 11 errata-xmlrpc 2019-11-05 20:39:40 UTC 
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2019:3311