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

Add crypto module #151

Merged
merged 21 commits into from
Oct 16, 2023
Merged

Add crypto module #151

merged 21 commits into from
Oct 16, 2023

Conversation

parfeon
Copy link
Contributor

@parfeon parfeon commented Oct 1, 2023

feat(crypto): add crypto module

Add crypto module that allows to configure SDK to encrypt and decrypt messages.

fix(crypto): fix legacy cryptor

Improved security of crypto implementation by adding enhanced AES-CBC cryptor.

@parfeon parfeon added status: done This issue is considered resolved. priority: medium This PR should be reviewed after all high priority PRs. type: feature This PR contains new feature. labels Oct 1, 2023
@parfeon parfeon requested a review from kleewho as a code owner October 1, 2023 22:21
@parfeon parfeon self-assigned this Oct 1, 2023
@parfeon parfeon requested a review from seba-aln as a code owner October 1, 2023 22:21
encrypted_message = Base64.decode64(message[:payload])
JSON.parse(crypto_module.decrypt(encrypted_message), quirks_mode: true)
#
# return message[:payload] if message[:channel].end_with?('-pnpres') || (@app.env[:cipher_key].nil? && @cipher_key.nil? && @cipher_key_selector.nil? && @env[:cipher_key_selector].nil?)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this stays commented or to be deleted?

else
message = message.to_json
message = Formatter.encode(message) if uri_escape
# def format_message(message, cipher_key = '', use_random_iv = false, uri_escape = true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this commented code. It stays?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seba-aln good catch! I forgot to clean it up.

if cryptor.nil?
identifier = header.identifier || 'UNKN'
raise UnknownCryptorError, {
message: "Decrypting data created by unknown cryptor. Please make sure to register
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a Ruby formatting thing or a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seba-aln what typo?

# Legacy payload doesn't have header.
return 0 if @data.nil?

9 + (data_size < 255 ? 1 : 3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall header size doesn't return the header size + data length? or the docs are misleading?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns the size of the header (how many bytes encode header information) – it used to compute offsets in data and pull out crypto-defined data followed by encrypted data.

@parfeon
Copy link
Contributor Author

parfeon commented Oct 16, 2023

@pubnub-release-bot release

@parfeon parfeon merged commit 312e266 into master Oct 16, 2023
7 checks passed
@parfeon parfeon deleted the CLEN-1588/decouple-crypto-module branch October 16, 2023 11:51
@pubnub-release-bot
Copy link

🚀 Release successfully completed 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium This PR should be reviewed after all high priority PRs. status: done This issue is considered resolved. type: feature This PR contains new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants