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

Aes unify #14

Closed
wants to merge 6 commits into from
Closed

Conversation

doomedraven
Copy link
Contributor

This one can replace CFB PR if you fine with unification as is the same and the algorithm byte is just after sizes

@doomedraven
Copy link
Contributor Author

restored the return types, as i need this for py3.8 which doesn't support dict type, is done now

This was referenced Nov 24, 2024
@jeFF0Falltrades
Copy link
Owner

Love this! I closed your other PR as I was just about to recommend combining this all into the same class when I saw this PR - well done 😃 . Also you taught me about contextlib.suppress which I'm going to definitely start using more, so thank you!

I made some minor changes in doomedraven#2 for clarity:

  • Renamed the class to ConfigDecryptorAESWithIV and moved it back to the top of SUPPORTED_DECRYPTORS: I plan to put this in the CONTRIBUTING.md file mentioned in small intro notes, how to start to make it easier for people to get on board #16 , but I order that list by how common a decryptor is in the wild, and AES CBC is by far the most common as that was the one used in the original version of AsyncRAT and its derivatives, so I like to keep it as first priority; This allows it to live alongside CFB with some kind of unifying name

  • Initialized self.aes_algo with a default value in the constructor - I specify the default values there so that other contributors know which class members to expect, even if they are set later on

  • Adjusted type hinting for your additions

  • Ran formatting/linting with ruff to bring the file into alignment with the others - I actually just made an Issue in Add standardized formatting and linting hooks for contributors #18 to automate this at some point so no other contributor has to worry about it; I'll get to work on that shortly, but in the meantime, FYI I use ruff with isort selection.

I tested this against the usual batch and had no issues, so if you approve and merge my changes, I'll do the same for yours.

@jeFF0Falltrades
Copy link
Owner

Also, can you bump the version in src/rat_king_parser/_version.py to 3.2.0 before we do final merge, please and thank you!

@doomedraven
Copy link
Contributor Author

about contextlib.suppress i did learn a lot from refurb library https://github.com/dosisod/refurb. this PR was moved to #19 with your changes, so closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants