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 Counter support #104

Merged
merged 5 commits into from
Oct 17, 2013
Merged

AES Counter support #104

merged 5 commits into from
Oct 17, 2013

Conversation

reaperhulk
Copy link
Member

  • vectors from RFC 3686
  • Documentation for the mode

)
def test_OpenSSL(self, key, iv, plaintext, ciphertext, api):
if not api.supports_cipher("aes-128-ctr"):
pytest.skip("Does not support AES CTR") # pragma: no cover
Copy link
Member

Choose a reason for hiding this comment

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

Are there really common OpenSSL's which don't support AES CTR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Any OpenSSL pre 1.0.0, which includes every default OS X (have you brew installed openssl yet? :))

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 389892f on reaperhulk:ctr-support into 169dee8 on pyca:master.

reusing a nonce compromises the security of all data
encrypted under the key. Specifically,
(pt1 xor keystream) xor (pt2 xor keystream) is
equivalent to (pt1 xor pt2).
Copy link
Member

Choose a reason for hiding this comment

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

I think this last sentence should be removed, I'm not sure it adds a lot of clarity/.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm fine with that. It's not easy to read without decent formatting and it will be entirely opaque to most people anyway.

CTR (Counter) is a mode of operation for block ciphers. It is considered
cryptographically strong.

:param bytes nonce: Must be random bytes. They do not need to be kept
Copy link
Member

Choose a reason for hiding this comment

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

I thought CTR only needed unique bytes? E.g. you can implement it by incrementing a counter.

Copy link
Member Author

Choose a reason for hiding this comment

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

You absolutely can. The advantage of starting with a purely random nonce is that even if you're reusing your keys the probability of reusing a counter value is lowered (although obviously non-zero and increasing as the size of the encrypted payload increases). If anyone has better language for this that captures the nuance of "random fine but not necessary if you're careful not to reuse a key with the same counter value" I'll change it.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know that you can rely on random with CTR mode. It's maybe OK for AES since it has a large enough block size, but CTR can be used for smaller block ciphers as well where that isn't the case.

Perhaps something like:

MUST be a unique value, any reuse of the nonce with the same key compromises the security of every message encrypted with that key. Must be the same number of bytes as the block_size of the cipher with a given key. The nonce does not need to be kept secret and may be included alongside the ciphertext.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about:

Must be unique bytes (may be randomly generated). Must be the same number of bytes as the block_size of the cipher. It is critical to never reuse a nonce with a given key. Unlike CBC reusing a nonce compromises the security of all data encrypted under the key.

That still doesn't quite encompass the idea that using nonce+1 with the same key is just as bad as nonce though.

Copy link
Member

Choose a reason for hiding this comment

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

I can't parse your last sentence.

Copy link
Member

Choose a reason for hiding this comment

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

Overall I think your suggestion is better, I just shy away from us implying that random is good enough when that really depends on block size.

Copy link
Member

Choose a reason for hiding this comment

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

block size, and how long the keys are good for.

Copy link
Member Author

Choose a reason for hiding this comment

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

So reusing the nonce with the same key compromises the entire stream, but if you were to encrypt plaintext1 with a given nonce, then encrypt plaintext2 with the same nonce but increment the last bit by 1, then you've essentially reused the nonce. Only the very first block would be different.

I will remove references to random as you make a good point re: block size dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

Proposed:

Must be a unique value. It is critical to never reuse a nonce (or its subsequent incremented counter values) with a given key. Any reuse of the nonce with the same key compromises the security of every message encrypted with that key. Must be the same number of bytes as the block_size of the cipher with a given key. The nonce does not need to be kept secret and may be included alongside the ciphertext.

Copy link
Member

Choose a reason for hiding this comment

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

+1

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0ba2f94 on reaperhulk:ctr-support into 169dee8 on pyca:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 4506428 on reaperhulk:ctr-support into 169dee8 on pyca:master.

CTR (Counter) is a mode of operation for block ciphers. It is considered
cryptographically strong.

:param bytes nonce: Recommended to be random. It is critical to never reuse
Copy link
Member

Choose a reason for hiding this comment

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

random bytes

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 89b3dd3 on reaperhulk:ctr-support into 169dee8 on pyca:master.

alex added a commit that referenced this pull request Oct 17, 2013
@alex alex merged commit b8f6a36 into pyca:master Oct 17, 2013
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants