Skip to content
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

Closes #138 IOS-XE certificate validation #139

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

netgab
Copy link
Contributor

@netgab netgab commented Feb 5, 2025

Added the option to control certificate validation, to the device connection as:

devices:
  eWLC:
        os: iosxe
        type: eWLC
        custom:
          abstraction:
            order: [os]
        connections:
          rest:
            class: rest.connector.Rest
            ip: 198.51.100.3
            #verify: True                  # <-- Enable certificate validation (default)
            verify: False                  # <-- Disable certificate validation

This setting is applied to the requests.session object once, which is applied to all following operations like GET, POST etc.

@netgab netgab requested a review from a team as a code owner February 5, 2025 14:26
@netgab netgab requested review from Taarini and omehrabi February 5, 2025 14:26
@netgab
Copy link
Contributor Author

netgab commented Feb 5, 2025

I guess the pipeline fails, because Python 3.8 is EoL
https://devguide.python.org/versions/

The runners don't support Python 3.8

Error: Version 3.8 with arch x64 not found
Available versions:

3.10.16 (x64)
3.11.11 (x64)
3.12.8 (x64)
3.13.1 (x64)
3.9.21 (x64)

Should the file CiscoTestAutomation-rest/.github/workflows/run_tests.yml also be modified, to exclude python-version 3.8?

@@ -96,6 +96,7 @@ def connect(self,
log.debug("Timeout: %s" % timeout)
self.content_type = default_content_type

self.verify = self.connection_info.get('verify', True)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this default to False?

Suggested change
self.verify = self.connection_info.get('verify', True)
self.verify = self.connection_info.get('verify', False)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of today, it defaults to True for the get requests (as no verify parameter is passed there).
Furthermore, I guess a good security practice is, to enable SSL/TLS certificate validation by default and only disable it, if there are specific reasons.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't argue, even if test tools often interact with lab devices where certificate discipline is often, err, sloppy..
but in the PR description you mentioned

            verify: False                  # <-- Disable certificate validation (default)            

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @oboehmer .. my bad. I updated the PR description above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants