-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PKCS7: added encryption with AES-256-CBC #12172
Conversation
6cde053
to
cc79ad2
Compare
Made some changes to adapt, let me know what you think @alex ! |
self._content_encryption_algorithm = ( | ||
self._content_encryption_algorithm or algorithms.AES128 | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't mutate self here, just use a local. You'll get wrong behavior on a reused builder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I'll need to modify the rust code accordingly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Looks like there's a merge conflict in the changelog. can you rebase/merge main before I review? |
added & updated tests accordingly updated documentation removed useless test vector
changed name to content_encryption_algorithm simplified the rust code accordingly tried to simplify the documentation
adapted rust code accordingly
84fdaca
to
b29e77d
Compare
Yup, done! |
A few things to check:
content_algorithm
instead of justalgorihm
mode
included (as we might need it on next S/MIME versions), should I include it now?