ENH: Add AES support for encrypting PDF files (merged via other PRs) #1816

exiledkingcc · 2023-04-27T13:55:58Z

codecov · 2023-04-28T04:59:32Z

Codecov Report

Patch coverage: 92.45% and project coverage change: -0.01 ⚠️

Comparison is base (2b47d9a) 93.89% compared to head (712acb2) 93.89%.

❗ Current head 712acb2 differs from pull request most recent head a7cfdb4. Consider uploading reports for the commit a7cfdb4 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1816      +/-   ##
==========================================
- Coverage   93.89%   93.89%   -0.01%     
==========================================
  Files          33       33              
  Lines        6983     6946      -37     
  Branches     1382     1374       -8     
==========================================
- Hits         6557     6522      -35     
  Misses        274      274              
+ Partials      152      150       -2

Impacted Files	Coverage Δ
pypdf/xmp.py	`98.67% <50.00%> (ø)`
pypdf/generic/_data_structures.py	`91.09% <69.23%> (ø)`
pypdf/_encryption.py	`94.95% <92.55%> (ø)`
pypdf/_protocols.py	`100.00% <100.00%> (ø)`
pypdf/_writer.py	`88.34% <100.00%> (-0.09%)`	⬇️
pypdf/generic/_base.py	`100.00% <100.00%> (ø)`
pypdf/generic/_outline.py	`100.00% <100.00%> (ø)`

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

eliminate redundant `Encrypt` entries

MartinThoma · 2023-04-29T19:20:43Z

pypdf/_writer.py

-        user_pwd: Optional[str] = None,  # deprecated
-        owner_pwd: Optional[str] = None,  # deprecated


Those two need to stay as long as we don't make a major version bump. As still many users are migrating from PyPDF2 (sometimes even PyPDF2 < 2.0.0), I want to keep the warnings for quite a while.

MartinThoma · 2023-04-29T19:21:37Z

pypdf/_writer.py

-        warnings.warn(
-            "pypdf only implements RC4 encryption so far. "
-            "The RC4 algorithm is insecure. Either use a library that supports "
-            "AES for encryption or put the PDF in an encrypted container, "
-            "for example an encrypted ZIP file."
-        )
-        if user_pwd is not None:
-            if user_password is not None:
-                raise ValueError(
-                    "Please only set 'user_password'. "
-                    "The 'user_pwd' argument is deprecated."
-                )
-            else:
-                warnings.warn(
-                    "Please use 'user_password' instead of 'user_pwd'. "
-                    "The 'user_pwd' argument is deprecated and "
-                    "will be removed in pypdf 4.0.0."
-                )
-                user_password = user_pwd
        if user_password is None:  # deprecated
            # user_password is only Optional for due to the deprecated user_pwd
            raise ValueError("user_password may not be None")

-        if owner_pwd is not None:  # deprecated
-            if owner_password is not None:
-                raise ValueError(
-                    "The argument owner_pwd of encrypt is deprecated. "
-                    "Use owner_password only."
-                )
-            else:
-                old_term = "owner_pwd"
-                new_term = "owner_password"
-                warnings.warn(
-                    message=(
-                        f"{old_term} is deprecated as an argument and will be "
-                        f"removed in pypdf 4.0.0. Use {new_term} instead"
-                    ),
-                    category=DeprecationWarning,
-                )
-                owner_password = owner_pwd
-


Sadly, those need to stay for the same reason: Keeping helpful deprection messages for users

MartinThoma · 2023-04-29T19:23:12Z

@exiledkingcc Thank you so much for picking this up 🙏

I'm currently quite busy with my private live, but I try to review everything tomorrow :-)

MartinThoma · 2023-04-29T19:23:56Z

pypdf/xmp.py

-    def writeToStream(
-        self, stream: StreamType, encryption_key: Union[None, str, bytes]
-    ) -> None:  # deprecated
-        """
-        Use :meth:`write_to_stream` instead.
-
-        .. deprecated:: 1.28.0
-        """
-        deprecation_with_replacement("writeToStream", "write_to_stream", "3.0.0")
-        self.write_to_stream(stream, encryption_key)


This needs to stay to help migrating users

@MartinThoma
this depreciation is quite old and it also applies to internal objects. I think we could try to delete it. It will be possible to restore it later.
Also write_to_stream() arguments have changed and for the same reasons I think the modification can be kept

I want to follow our deprecation process:

Deprecation warning was added in 1.28.0 via DEP: Class, variable, and module names #867 in May 2022

Deprecation error was added in 3.0.0 <--- it should have been removed in 2.0.0; keep also in mind that we migrated PyPDF2 to pypdf. To the pypdf users it's just fair to have a little bit more time.

Removal: Originally it should be removed in 3.0.0, but now we're going to remove it in 4.0.0

It has "only" been one year since the warning was added. I want to make a pypdf==4.0.0 release with a lot of deprecated code removals in 2024 (probably early January). Until then, I want to keep those methods to allow people to have an easier time to transition.

