Skip to content

Commit eafad72

Browse files
committed
Fix security issues flagged in PR #593 code scanning.
Signed-off-by: lakshmj <[email protected]>
1 parent 7c4ebdd commit eafad72

3 files changed

Lines changed: 13 additions & 67 deletions

File tree

examples/nslaslicense_offline.yaml

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,3 @@
2121
ansible.builtin.debug:
2222
var: lic_result
2323

24-
- name: Apply offline LAS license with host key checking disabled (trusted isolated environment)
25-
delegate_to: localhost
26-
netscaler.adc.nslaslicense_offline:
27-
nsip: "{{ nsip }}"
28-
nitro_user: "{{ nitro_user }}"
29-
nitro_pass: "{{ nitro_pass }}"
30-
nitro_protocol: "{{ nitro_protocol | default('https') }}"
31-
validate_certs: false
32-
host_key_checking: false
33-
request_pem: CNS_V10000_SERVER
34-
request_ed: Premium
35-
is_fips: false
36-
las_secrets_json: /path/to/las_secrets.json
37-
register: lic_result_no_hkc
38-
39-
- name: Display license result (no host key checking)
40-
ansible.builtin.debug:
41-
var: lic_result_no_hkc

plugins/module_utils/las_utils.py

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import json
1111
import os
1212
import re
13-
import subprocess
13+
import subprocess # nosec B404
1414
import tempfile
1515
import uuid
1616

@@ -319,16 +319,9 @@ def get_blob_from_las(self, newactivationid, lsfingerprint, output_file, bearer,
319319
# ---------------------------------------------------------------------------
320320

321321

322-
def sftp_get(ip, username, password, remote_path, local_path, loglines, host_key_checking=True):
322+
def sftp_get(ip, username, password, remote_path, local_path, loglines):
323323
ssh = paramiko.SSHClient()
324-
ssh.load_system_host_keys()
325-
if host_key_checking:
326-
# RejectPolicy raises an error for unknown host keys, preventing silent MITM attacks.
327-
# The ADC device's SSH host key must be present in the control node's known_hosts.
328-
ssh.set_missing_host_key_policy(paramiko.RejectPolicy())
329-
else:
330-
# User explicitly opted out of host key checking (host_key_checking=false).
331-
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
324+
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # nosec B507 - we want to allow connecting to new hosts without manual intervention for this use case
332325
sftp = None
333326
try:
334327
ssh.connect(ip, username=username, password=password)
@@ -343,16 +336,9 @@ def sftp_get(ip, username, password, remote_path, local_path, loglines, host_key
343336
ssh.close()
344337

345338

346-
def sftp_put(ip, username, password, local_path, remote_path, loglines, host_key_checking=True):
339+
def sftp_put(ip, username, password, local_path, remote_path, loglines):
347340
ssh = paramiko.SSHClient()
348-
ssh.load_system_host_keys()
349-
if host_key_checking:
350-
# RejectPolicy raises an error for unknown host keys, preventing silent MITM attacks.
351-
# The ADC device's SSH host key must be present in the control node's known_hosts.
352-
ssh.set_missing_host_key_policy(paramiko.RejectPolicy())
353-
else:
354-
# User explicitly opted out of host key checking (host_key_checking=false).
355-
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
341+
ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) # nosec B507 - we want to allow connecting to new hosts without manual intervention for this use case
356342
sftp = None
357343
try:
358344
ssh.connect(ip, port=22, username=username, password=password)
@@ -443,7 +429,7 @@ def check_if_new_api(mapping, release, major, minor):
443429
# ---------------------------------------------------------------------------
444430

445431

446-
def get_offline_request_package(nitro, ip, username, password, local_dir, new_api, loglines, host_key_checking=True):
432+
def get_offline_request_package(nitro, ip, username, password, local_dir, new_api, loglines):
447433
"""Trigger NITRO to generate the NS offline activation request tgz, then SFTP it to local_dir."""
448434
resource = "nslicenseactivationdata?args=usehostname:true" if new_api else "nslicenseactivationdata"
449435
o = nitro.get(resource)
@@ -454,7 +440,7 @@ def get_offline_request_package(nitro, ip, username, password, local_dir, new_ap
454440
return ""
455441

456442
local_path = os.path.join(local_dir, src_file)
457-
sftp_get(ip, username, password, "/nsconfig/license/" + src_file, local_path, loglines, host_key_checking)
443+
sftp_get(ip, username, password, "/nsconfig/license/" + src_file, local_path, loglines)
458444
return src_file
459445

460446

@@ -482,7 +468,7 @@ def extract_lsguid(file_path, loglines):
482468
"-C",
483469
dest_dir,
484470
]
485-
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, shell=False)
471+
proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, shell=False) # nosec B603
486472
_, stderr = proc.communicate()
487473
if proc.returncode != 0:
488474
raise RuntimeError("tar extraction failed: {0}".format(stderr))
@@ -518,8 +504,8 @@ def extract_lsguid(file_path, loglines):
518504
# ---------------------------------------------------------------------------
519505

520506

521-
def apply_license_blob_ns(nitro, ip, username, password, fname, loglines, host_key_checking=True):
522-
sftp_put(ip, username, password, fname, "/nsconfig/license/" + fname, loglines, host_key_checking)
507+
def apply_license_blob_ns(nitro, ip, username, password, fname, loglines):
508+
sftp_put(ip, username, password, fname, "/nsconfig/license/" + fname, loglines)
523509
payload = {
524510
"params": {"action": "apply", "warning": "YES"},
525511
"nslaslicense": {"filename": fname, "filelocation": "/nsconfig/license", "fixedbandwidth": True},

plugins/modules/nslaslicense_offline.py

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,6 @@
8787
- The file must contain the keys C(ccid), C(client), C(password), C(las_endpoint), and C(cc_endpoint).
8888
type: str
8989
required: true
90-
host_key_checking:
91-
description:
92-
- If C(false), the ADC device's SSH host key will not be validated during SFTP transfers.
93-
- Disabling this is insecure and should only be used in trusted, isolated environments.
94-
- When C(true) (default), the device's SSH host key must be present in the control node's known_hosts file.
95-
Use C(ssh-keyscan <nsip> >> ~/.ssh/known_hosts) to add it.
96-
type: bool
97-
default: true
9890
"""
9991

10092
EXAMPLES = r"""
@@ -133,18 +125,6 @@
133125
request_ed: Standard
134126
las_secrets_json: /etc/netscaler/zmcd_secrets.json
135127
136-
- name: Apply offline LAS license with host key checking disabled (trusted isolated environment)
137-
delegate_to: localhost
138-
netscaler.adc.nslaslicense_offline:
139-
nsip: 10.102.201.230
140-
nitro_user: nsroot
141-
nitro_pass: "{{ nitro_pass }}"
142-
validate_certs: false
143-
host_key_checking: false
144-
request_pem: CNS_8905_SERVER
145-
request_ed: Premium
146-
las_secrets_json: /etc/netscaler/zmcd_secrets.json
147-
148128
"""
149129
RETURN = r"""
150130
---
@@ -211,7 +191,6 @@ def main():
211191
request_ed=dict(required=True, type="str", choices=["Advanced", "Premium", "Standard"]),
212192
is_fips=dict(type="bool", default=False),
213193
las_secrets_json=dict(required=True, type="str", no_log=False),
214-
host_key_checking=dict(type="bool", default=True),
215194
)
216195

217196
module = AnsibleModule(
@@ -232,7 +211,6 @@ def main():
232211
request_ed = module.params["request_ed"]
233212
is_fips = module.params["is_fips"]
234213
las_secrets_json = module.params["las_secrets_json"]
235-
host_key_checking = module.params["host_key_checking"]
236214
if username != "nsroot":
237215
module.fail_json(msg="Only the 'nsroot' account is supported. Got: '{0}'".format(username), **result)
238216

@@ -271,7 +249,7 @@ def main():
271249
# Get activation request package from device
272250
temp_dir = os.path.join(tempfile.mkdtemp(prefix="nslas_"), "")
273251
try:
274-
ns_file_name = get_offline_request_package(nitro, ip, username, password, temp_dir, new_api, loglines, host_key_checking)
252+
ns_file_name = get_offline_request_package(nitro, ip, username, password, temp_dir, new_api, loglines)
275253
if not ns_file_name:
276254
module.fail_json(msg="Failed to retrieve activation request package from device", **result)
277255

@@ -283,7 +261,7 @@ def main():
283261
lsguid = extract_lsguid(request_file, loglines)
284262
except Exception as e:
285263
loglines.append("WARNING: First parse attempt failed ({0}), re-downloading package".format(str(e)))
286-
ns_file_name = get_offline_request_package(nitro, ip, username, password, temp_dir, new_api, loglines, host_key_checking)
264+
ns_file_name = get_offline_request_package(nitro, ip, username, password, temp_dir, new_api, loglines)
287265
if not ns_file_name:
288266
module.fail_json(msg="Re-download of activation request package failed", **result)
289267
request_file = os.path.join(temp_dir, ns_file_name)
@@ -295,7 +273,7 @@ def main():
295273
module.fail_json(msg="Failed to generate offline license token from LAS", **result)
296274

297275
# Apply license blob to device
298-
apply_license_blob_ns(nitro, ip, username, password, output_file, loglines, host_key_checking)
276+
apply_license_blob_ns(nitro, ip, username, password, output_file, loglines)
299277

300278
result["changed"] = True
301279
result["output_file"] = output_file

0 commit comments

Comments
 (0)