Bug 1956750 - Python oVirt SDK overwrites /dev/null with cookie file
Summary: Python oVirt SDK overwrites /dev/null with cookie file
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: ovirt-engine-sdk-python
Classification: oVirt
Component: General
Version: 4.4.11
Hardware: Unspecified
OS: Unspecified
medium
high
Target Milestone: ovirt-4.4.7
: ---
Assignee: Ori Liel
QA Contact: Guilherme Santos
URL:
Whiteboard:
: 1939387 1953476 (view as bug list)
Depends On:
Blocks: 1939387
TreeView+ depends on / blocked
 
Reported: 2021-05-04 11:02 UTC by Richard W.M. Jones
Modified: 2022-03-03 11:04 UTC (History)
18 users (show)

Fixed In Version: python3-ovirt-engine-sdk4-4.4.13
Doc Type: No Doc Update
Doc Text:
Clone Of: 1939387
Environment:
Last Closed: 2021-07-06 07:28:06 UTC
oVirt Team: Infra
Embargoed:
pm-rhel: ovirt-4.4+
lsvaty: blocker-
gdeolive: testing_ack+


Attachments (Terms of Use)
ovirt-engine-sdk-python-4.4.10-remove-bogus-cookiejar.patch (658 bytes, patch)
2021-05-05 09:19 UTC, Richard W.M. Jones
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 114690 0 master MERGED Preserve /dev/null when running under root user 2021-05-12 09:54:28 UTC

Description Richard W.M. Jones 2021-05-04 11:02:37 UTC
+++ This bug was initially created as a clone of Bug #1939387 +++

Description of problem:
'/dev/null' is changed to regular file unexpectedly

Version-Release number of selected component (if applicable):
virt-v2v-1.43.3-2.el9.x86_64
kernel-5.11.0-2.el9.x86_64
nbdkit-1.25.2-1.el9.x86_64
libguestfs-1.44.0-5.el9.x86_64
libvirt-7.0.0-4.el9.x86_64
qemu-kvm-5.2.0-7.el9.x86_64
python3-3.9.2-1.el9.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Run virt-v2v command
virt-v2v -ic vpx://root.x.x/data/x.x.x.x/?no_verify=1 -o rhv-upload -of raw -os nfs_data -oc https://x.x.x.x/ovirt-engine/api -op /root/rhv_upload_passwd_file -oo rhv-cluster=NFS -oo rhv-verifypeer=false -oo rhv-direct -b ovirtmgmt esx6.5-win10-x86_64 -it vddk -io vddk-libdir=/root/vddk_libdir/latest -io vddk-thumbprint=x:x:x -on esx6.5-win10-x86_64p86 -ip /root/v2v_vpx_passwd -v -x
2. Check the /dev/null file.

Actual results:
# ll /dev/null -Z
-rw-r--r--. 1 root root unconfined_u:object_r:device_t:s0 60 Mar 16 04:48 /dev/null

Expected results:
# ll /dev/null -Z
crw-rw-rw-. 1 root root unconfined_u:object_r:null_device_t:s0 1, 3 Mar 16 04:41 /dev/null

Additional info:
This problem doesn't happen on rhel8.


See upstream bug for details and fix:
https://github.com/oVirt/ovirt-engine-sdk/issues/34

Comment 1 Richard W.M. Jones 2021-05-05 09:19:27 UTC
Created attachment 1779700 [details]
ovirt-engine-sdk-python-4.4.10-remove-bogus-cookiejar.patch

The fix is simply to remove the two lines setting COOKIEFILE
and COOKIEJAR.  Both lines are incorrect.

Comment 2 Martin Perina 2021-05-05 11:56:40 UTC
*** Bug 1953476 has been marked as a duplicate of this bug. ***

Comment 3 RHEL Program Management 2021-05-05 11:58:02 UTC
This bug report has Keywords: Regression or TestBlocker.
Since no regressions or test blockers are allowed between releases, it is also being identified as a blocker for this release. Please resolve ASAP.

Comment 4 Martin Perina 2021-05-06 12:14:48 UTC
This is not a blocker for RHV 4.4.6, the code is there from RHV 4.0, we will try to fix in RHV 4.4.7