Is there a reason to have it in this PR?

To me it seems as if this PR would become a lot smaller and thus easier to review without those changes.

as @pubpub-zz said, write_to_stream() arguments have changed, so writeToStream() should change, or be deleted. and it's not used in current code, and users do not use it, it's safe to delete it.

This makes PR #1816 smaller Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

pypdf/_writer.py

This makes PR #1816 smaller Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

MartinThoma · 2023-06-18T07:26:02Z

@exiledkingcc Just a short heads-up: I have not forgotten this PR and I'm excited to get it into pypdf!

However, I also want to review it thoroughly. Recently I had a lot going on in my private live (being sick / buying my own house) which meant that I didn't have the time for it so far. I will do it eventually :-) (I promise to do it this year, I hope to do it in July)

exiledkingcc · 2023-06-18T09:28:26Z

@MartinThoma it's ok, just take your time. and take care. 😊

MartinThoma · 2023-06-24T17:01:39Z

I've just added the "breaking change" label so that I don't forget that the PR currently contains breaking changes. I want to undo them for the moment, but I also want to collect those things so that I can do several breaking changes next year.

The idea is to keep pypdf stable for a while to give the community time to move from PyPDF2 to pypdf.

Full credit goes to exiledkingcc This PR was only made to make it easier to merge the other changes / to avoid merge conflicts. Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

* `CryptFilter.encrypt_object` implemented * `AlgV5.generate_values` now crops the user_password / owner_password to 127 bytes * The `EncryptAlgorithm` Enum was added. It contains the parameter V (version), R (revision), and length * `Encryption.encrypt_object` was added * `Encryption.write_entry` was added * The static method `Encryption.make` was added This PR was only made to make it easier to merge the other changes / to avoid merge conflicts of other changes with #1816 . Full credit goes to exiledkingcc. The PR is marked as "MAINT" as it doesn't add a new feature that an end-user could use. Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

* PdfWriter.encrypt: Add 'algorithm' parameter * PdfWriter: Add _encryption property * PdfWriter: Add _encrypt_entry property This change was made in another PR to avoid merge conflicts / get it merged soon. Full credit for the work goes to exiledkingcc who did all of the work in #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

* PdfWriter.encrypt: Add 'algorithm' parameter * PdfWriter: Add _encryption property * PdfWriter: Add _encrypt_entry property This change was made in #1816 to avoid merge conflicts / get it merged soon. Full credit for the work goes to exiledkingcc who did all of the work in #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

This PR was made to avoid breaking changes the original PR introduced. We want to remove the outdated parameters, but not at the moment. See https://pypdf.readthedocs.io/en/latest/dev/deprecations.html Full credit for the work goes to exiledkingcc via PR #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

Partially taken from #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

* MAINT: Deprecate the encryption_key parameter of write_to_stream Partially taken from #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

MartinThoma · 2023-07-02T21:40:06Z

@exiledkingcc I finally managed to merge all of the changes 🎉

It was a bit difficult because there were a lot of changes and I needed to get an overview. Hence I made several small PRs. I also didn't want to introduce breaking changes now.

I gave you full credit via the co-authored-by feature + every commit message. I also made a shout-out on Twitter + in the release notes to ensure that you get all the credit for the awesome work you did 🙏 Thank you 🤗

exiledkingcc added 9 commits April 27, 2023 16:48

ENH: implements encrypt parts for class Encryption

a1c5542

ENH: use class Encryption for PdfWriter to encrypt

6bff24a

ENH: support AES for encrypting

62699bf

ROB: fix when PdfWriter encrypt multiple times

7a60a9d

ROB: fix test_workflows.py::test_basic_features

22c9a4b

STY: cleanup. remove deprecated writeToStream

a915020

STY: cleanup

37846e7

ROB: fix test_page.test_empyt_password_1088

b903684

TST: fix tests when no PyCryptodome installed

7e60138

exiledkingcc added 2 commits April 29, 2023 11:10

MAINT: remove useless _security.py

0658468

ROB: fix calling PdfWriter::encrypt multiple times

8d8d01f

eliminate redundant `Encrypt` entries

exiledkingcc force-pushed the encrypt branch from 6795e68 to 8d8d01f Compare April 29, 2023 06:16

MartinThoma reviewed Apr 29, 2023

View reviewed changes

MartinThoma changed the title ~~add AES support for encrypting PDF files~~ ENH: Add AES support for encrypting PDF files Apr 29, 2023

MartinThoma added nf-security Non-functional change: Security workflow-encryption From a users perspective, encryption is the affected feature/workflow labels Apr 29, 2023

MartinThoma reviewed Apr 29, 2023

View reviewed changes

MartinThoma mentioned this pull request Apr 30, 2023

MAINT: Prepare for adding AES enryption support #1818

Merged

Merge branch 'main' into encrypt

e49abc5

