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 encrypt_text/decrypt_text function using aes256 algorithm (2.1.5) #17366

Merged

Conversation

xsellier
Copy link
Contributor

@xsellier xsellier commented Mar 8, 2018

This merge request should fix this issue #1969

  • Tested using the following gdscript
  var encrypt_key = 'supersecretkey'
  var original_string = {
    test = 'aAáäç,©./:\'[p',
    test2 = 156,
    as56 = '©ç9865321898#@!$%6&'
  }

  var encrypted_string = original_string.to_json().encrypt_text(encrypt_key)
  var decrypted_string = encrypted_string.decrypt_text(encrypt_key)

  print('original_string: %s' % [original_string])
  print('encrypted_string: %s' % [encrypted_string])
  print('decrypted_string: %s' % [decrypted_string])

I'm using it for large amount of json, and it works flawlessly

@xsellier xsellier closed this Mar 8, 2018
@xsellier xsellier reopened this Mar 8, 2018
@xsellier xsellier changed the title (WIP) Add encrypt_text/decrypt_text function using aes256 algorithm (WIP) Add encrypt_text/decrypt_text function using aes256 algorithm (2.1.5) Mar 8, 2018
@xsellier xsellier force-pushed the feature/add-encrypt-decrypt-string branch 2 times, most recently from c6d78f8 to 4f21c60 Compare March 9, 2018 03:04
@xsellier xsellier changed the title (WIP) Add encrypt_text/decrypt_text function using aes256 algorithm (2.1.5) Add encrypt_text/decrypt_text function using aes256 algorithm (2.1.5) Mar 9, 2018
@xsellier xsellier force-pushed the feature/add-encrypt-decrypt-string branch from 4f21c60 to cd73c86 Compare March 9, 2018 03:06
@akien-mga akien-mga added this to the 2.1 milestone Mar 9, 2018
@i80and
Copy link

i80and commented Mar 9, 2018

I have no right to jump in here as just a rando, but this looks like it's implementing ECB?

At minimum you want to implement CBC: https://en.wikipedia.org/wiki/Block_cipher_mode_of_operation#Cipher_Block_Chaining_(CBC)

In CBC mode, each block of plaintext is XORed with the previous ciphertext block before
being encrypted. This way, each ciphertext block depends on all plaintext blocks processed
up to that point. To make each message unique, an initialization vector must be used in the
first block.

@xsellier
Copy link
Contributor Author

xsellier commented Mar 9, 2018

@i80and You are right it is ECB. Since only ECB is implemented and already used by Godot Engine, I will keep going using AES ECB. Plus CBC is not implemented in Godot Engine.

Feel free to open another issue to be able to use another encryption algorithm (here is why CBC is way better than ECB)

@xsellier xsellier force-pushed the feature/add-encrypt-decrypt-string branch from cd73c86 to 0332719 Compare March 20, 2018 14:39
@akien-mga
Copy link
Member

This should be implemented in the master branch too. As I expect there will be quite some bikeshedding there, I'll merge this one in the meantime for 2.1. Users of this method should not expect forward compatibility when porting their projects to 3.x.

@akien-mga akien-mga merged commit ddcea2a into godotengine:2.1 Mar 23, 2018
@xsellier
Copy link
Contributor Author

@akien-mga Do you want me to port it to the mater branch ? (as is)

@akien-mga
Copy link
Member

Yeah that would be good to start the discussion. Maybe we won't have as much bikeshedding as I fear and will be able to merge it as is :D

@xsellier xsellier deleted the feature/add-encrypt-decrypt-string branch March 23, 2018 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants