You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by "fazledyn-or (via GitHub)" <gi...@apache.org> on 2023/08/22 08:17:06 UTC
[GitHub] [libcloud] fazledyn-or opened a new issue, #1945: Bug Reported by Intelligent Code Repair in file `libcloud/compute/drivers/vsphere.py`
fazledyn-or opened a new issue, #1945:
URL: https://github.com/apache/libcloud/issues/1945
## Summary
This bug report is created by manually analyzing the source codes based on two fixes generated by Intelligent Code Repair tool (iCR).
## Detailed Information
- Python: 3.8.10
- OS: Ubuntu 20.04
# Suggested Fix 1
In your project file [libcloud/compute/drivers/vsphere.py](https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/vsphere.py) on Line 111, there’s a code segment that goes-
```py
context = ssl.create_default_context(cafile=ca_cert)
self.connection = connect.SmartConnect(
host=host,
port=port,
user=username,
pwd=password,
sslContext=context,
)
```
While triaging your repository, we noticed that the `connect.SmartConnect` method from `pyVim` library uses a method called `Connect` that calls a method called `__Login` which creates a `SoapStubAdapter` class object. A comment on that class on Line [1380 - 1384](https://github.com/vmware/pyvmomi/blob/master/pyVmomi/SoapAdapter.py#L1380) goes-
```py
# @param sslContext SSL Context describing the various SSL options. It is
# only supported in Python 2.7.9 or higher.
# if sslContext is used, load cert & key to the context with API
# sslContext = ssl.create_default_context(cafile=ca_cert_file)
# sslContext.load_cert_chain(key_file, cert_file)
```
However, in your source file the Certificate Chain isn’t loaded into `sslContext` object. We suggest that you load the certificate chain into the `sslContext` object as mentioned in the comments.
# Suggested Fix 2
In the same [file](https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/vsphere.py#L134) on Line 131 - 135, it goes-
```py
if "certificate verify failed" in error_message:
# bypass self signed certificates
try:
context = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
context.verify_mode = ssl.CERT_NONE
```
Now, it says here that the following code is to bypass the self-signed certificates. In this case, the official [documentation for ssl](https://docs.python.org/3/library/ssl.html#ssl.PROTOCOL_SSLv23) says-
> `ssl.PROTOCOL_SSLv23`
> Alias for `PROTOCOL_TLS`.
> Deprecated since version 3.6: Use `PROTOCOL_TLS` instead.
To clear the confusion, it’s suggested that you use `PROTOCOL_TLS` while instantiating the `context` object. However, if the code is used for some other reason that bypassing self-signed certificates, please let us have a discussion.
### CLA Requirements:
This section is only relevant if your project requires contributors to sign a Contributor License Agreement (CLA) for external contributions.
All contributed commits are already automatically signed off.
The meaning of a signoff depends on the project, but it typically certifies that committer has the rights to submit this work under the same license and agrees to a Developer Certificate of Origin (see https://developercertificate.org/ for more information).
- [Git Commit Sign Off documentation](https://developercertificate.org/)
### Sponsorship and Support
This work is done by the security researchers from OpenRefactory and is supported by the [Open Source Security Foundation (OpenSSF)](https://openssf.org/): [Project Alpha-Omega](https://alpha-omega.dev/). Alpha-Omega is a project partnering with open source software project maintainers to systematically find new, as-yet-undiscovered vulnerabilities in open source code - and get them fixed - to improve global software supply chain security.
The bug is found by running the iCR tool by [OpenRefactory, Inc.](https://openrefactory.com/) and then manually triaging the results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on issue #1945: Bug Reported by iCR in file `libcloud/compute/drivers/vsphere.py`
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on issue #1945:
URL: https://github.com/apache/libcloud/issues/1945#issuecomment-1688297032
Thanks for reporting this.
That code is pretty old and I'm wondering how much of it still relevant. It appears a lot of it was needed in the past due to compatibility with old Python version.
I would personally feel much more comfortable if that code could be simplified to reduce all the possible permutations and edge cases. This should reduce the surface area and make security and other issues less likely.
And in general I think that code is not following best practices. Doing something like this here (https://github.com/apache/libcloud/blob/abba8c1719a8bda6db8efde2d46fd1b423ae4304/libcloud/container/types.py#L19) and simply accepting any server certificate when that error is received seems like a bad and insecure practice to me. So if anything, that could should be removed.
If we want to leave such option in the code, it should be an explicit opt-in by the end user - they need to understand and accept the consequences if they chose to skip ca cert validation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] fazledyn-or commented on issue #1945: Bug Reported by iCR in file `libcloud/compute/drivers/vsphere.py`
Posted by "fazledyn-or (via GitHub)" <gi...@apache.org>.
fazledyn-or commented on issue #1945:
URL: https://github.com/apache/libcloud/issues/1945#issuecomment-1689352111
Hi @Kami, thanks for replying. I think we can apply the following fixes-
# Case 1
We can rewrite the `vsphere.py` file into something like this- we put option for `keyfile` and `certfile`. Then it's completely depends on the user. They may not use the certificate chain but the support is there in the library.
### [class VSphereNodeDriver](https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/vsphere.py#L80)
```py
...
context = ssl.create_default_context(cafile=ca_cert)
if certfile and keyfile:
context.load_cert_chain(certfile=certfile, keyfile=keyfile)
self.connection = connect.SmartConnect(
host=host,
port=port,
user=username,
pwd=password,
sslContext=context,
)
```
### [class VSphere_REST_NodeDriver](https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/vsphere.py#L1182)
Since `VSphereNodeDriver` is used by this class too, we add support for certificate chain here too. The updated code would look something like this-
```py
def __init__(self, key, secret=None, secure=True, host=None, port=443, ca_cert=None, certfile=None, keyfile=None):
...
if ca_cert:
self.connection.connection.ca_cert = ca_cert
else:
self.connection.connection.ca_cert = False
if certfile and keyfile:
self.connection.connection.certfile = certfile
self.connection.connection.keyfile = keyfile
else:
self.connection.connection.certfile = False
self.connection.connection.keyfile = False
```
And then use it as below-
```py
self.driver_soap = VSphereNodeDriver(
...
ca_cert=self.connection.connection.ca_cert,
certfile=self.connection.connection.certfile,
keyfile=self.connection.connection.keyfile,
)
```
# Case 2
Since I don't have the entire context about the whole project, I think simply replacing `PROTOCOL_SSLv23` with `PROTOCOL_TLS` will suffice. The fixed code would look something like this-
```py
if "certificate verify failed" in error_message:
# bypass self signed certificates
try:
context = ssl.SSLContext(ssl.PROTOCOL_TLS)
context.verify_mode = ssl.CERT_NONE
```
Please let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org