From 49813968a0ab6d31fd7f2d566acd14c79bbfd37a Mon Sep 17 00:00:00 2001 From: Thomas O'Donnell Date: Thu, 7 May 2020 11:14:48 +0200 Subject: [PATCH 1/3] Add trust_input option to postgresql_slot module Have added a trust_input option to the postgresql_slot module. This only checks the session_role since all other options are passed as parameters. --- .../database/postgresql/postgresql_slot.py | 13 +++++ .../tasks/postgresql_slot_initial.yml | 55 +++++++++++++------ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/plugins/modules/database/postgresql/postgresql_slot.py b/plugins/modules/database/postgresql/postgresql_slot.py index 4c812fdb933..f2672f1305f 100644 --- a/plugins/modules/database/postgresql/postgresql_slot.py +++ b/plugins/modules/database/postgresql/postgresql_slot.py @@ -70,6 +70,11 @@ - Permissions checking for SQL commands is carried out as though the session_role were the one that had logged in originally. type: str + trust_input: + description: + - If C(no), check whether values of some parameters are potentially dangerous. + type: bool + default: yes notes: - Physical replication slots were introduced to PostgreSQL with version 9.4, @@ -89,6 +94,7 @@ author: - John Scalia (@jscalia) - Andrew Klychkov (@Andersson007) +- Thomas O'Donnell (@andytom) extends_documentation_fragment: - community.general.postgres @@ -147,6 +153,9 @@ pass from ansible.module_utils.basic import AnsibleModule +from ansible_collections.community.general.plugins.module_utils.database import ( + check_input, +) from ansible_collections.community.general.plugins.module_utils.postgres import ( connect_to_db, exec_sql, @@ -229,6 +238,7 @@ def main(): session_role=dict(type="str"), output_plugin=dict(type="str", default="test_decoding"), state=dict(type="str", default="present", choices=["absent", "present"]), + trust_input=dict(type="bool", default=True), ) module = AnsibleModule( @@ -242,6 +252,9 @@ def main(): state = module.params["state"] output_plugin = module.params["output_plugin"] + if not module.params["trust_input"]: + check_input(module, module.params['session_role']) + if immediately_reserve and slot_type == 'logical': module.fail_json(msg="Module parameters immediately_reserve and slot_type=logical are mutually exclusive") diff --git a/tests/integration/targets/postgresql_slot/tasks/postgresql_slot_initial.yml b/tests/integration/targets/postgresql_slot/tasks/postgresql_slot_initial.yml index e7977f4bcdd..8a60b6d0f36 100644 --- a/tests/integration/targets/postgresql_slot/tasks/postgresql_slot_initial.yml +++ b/tests/integration/targets/postgresql_slot/tasks/postgresql_slot_initial.yml @@ -1,3 +1,4 @@ +--- # Copyright: (c) 2019, Andrew Klychkov (@Andersson007) # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) @@ -55,7 +56,7 @@ query: "SELECT 1 FROM pg_replication_slots WHERE slot_name = 'slot0'" ignore_errors: yes register: result - + - assert: that: - result.rowcount == 0 @@ -92,7 +93,7 @@ query: "SELECT 1 FROM pg_replication_slots WHERE slot_name = 'slot0' and slot_type = 'physical'" ignore_errors: yes register: result - + - assert: that: - result.rowcount == 1 @@ -123,7 +124,7 @@ query: "SELECT 1 FROM pg_replication_slots WHERE slot_name = 'slot0' and slot_type = 'physical'" ignore_errors: yes register: result - + - assert: that: - result.rowcount == 1 @@ -154,7 +155,7 @@ query: "SELECT 1 FROM pg_replication_slots WHERE slot_name = 'slot0' and slot_type = 'physical'" ignore_errors: yes register: result - + - assert: that: - result.rowcount == 1 @@ -191,7 +192,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('9.6', '>=') - + - assert: that: - result.rowcount == 1 @@ -238,7 +239,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 0 @@ -273,7 +274,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 1 @@ -309,7 +310,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') - + - assert: that: - result.rowcount == 1 @@ -344,7 +345,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 1 @@ -383,7 +384,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 1 @@ -423,7 +424,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 1 @@ -458,7 +459,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 0 @@ -494,7 +495,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') - + - assert: that: - result.rowcount == 0 @@ -529,7 +530,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('10', '>=') and ansible_distribution == 'Ubuntu' - + - assert: that: - result.rowcount == 0 @@ -568,7 +569,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('9.6', '>=') - + - assert: that: - result.rowcount == 1 @@ -603,7 +604,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('9.6', '>=') - + - assert: that: - result.rowcount == 0 @@ -639,7 +640,7 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('9.6', '>=') - + - assert: that: - result.rowcount == 0 @@ -674,12 +675,30 @@ ignore_errors: yes register: result when: postgres_version_resp.stdout is version('9.6', '>=') - + - assert: that: - result.rowcount == 0 when: postgres_version_resp.stdout is version('9.6', '>=') +# Check trust input +- name: postgresql_slot - try using a bad name + postgresql_slot: + session_role: 'curious.anonymous"; SELECT * FROM information_schema.tables; --' + db: postgres + name: slot1 + trust_input: no + register: result + ignore_errors: true + when: postgres_version_resp.stdout is version('9.6', '>=') + +- name: postgresql_slot - check that using a dangerous name fails + assert: + that: + - result is failed + - result.msg is search('is potentially dangerous') + when: postgres_version_resp.stdout is version('9.6', '>=') + # # clean up # From ff75a2dbaf76f5d7bc5fef525f6fae8e5bab079e Mon Sep 17 00:00:00 2001 From: Thomas O'Donnell Date: Thu, 7 May 2020 11:23:54 +0200 Subject: [PATCH 2/3] Add Changelog fragment --- changelogs/fragments/298-postgresql_slot_add_trust_input.yml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/fragments/298-postgresql_slot_add_trust_input.yml diff --git a/changelogs/fragments/298-postgresql_slot_add_trust_input.yml b/changelogs/fragments/298-postgresql_slot_add_trust_input.yml new file mode 100644 index 00000000000..fb2cbece340 --- /dev/null +++ b/changelogs/fragments/298-postgresql_slot_add_trust_input.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - postgresql_slot - add the ``trust_input`` parameter (https://github.com/ansible-collections/community.general/pull/298). From 18c8a71dd4f5ef51a9f51dcea7a2deb443d9255a Mon Sep 17 00:00:00 2001 From: Thomas O'Donnell Date: Thu, 7 May 2020 17:18:44 +0200 Subject: [PATCH 3/3] Update docs following PR review --- plugins/modules/database/postgresql/postgresql_slot.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/modules/database/postgresql/postgresql_slot.py b/plugins/modules/database/postgresql/postgresql_slot.py index f2672f1305f..4a91561b3a5 100644 --- a/plugins/modules/database/postgresql/postgresql_slot.py +++ b/plugins/modules/database/postgresql/postgresql_slot.py @@ -72,7 +72,8 @@ type: str trust_input: description: - - If C(no), check whether values of some parameters are potentially dangerous. + - If C(no), check the value of I(session_role) is potentially dangerous. + - It sense to use C(no) only when SQL injections via I(session_role) are possible. type: bool default: yes