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

mikrotikapi.py: Add SSL cert verification #396

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arfoll
Copy link
Contributor

@arfoll arfoll commented Jan 14, 2025

Proposed change

This only adds the minimal ssl cert verification, disabling the hostname check which I think is not so useful anwyays on internal routers/switches which commonly will not have externally routable hostnames and disables the strict checking to allow easier use of self made CAs without intermediate CAs restricting key usage etc...

Type of change

  • Bugfix
  • New feature
  • Code quality improvements to existing code or addition of tests
  • Documentation

One could argue not verifying SSL certs is a bug but I guess this was done on purpose and I didn't hide it behind a config flag...

Additional information

Checklist

  • The code change is tested and works locally.
  • The code has been formatted using Black.
  • Tests have been added to verify that the new code works.
  • Documentation added/updated if required.

This only adds the minimal ssl cert verification, disabling the hostname check
which I think is not so useful anwyays on internal routers/switches which
commonly will not have externally routable hostnames and disables the strict
checking to allow easier use of self made CAs without intermediate CAs
restricting key usage etc...
@arfoll
Copy link
Contributor Author

arfoll commented Jan 14, 2025

The black verification fails do not seem related to my PR, it's passing on the mikrotikapi.py entirely with or without my changes

Copy link
Owner

tomaae commented Jan 15, 2025

What do you mean about minimal cert verification?
Right now it is set to allow encrypted communication for people who want that extra layer.
We obviously dont want hostname verification for network devices, no argument there.

@arfoll
Copy link
Contributor Author

arfoll commented Jan 15, 2025 via email

Copy link
Owner

tomaae commented Jan 16, 2025

It is doable, but it would probably break many current setups.
This would require "verify certificate" checkbox option on setup page to turn this on.

Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants