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

Improve cookie crypto for CookieDealer #373

Merged
merged 2 commits into from
Jun 8, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ The format is based on the [KeepAChangeLog] project.
### Fixed
- [#369]: The AuthnEvent object is now serialized to JSON for the session.

### Security
- [#363]: Fixed IV reuse for CookieDealer class. Replaced the encrypt-then-mac construction with a proper AEAD (AES-SIV).

[#324]: https://github.com/OpenIDC/pyoidc/pull/324
[#369]: https://github.com/OpenIDC/pyoidc/pull/369
[#363]: https://github.com/OpenIDC/pyoidc/issue/363

## 0.10.0.1 [UNRELEASED]

Expand Down
83 changes: 83 additions & 0 deletions src/oic/utils/aes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

from Cryptodome import Random
from Cryptodome.Cipher import AES
from six import binary_type
from six import indexbytes
from six import text_type

__author__ = 'rolandh'

Expand Down Expand Up @@ -105,6 +107,87 @@ def decrypt(key, msg, iv=None, padding="PKCS#7", b64dec=True):
return res.decode("utf-8")


class AEAD(object):
"""
Authenticated Encryption with Associated Data Wrapper

This does encryption and integrity check in one
operation, so you do not need to combine HMAC + encryption
yourself.

:param key: The key to use for encryption.
:type key: bytes
:param iv: The initialization vector.
:type iv: bytes
:param mode: One of the AEAD available modes.

Your key and initialization vectors should be created from random bytes
of sufficient length.

For the default SIV mode, you need one of:

- 256-bit key, 128-bit IV to use AES-128
- 384-bit key, 192-bit IV to use AES-192
- 512-bit key, 256-bit IV to use AES-256

"""
def __init__(self, key, iv, mode=AES.MODE_SIV):
assert isinstance(key, binary_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we not raise an ImproperlyConfigured error here for consistency?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This assert is mostly there to help with PY2/PY3 issues so keys/ivs etc stay 'byte strings'.
The configuration mistakes possible should be handled higher up, down here it is a programming error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it 👍

assert isinstance(iv, binary_type)
self.key = key
self.mode = mode
self.iv = iv
self.kernel = AES.new(self.key, self.mode, self.iv)

def add_associated_data(self, data):
"""
Add data to include in the MAC

This data is protected by the MAC but not encrypted.

:param data: data to add in the MAC calculation
:type data: bytes
"""
if isinstance(data, text_type):
data = data.encode('utf-8')
self.kernel.update(data)

def encrypt_and_tag(self, cleardata):
"""
Encrypt the given data

Encrypts the given data and returns the encrypted
data and the MAC to later verify and decrypt the data.

:param cleardata: data to encrypt
:type cleardata: bytes

:returns: 2-tuple of encrypted data and MAC
"""
assert isinstance(cleardata, binary_type)
return self.kernel.encrypt_and_digest(cleardata)

def decrypt_and_verify(self, cipherdata, tag):
"""
Decrypt and verify

Checks the integrity against the tag and decrypts the
data. Any associated data used during encryption
needs to be added before calling this too.

:param cipherdata: The encrypted data
:type cipherdata: bytes
:param tag: The MAC tag
:type tag: bytes
"""
assert isinstance(cipherdata, binary_type)
assert isinstance(tag, binary_type)
try:
return self.kernel.decrypt_and_verify(cipherdata, tag)
except ValueError:
raise AESError("Failed to verify data")


if __name__ == "__main__":
key_ = "1234523451234545" # 16 byte key
# Iff padded the message doesn't have to be multiple of 16 in length
Expand Down
Loading