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 Support for AES Encryption/Decryption with ECB cipher mode. #3

Open
bssayeda opened this issue Oct 24, 2015 · 13 comments
Open

Add Support for AES Encryption/Decryption with ECB cipher mode. #3

bssayeda opened this issue Oct 24, 2015 · 13 comments

Comments

@bssayeda
Copy link

It would be nice to have support for AES Encryption/Decryption with ECB cipher mode. I know that at least the OS X APIs has support for ECB and so does OpenSSL. While there are great cases that ECB mode should not be used historically, there are some use cases I have run to where data that is encrypted using AES ECB is encrypted (along with other data) a second time with AES in CBC mode. I would rather not have to depend on another encryption library just for that use case.

@wbond
Copy link
Owner

wbond commented Oct 24, 2015

So is this just a case of needing decryption? ECB seems extremely insecure, so I'd like to avoid exposing it where possible. Perhaps an option could be to have a keyword arg such as allow_insecure=True that would be required to call the function?

Also curious, how did find out about this project?

@bssayeda
Copy link
Author

For my case I would need support for encryption although having decryption support as well would help with writing test cases.

As for how I found this project, I did a Google search for "Python Crypto OS X Windows Linux github" and this project was one of the first results that came up.

@bssayeda
Copy link
Author

I do agree that by default the APIs should opt for the more secure ciphers and if something calls for ECB the consumer should override the default behavior by adding the allow_insecure=True keyword.

@glyph
Copy link

glyph commented Oct 30, 2015

For what it's worth, I don't think this should be added; it's very dangerous to even call ECB "encryption", and the construction of CBC+ECB sounds like nonsense to me.

@bssayeda
Copy link
Author

@glyph I agree, unfortunately its I'm dealing with a spec that I have no control over =\

@wbond
Copy link
Owner

wbond commented Oct 30, 2015

@glyph As always, I appreciate your thoughts.

So far oscrypto has been about implementing primitives, mostly towards the goal of loading private keys and certs off of disk and using them for signing, verification, revocation checking and path validation. I don't believe that ECB is used anywhere in OpenSSL encrypted private keys, PKCS#5, PKCS#8, PKCS#7, CMS or PKCS#12.

@bssayeda Is the spec you are dealing with an in-house spec, or something public/published?

@bssayeda
Copy link
Author

@wbond The spec should be available publicly: https://developer.apple.com/services-account/download?path=/Developer_Tools/FairPlay_Streaming_SDK/FairPlay_Streaming_Server_SDK.zip

The PDF enclosed with the package contains the spec.

It is a long read so the area that uses AES ECB is on page 24 related to bytes 16-47. Those bytes are encrypted using AES ECB and then it and the other parts of the payload are encrypted using AES CBC.

@wbond
Copy link
Owner

wbond commented Oct 30, 2015

@bssayeda Thanks for providing the link.

I ran into some similar bad constructs when dealing with PDF encryption.

@glyph
Copy link

glyph commented Oct 30, 2015

Ah well. @bssayeda - good to know where the requirement comes from, I guess that is widespread enough that it is not going away. I retract my objection, although I would say a function that deals with ECB should not have the word "encrypt" in its name, even if "insecure" is in there :). "encode/decode", maybe.

@bssayeda
Copy link
Author

Encode/Decode in this case is probably the better choice if the call for it will be separate from the existing API call.

@wbond
Copy link
Owner

wbond commented Oct 30, 2015

Well, all cipher/mode/padding combinations are unique function calls in the public API. This was done on purpose for two reasons:

  1. Prevent invalid combinations and extra checking logic
  2. Make it easier to find usage of different combinations

@bssayeda What padding is being used in the spec? (sorry for being too lazy to download the SDK right now)

@bssayeda
Copy link
Author

@wbond For this case based off the reference implementation Apple includes in the package it looks like AES ECB "Encoding" with no padding.

@lambdaq
Copy link

lambdaq commented May 16, 2017

Hi, @wbond

I hope this lib can add ECB is not for security reasons, but because many dumb-ass Java programmers write code like

Cipher.getInstance("DES")
Cipher.getInstance("AES")

without second thinking.

According to JCA guide

Cipher.getInstance("DES") == Cipher.getInstance("DES/ECB/PKCS5Padding") 
Cipher.getInstance("AES") == Cipher.getInstance("AES/ECB/PKCS5Padding") 

Because in real world, we have to encrypt/decrypt data produced by these programmers, so I recommend providing the infamous ECB mode in oscrypto library.

btw I encountered oscrypto from reddit like years ago, didn't realise you where also the package_control author. Thanks for your awesome work! 👍

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

No branches or pull requests

4 participants