Comment 5 Richard W.M. Jones 2021-05-07 08:59:12 UTC
(In reply to Martin Perina from comment #4)
> This is not a blocker for RHV 4.4.6, the code is there from RHV 4.0, we will
> try to fix in RHV 4.4.7

This is a serious bug that affects virt-v2v when run as root (actually damaging
the hosts that virt-v2v runs on), and the change is a two line fix that has
already been tested by QE.  Please fix it.

Comment 6 Martin Perina 2021-05-07 10:03:05 UTC
(In reply to Richard W.M. Jones from comment #5)
> (In reply to Martin Perina from comment #4)
> > This is not a blocker for RHV 4.4.6, the code is there from RHV 4.0, we will
> > try to fix in RHV 4.4.7
> 
> This is a serious bug that affects virt-v2v when run as root (actually
> damaging
> the hosts that virt-v2v runs on), and the change is a two line fix that has
> already been tested by QE.  Please fix it.

Those 2 particular lines has been added in RHV 4.1 to support asynchronous request (more info in BZ1436981). RHV 4.4.6 is now in blocker only mode, meaning that this change would mean that QE would need to reverify the whole asynchronous request feature.
Lucie, are you OK to do that?

Comment 7 Richard W.M. Jones 2021-05-07 10:07:16 UTC
Is that true?  The change you're referring to just moves the incorrect code,
it doesn't add those:

https://gerrit.ovirt.org/c/ovirt-engine-sdk/+/76041/5/sdk/lib/ovirtsdk4/__init__.py

Comment 8 Martin Perina 2021-05-07 10:17:30 UTC
(In reply to Martin Perina from comment #6)
> (In reply to Richard W.M. Jones from comment #5)
> > (In reply to Martin Perina from comment #4)
> > > This is not a blocker for RHV 4.4.6, the code is there from RHV 4.0, we will
> > > try to fix in RHV 4.4.7
> > 
> > This is a serious bug that affects virt-v2v when run as root (actually
> > damaging
> > the hosts that virt-v2v runs on), and the change is a two line fix that has
> > already been tested by QE.  Please fix it.
> 
> Those 2 particular lines has been added in RHV 4.1 to support asynchronous
> request (more info in BZ1436981). RHV 4.4.6 is now in blocker only mode,
> meaning that this change would mean that QE would need to reverify the whole
> asynchronous request feature.
> Lucie, are you OK to do that?

Correction, so actually the code around COOKIEFILE and COOKIEJAR is in the Python SDK since 2014, when we switched to pycurl:

https://gerrit.ovirt.org/33064

Comment 9 Guilherme Santos 2021-05-10 11:57:11 UTC
Hi Martin, with the fixed merged, we can test it for 4.4.6 0day. We can run a patch with the examples used to verify BZ1436981 and other sdk functionalities and verify based on that

Comment 10 Ori Liel 2021-05-10 12:10:05 UTC
cookie-file' and 'cookie-jar' were set to /dev/null in order to make libcurl store cookies only in memory, rather than be written to a file (this is done for security reasons, as sometimes these files are not cleaned-up properly, for example in case of a crash in the system).

Removing this configuration will revert to the default behavior of the library, which is that cookies are disabled. I'm in the process of understanding the implications of this, but it's not trivial. For example, Engine old implementation of session-based authentication relies on cookies, so clients scripts with session-based authentication making requests to old versions of the Engine (I'm trying to determine how old) are expected to break.

I don't have a ready environment to test this, but I have reason to believe that removing only the cookie-jar line, and leaving the cookie-file line as is, can both preserve the current mode of working with cookies and solve the bug. Is that something that can be quickly tested? 

(thanks to Juan Hernandez (@jhernand), author of the python-sdk, for helping me with this infomration)

Comment 11 Richard W.M. Jones 2021-05-10 13:42:35 UTC
Setting these to /dev/null was not the correct way to do this.  The
documentation of curl is very confusing, I found it was better to go
straight to the sources, but this is what you need to do:

- Do not set COOKIEJAR at all.

- Set COOKIEFILE to the empty string "" soon after creating the handle.
  This enables cookie processing.

- As long as you never set COOKIEJAR and never use COOKIELIST, no
  cookies will be written out.

You can test this using a simple Python script:

--------------
#!/usr/bin/python3
import pycurl

c = pycurl.Curl()
c.setopt(pycurl.COOKIEFILE, "")

# This URL is chosen because it redirects and sets cookies.
c.setopt(pycurl.URL, "http://ibm.com")
c.setopt(pycurl.FOLLOWLOCATION, 1)

# Enable verbose output so we can see what it's doing.
c.setopt(pycurl.VERBOSE, 1)

c.perform()

Comment 13 Guilherme Santos 2021-07-02 10:46:52 UTC
Verified on:
ovirt-engine-sdk-python==4.4.13

Steps:
1. # ll -Z /dev/null 
crw-rw-rw-. 1 root root system_u:object_r:null_device_t:s0 1, 3 Jul  2 12:48 /dev/null

2. Run script used to verify the sync bug  BZ1436981 mention on comment #6
3.  # ll -Z /dev/null 
crw-rw-rw-. 1 root root system_u:object_r:null_device_t:s0 1, 3 Jul  2 12:48 /dev/null

4. Run valitation script provided in comment #11
5.  # ll -Z /dev/null 
crw-rw-rw-. 1 root root system_u:object_r:null_device_t:s0 1, 3 Jul  2 12:48 /dev/null

Results:
ovirt-sdk not screwing /dev/null and working properly as expected

Comment 14 Sandro Bonazzola 2021-07-06 07:28:06 UTC
This bugzilla is included in oVirt 4.4.7 release, published on July 6th 2021.

Since the problem described in this bug report should be resolved in oVirt 4.4.7 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.

Comment 15 Xiaodai Wang 2022-03-03 11:04:47 UTC
*** Bug 1939387 has been marked as a duplicate of this bug. ***


Note You need to log in before you can comment on or make changes to this bug.