MartinThoma added a commit that referenced this pull request Apr 30, 2023

MAINT: Make encryption_key parameter of write_to_stream optional

b17d90f

This makes PR #1816 smaller Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

MartinThoma mentioned this pull request Apr 30, 2023

MAINT: Make encryption_key parameter of write_to_stream optional #1819

Merged

MartinThoma reviewed Apr 30, 2023

View reviewed changes

pypdf/_writer.py Outdated Show resolved Hide resolved

MartinThoma added a commit that referenced this pull request Apr 30, 2023

MAINT: Make encryption_key parameter of write_to_stream optional (#1819)

3de03b7

This makes PR #1816 smaller Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

Merge branch 'main' into encrypt

ab572ef

MartinThoma mentioned this pull request Apr 30, 2023

MAINT: Add R parameter to generate_values #1820

Merged

MartinThoma added 2 commits April 30, 2023 13:55

Merge branch 'main' into encrypt

5385eaf

Apply black

4ad18be

Merge branch 'main' into encrypt

31d5225

MartinThoma added the breaking-change A planned breaking change label Jun 24, 2023

MartinThoma added the soon PRs that are almost ready to be merged, issues that get solved pretty soon label Jun 24, 2023

Merge branch 'main' into encrypt

712acb2

MartinThoma mentioned this pull request Jun 25, 2023

MAINT: Prepare for adding AES support via #1816 #1917

Merged

MartinThoma added 2 commits June 25, 2023 15:01

Merge branch 'main' into encrypt

0bb6b27

Merge branch 'main' into encrypt

c133384

MartinThoma mentioned this pull request Jun 25, 2023

ENH: Add AES support for encrypting PDF files #1918

Merged

Merge branch 'main' into encrypt

6232dbc

MartinThoma mentioned this pull request Jul 2, 2023

MAINT: Remove private _security submodule #1935

Merged

Merge branch 'main' into encrypt

43f636c

MartinThoma added a commit that referenced this pull request Jul 2, 2023

MAINT: Deprecate the encryption_key parameter of write_to_stream

66bd714

Partially taken from #1816 Co-authored-by: exiledkingcc <exiledkingcc@gmail.com>

MartinThoma mentioned this pull request Jul 2, 2023

MAINT: Deprecate the encryption_key parameter of write_to_stream #1936

Merged

MartinThoma added 2 commits July 2, 2023 23:10

Merge branch 'main' into encrypt

ac82785

Merge branch 'main' into encrypt

a7cfdb4

MartinThoma closed this Jul 2, 2023

MartinThoma changed the title ~~ENH: Add AES support for encrypting PDF files~~ ENH: Add AES support for encrypting PDF files (merged via other PRs) Jul 2, 2023

exiledkingcc deleted the encrypt branch July 3, 2023 09:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add AES support for encrypting PDF files (merged via other PRs) #1816

ENH: Add AES support for encrypting PDF files (merged via other PRs) #1816

exiledkingcc commented Apr 27, 2023 •

edited by MartinThoma

Loading

codecov bot commented Apr 28, 2023 •

edited

Loading

MartinThoma Apr 29, 2023

MartinThoma Apr 29, 2023

MartinThoma commented Apr 29, 2023

MartinThoma Apr 29, 2023

pubpub-zz Apr 29, 2023

MartinThoma May 1, 2023 •

edited

Loading

exiledkingcc May 2, 2023

MartinThoma commented Jun 18, 2023

exiledkingcc commented Jun 18, 2023

MartinThoma commented Jun 24, 2023

MartinThoma commented Jul 2, 2023

		user_pwd: Optional[str] = None, # deprecated
		owner_pwd: Optional[str] = None, # deprecated

ENH: Add AES support for encrypting PDF files (merged via other PRs) #1816

ENH: Add AES support for encrypting PDF files (merged via other PRs) #1816

Conversation

exiledkingcc commented Apr 27, 2023 • edited by MartinThoma Loading

codecov bot commented Apr 28, 2023 • edited Loading

Codecov Report

MartinThoma Apr 29, 2023

Choose a reason for hiding this comment

MartinThoma Apr 29, 2023

Choose a reason for hiding this comment

MartinThoma commented Apr 29, 2023

MartinThoma Apr 29, 2023

Choose a reason for hiding this comment

pubpub-zz Apr 29, 2023

Choose a reason for hiding this comment

MartinThoma May 1, 2023 • edited Loading

Choose a reason for hiding this comment

exiledkingcc May 2, 2023

Choose a reason for hiding this comment

MartinThoma commented Jun 18, 2023

exiledkingcc commented Jun 18, 2023

MartinThoma commented Jun 24, 2023

MartinThoma commented Jul 2, 2023

exiledkingcc commented Apr 27, 2023 •

edited by MartinThoma

Loading

codecov bot commented Apr 28, 2023 •

edited

Loading

MartinThoma May 1, 2023 •

edited

Loading