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

lib389 should use filter for tarfile as recommended by PEP 706 - #5867

Closed
progier389 opened this issue Jul 28, 2023 · 3 comments · Fixed by #5868
Closed

lib389 should use filter for tarfile as recommended by PEP 706 - #5867

progier389 opened this issue Jul 28, 2023 · 3 comments · Fixed by #5868
Assignees
Labels
In JIRA ticket is in JIRA lib389 Involves lib389 librabry
Milestone

Comments

@progier389
Copy link
Contributor

progier389 commented Jul 28, 2023

Issue Description
Should add tarfile filter every time tarfile.open is called. (To avoid a deprecation warning.)
The impacted locations are:

  1. lib389.backupFS
  2. lib389.restoreFS
  3. lib389.tools.instanceBackupFS
  4. lib389.tools.instanceRestoreFS
  5. dirsrvtests/tests/tickets/ticket47988_test.py

In practice it is more about filling a checkbox than a real issue as the impacted lib389 functions
are not used within 389ds nor within freeipa
(IMHO that is plain dead code, BTW this sound logical because backup/restore is directly handled by the ns-slapd binary
and lib389 should only act as a wrapper to the online and offline backup/restore tasks )

So I will add:
tar.extraction_filter = (lambda member, path: member)
after each call to tarfile.open

See: BZ 2207691

Package Version and Platform:

  • Platform: Fedora, RHEL 8
  • Package and version: main

Steps to Reproduce
No reproducer:
seen by code review whether tar.extraction_filter is set after tar = tarfile.open(...)
Result when issue is not fixed:

find . -type f | xargs egrep -n "extraction_filter|tarfile.open" /dev/null 
./src/lib389/lib389/tools.py:303:        tar = tarfile.open(backup_file, "w:gz")
./src/lib389/lib389/tools.py:361:        tar = tarfile.open(backup_file)
./src/lib389/lib389/__init__.py:1380:        tar = tarfile.open(backup_file, "w:gz")
./src/lib389/lib389/__init__.py:1453:        tar = tarfile.open(backup_file)
./dirsrvtests/tests/tickets/ticket47988_test.py:84:    tar = tarfile.open(tarFile, 'r:gz')

Expected result:

find . -type f | xargs egrep -n "extraction_filter|tarfile.open" /dev/null 
./src/lib389/lib389/tools.py:303:        tar = tarfile.open(backup_file, "w:gz")
./src/lib389/lib389/tools.py:304:        tar.extraction_filter = (lambda member, path: member)
./src/lib389/lib389/tools.py:362:        tar = tarfile.open(backup_file)
./src/lib389/lib389/tools.py:363:        tar.extraction_filter = (lambda member, path: member)
./src/lib389/lib389/__init__.py:1380:        tar = tarfile.open(backup_file, "w:gz")
./src/lib389/lib389/__init__.py:1381:        tar.extraction_filter = (lambda member, path: member)
./src/lib389/lib389/__init__.py:1454:        tar = tarfile.open(backup_file)
./src/lib389/lib389/__init__.py:1455:        tar.extraction_filter = (lambda member, path: member)
./dirsrvtests/tests/tickets/ticket47988_test.py:84:    tar = tarfile.open(tarFile, 'r:gz')
./dirsrvtests/tests/tickets/ticket47988_test.py:85:    tar.extraction_filter = (lambda member, path: member)

@vashirov
Copy link
Member

In practice it is more about filling a checkbox than a real issue as the impacted lib389 functions
are not used within 389ds nor within freeipa
(IMHO that is plain dead code, BTW this sound logical because backup/restore is directly handled by the ns-slapd binary
and lib389 should only act as a wrapper to the online and offline backup/restore tasks )

I checked git log and found that these functions were used to speed up testing:
#921
b5294b4

These days we use pytest fixtures and recreate instances for each test. So this is indeed a dead code. Maybe we should delete it?

@progier389
Copy link
Contributor Author

I wondered the same before creating the PR but I always hesitate to remove functions outside of a major version change. ( we know that some other projects (like freeipa) use directly lib389 and (as far as I know) there is no clear documentation of the public API so we may break something.
My feeling is that we should only solve the problem that we have now and postpone the functions removal
That said we may want to create an issue to avoid forgetting it ...

progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
progier389 added a commit that referenced this issue Aug 1, 2023
…EP 706 (#5868)

Problem:
tarfile interface evolved after CVE-2007-4559 and using object generated by tarfile.open without setting explicitly a filter has been obsoleted.

Solution:
Add an extraction_filter after every tarfile.open call

**Issue:** [5867](#5867)

**Reviewed by:**  @droideck  Thanks !

(cherry picked from commit 92cc2b1)
@progier389
Copy link
Contributor Author

96959cf..92cc2b1 main -> main
b99ee8e..3b43a7b 389-ds-base-2.3 -> 389-ds-base-2.3
15aa5e8..5e49564 389-ds-base-2.2 -> 389-ds-base-2.2
687765f..98b5df6 389-ds-base-2.1 -> 389-ds-base-2.1
3f1f1e2..3470b8e 389-ds-base-2.0 -> 389-ds-base-2.0
86c5db2..66106c8 389-ds-base-1.4.4 -> 389-ds-base-1.4.4
b6d160d..7e07c95 389-ds-base-1.4.3 -> 389-ds-base-1.4.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In JIRA ticket is in JIRA lib389 Involves lib389 librabry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants