-
Notifications
You must be signed in to change notification settings - Fork 48
New validation for CFD CSCwf26626 #303
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
Closed
Harinadh-Saladi
wants to merge
10
commits into
datacenter:master
from
Harinadh-Saladi:cscwf26626_hsaladi
Closed
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
11a7f81
Added disabled_cipher_check for CSCwf26626
Harinadh-Saladi 2ddec88
Fix fabricNode.json: Replace leaf node with third APIC controller (ap…
Harinadh-Saladi 75fde2d
Updated test data JSON files to array format with full cipher attributes
Harinadh-Saladi 3145419
Added additional test cases for disabled_cipher_check and modified va…
Harinadh-Saladi 9c9a5d8
Fixed nginx log check: Filtered out command echo and prompt correctly
Harinadh-Saladi 2ddbe6f
Updated disabled_cipher_check with nginx log filtering fix
Harinadh-Saladi c156f7d
Removed break from line number 1608 as it was added to proceed with t…
Harinadh-Saladi 6cb1209
Merging latest master changes
Harinadh-Saladi 60cebbb
Remove merge conflict markers from previous commit
Harinadh-Saladi 79fba88
Removed target version missing check since when tversion is not provi…
Harinadh-Saladi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5970,6 +5970,121 @@ def configpush_shard_check(tversion, **kwargs): | |
|
|
||
| return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) | ||
|
|
||
| <<<<<<< HEAD | ||
| @check_wrapper(check_title="Disabled Cipher Configuration") | ||
| def disabled_cipher_check(tversion, username, password, fabric_nodes, **kwargs): | ||
| headers = ["APIC", "Disabled Cipher Count", "Nginx Log Check Status"] | ||
| data = [] | ||
| recommended_action = "Re-enable the disabled ciphers or contact Cisco TAC for guidance on cipher configuration" | ||
| doc_url = "https://datacenter.github.io/ACI-Pre-Upgrade-Validation-Script/validations/#disabled-cipher-configuration" | ||
|
|
||
| # Check 1: Verify target version is 6.0.2 | ||
| if not tversion: | ||
| return Result(result=MANUAL, msg=TVER_MISSING) | ||
|
|
||
| if not (tversion.same_as("6.0(2a)") or tversion.same_as("6.0(2h)") or tversion.same_as("6.0(2j)")): | ||
| return Result(result=NA, msg=VER_NOT_AFFECTED) | ||
|
|
||
| # Check 2: Query for disabled ciphers | ||
| cipher_api = "commCipher.json?query-target-filter=and(or(wcard(commCipher.id,\"ECDHE-RSA\"),wcard(commCipher.id,\"DHE-RSA\"),wcard(commCipher.id,\"TLS_AES_256\")),eq(commCipher.state,\"disabled\"))" | ||
| try: | ||
| disabled_ciphers = icurl("class", cipher_api) | ||
| disabled_cipher_count = len(disabled_ciphers) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Lovkesh, The entire codebase uses the same pattern - working directly with the array returned by icurl function. |
||
| except Exception as e: | ||
| log.error("Failed to query disabled ciphers: %s", str(e)) | ||
| return Result(result=ERROR, msg="Failed to query disabled ciphers: {}".format(str(e)), doc_url=doc_url) | ||
|
|
||
| if disabled_cipher_count == 0: | ||
| return Result(result=PASS, msg="No disabled ciphers found", doc_url=doc_url) | ||
|
|
||
| # Check 3: SSH to all APICs and check nginx logs | ||
| controllers = [node for node in fabric_nodes if node["fabricNode"]["attributes"]["role"] == "controller"] | ||
|
|
||
| if not controllers: | ||
| return Result(result=ERROR, msg="No controllers found in fabricNode. Is the cluster healthy?", doc_url=doc_url) | ||
|
|
||
| prints("") | ||
|
|
||
| for apic in controllers: | ||
| attr = apic["fabricNode"]["attributes"] | ||
| apic_name = attr["name"] | ||
| node_title = "Checking %s..." % apic_name | ||
| prints(node_title, end=" ") | ||
|
|
||
| try: | ||
| c = Connection(attr["address"]) | ||
| c.username = username | ||
| c.password = password | ||
| c.log = LOG_FILE | ||
| c.connect() | ||
| except Exception as e: | ||
| log.error("Connection failed to %s: %s", apic_name, str(e)) | ||
| data.append([apic_name, str(disabled_cipher_count), "Connection Error: {}".format(str(e))]) | ||
| prints(ERROR) | ||
| continue | ||
|
|
||
| try: | ||
| cmd = "zgrep \"Failed to write nginxproxy conf file\" /var/log/dme/log/nginx.bin.war* 2>/dev/null | head -20" | ||
| c.cmd(cmd, timeout=35) | ||
|
|
||
| # Log raw output for debugging | ||
| log.debug("APIC %s raw output: %s", apic_name, repr(c.output)) | ||
|
|
||
| # Check if output contains actual error messages | ||
| # If zgrep finds matches, the output will have error messages before the prompt | ||
| # If zgrep finds nothing, the output will only have the command echo and prompt | ||
| # Look for lines between command and prompt that contain the error message | ||
| lines = c.output.split("\n") | ||
| found_error = False | ||
|
|
||
| for line in lines: | ||
| # Skip the command echo line and prompt line | ||
| if "zgrep" in line or line.strip().endswith("#"): | ||
| continue | ||
| # If this line contains the error message and it's not empty, we found it | ||
| if line.strip() and "Failed to write nginxproxy conf file" in line: | ||
| found_error = True | ||
| log.debug("APIC %s found error line: %s", apic_name, repr(line)) | ||
| break | ||
|
|
||
| if found_error: | ||
| data.append([apic_name, str(disabled_cipher_count), "FOUND"]) | ||
| log.debug("APIC %s: Nginx error FOUND in logs", apic_name) | ||
| else: | ||
| data.append([apic_name, str(disabled_cipher_count), "Not found in nginx logs"]) | ||
| log.debug("APIC %s: Nginx error NOT found in logs (only prompt returned)", apic_name) | ||
|
|
||
| prints(DONE) | ||
| except pexpect.TIMEOUT: | ||
| log.warning("Command timeout on %s", apic_name) | ||
| data.append([apic_name, str(disabled_cipher_count), "Command Timeout"]) | ||
| prints(ERROR) | ||
| except pexpect.EOF: | ||
| log.warning("Connection closed unexpectedly on %s", apic_name) | ||
| data.append([apic_name, str(disabled_cipher_count), "Connection Closed"]) | ||
| prints(ERROR) | ||
| except Exception as e: | ||
| log.exception("Error checking nginx logs on %s", apic_name) | ||
| data.append([apic_name, str(disabled_cipher_count), "Error: {}".format(str(e))]) | ||
| prints(ERROR) | ||
| finally: | ||
| try: | ||
| c.close() | ||
| except Exception: | ||
| pass | ||
|
|
||
| # Determine final result based on priority: FAIL_O > ERROR > PASS | ||
| if not data: | ||
| return Result(result=ERROR, msg="Unable to check nginx logs on any APIC", headers=headers, data=[], doc_url=doc_url) | ||
|
|
||
| if any("FOUND" in row[2] for row in data): | ||
| result = FAIL_O | ||
| elif any("Error" in row[2] or "Timeout" in row[2] or "Closed" in row[2] for row in data): | ||
| result = ERROR | ||
| else: | ||
| result = PASS | ||
| return Result(result=result, headers=headers, data=data, recommended_action=recommended_action, doc_url=doc_url) | ||
| ======= | ||
|
|
||
| @check_wrapper(check_title='APIC VMM inventory sync fault (F0132)') | ||
| def apic_vmm_inventory_sync_faults_check(**kwargs): | ||
|
|
@@ -6006,6 +6121,7 @@ def apic_vmm_inventory_sync_faults_check(**kwargs): | |
| unformatted_data=unformatted_data, | ||
| recommended_action=recommended_action, | ||
| doc_url=doc_url) | ||
| >>>>>>> upstream/master | ||
|
|
||
| # ---- Script Execution ---- | ||
|
|
||
|
|
@@ -6168,6 +6284,7 @@ class CheckManager: | |
| standby_sup_sync_check, | ||
| isis_database_byte_check, | ||
| configpush_shard_check, | ||
| disabled_cipher_check, | ||
|
|
||
| ] | ||
| ssh_checks = [ | ||
|
|
@@ -6312,7 +6429,7 @@ def main(_args=None): | |
| # Print result of each failed check | ||
| for index, check_id in enumerate(cm.check_ids): | ||
| result_obj = cm.get_check_result(check_id) | ||
| if not result_obj or result_obj.result in (NA, PASS): | ||
| if not result_obj or result_obj.result in (NA,PASS): | ||
| continue | ||
| check_title = cm.get_check_title(check_id) | ||
| print_result(index + 1, cm.total_checks, check_title, **result_obj.as_dict()) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
tests/checks/disabled_cipher_check/commCipher_disabled_ciphers.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| { | ||
| "totalCount": "2", | ||
| "imdata": [ | ||
| { | ||
| "commCipher": { | ||
| "attributes": { | ||
| "annotation": "", | ||
| "childAction": "", | ||
| "configurable": "yes", | ||
| "dn": "uni/fabric/comm-default/https/cph-ECDHE-RSA-AES256-SHA384", | ||
| "extMngdBy": "", | ||
| "id": "ECDHE-RSA-AES256-SHA384", | ||
| "lcOwn": "local", | ||
| "modTs": "2025-12-09T12:07:57.857+00:00", | ||
| "state": "disabled", | ||
| "status": "", | ||
| "uid": "0", | ||
| "userdom": "all", | ||
| "weak": "yes" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "commCipher": { | ||
| "attributes": { | ||
| "annotation": "", | ||
| "childAction": "", | ||
| "configurable": "yes", | ||
| "dn": "uni/fabric/comm-default/https/cph-ECDHE-RSA-AES128-SHA256", | ||
| "extMngdBy": "", | ||
| "id": "ECDHE-RSA-AES128-SHA256", | ||
| "lcOwn": "local", | ||
| "modTs": "2025-12-09T12:07:57.857+00:00", | ||
| "state": "disabled", | ||
| "status": "", | ||
| "uid": "0", | ||
| "userdom": "all", | ||
| "weak": "yes" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } |
4 changes: 4 additions & 0 deletions
4
tests/checks/disabled_cipher_check/commCipher_no_disabled_ciphers.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "totalCount": "0", | ||
| "imdata": [] | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| [ | ||
| { | ||
| "fabricNode": { | ||
| "attributes": { | ||
| "dn": "topology/pod-1/node-1", | ||
| "address": "10.0.0.1", | ||
| "fabricSt": "active", | ||
| "id": "1", | ||
| "name": "apic1", | ||
| "role": "controller" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "fabricNode": { | ||
| "attributes": { | ||
| "dn": "topology/pod-1/node-2", | ||
| "address": "10.0.0.2", | ||
| "fabricSt": "active", | ||
| "id": "2", | ||
| "name": "apic2", | ||
| "role": "controller" | ||
| } | ||
| } | ||
| }, | ||
| { | ||
| "fabricNode": { | ||
| "attributes": { | ||
| "dn": "topology/pod-1/node-3", | ||
| "address": "10.0.0.3", | ||
| "fabricSt": "active", | ||
| "id": "3", | ||
| "name": "apic3", | ||
| "role": "controller" | ||
| } | ||
| } | ||
| } | ||
| ] | ||
|
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we have <<<<<< HEAD ?