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

feat: Introducing support for VMware module without SSL validation #38

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cristianocasella
Copy link

Hello everyone,
this PR add the support for a VMware virtual host manager skipping the TLS validation.

As many VMware are using self-signed certificate, this reduce the complexity of the setup.

Tested on Uyuni 2024.05.

This topic has been discussed also here: https://lists.opensuse.org/archives/list/[email protected]/message/EHPKTVYUEFCPR7DS5PDLEL7DI56DDO2J/

Copy link
Member

@meaksh meaksh left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution!

Let's try to avoid having duplicated code between the two VMware modules, as they share most of it.

I would consider the new VMwareNoSSL.py module file as something like:

import atexit

from gatherer.modules.VMware import VMware

    
try:
    from pyVim.connect import SmartConnectNoSSL, Disconnect
    IS_VALID = True
except ImportError as ex:
    IS_VALID = False

class VMwareNoSSL(VMware):
    def run(self):
        ...

This way we would just need to redefine the run function to use the SmartConnectNoSSL. How does this sound to you?

One more thing that would be needed here is to add this new file to be included as part of the virtual-host-gatherer-VMware subpackage: https://github.com/uyuni-project/virtual-host-gatherer/blob/master/virtual-host-gatherer/virtual-host-gatherer.spec#L152 and also a new changelog entry.

You can take some inspiration from: #5

@cristianocasella could you please consider this? Thanks!

@cristianocasella
Copy link
Author

Hello @meaksh, really thanks for the quick feedback and reply.

I'm fine implementing the logic that you suggested, my initial concern regarding this approach is that this is silently ignoring SSL validation for everyone (or at least you lose the control on it). This could be a security issue for many users, preventing them from adopting the library/tool.

There is any "easy" way to add a flag into the UI so that we manage the SSL skip with a parameter? Instead that going for attempt?

@meaksh
Copy link
Member

meaksh commented Jun 24, 2024

Hello @meaksh, really thanks for the quick feedback and reply.

I'm fine implementing the logic that you suggested, my initial concern regarding this approach is that this is silently ignoring SSL validation for everyone (or at least you lose the control on it). This could be a security issue for many users, preventing them from adopting the library/tool.

Hey @cristianocasella, I totally agree on your concerns, but what I'm suggesting here is to have 2 different separated modules, VMware (with SSL validation) and VMwareNoSSL (without it), each one on a different file, so the user can choose between them. It is just that most of the code for VMwareNoSSL is being reused from the VMware module.

There is any "easy" way to add a flag into the UI so that we manage the SSL skip with a parameter? Instead that going for attempt?

I was also initially thinking on introducing a parameter for the VMware module, like disable_ssl checkbox or similar. That is definetely also a valid and simpler approach but AFAIK it would requiere also some changes on the Java side, as currently all parameters are considered strings.

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