-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[WIP] Started trying to document symmetric encryption #26
Conversation
>>> from cryptography.primitives import BlockCipher, CBC | ||
>>> from cryptography.primitives.aes import AES | ||
>>> cipher = BlockCipher(AES(key), CBC(iv)) | ||
>>> cipher.encrypt("my secret message") + cipher.finalize() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requiring a .finalize() call seems error-prone and adding strings together like this is inefficient (ignoring CPython optimizations). Wouldn't it be better, if BlockCipher would behave like a StringBuilder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way to avoid a finalize call. Are you suggesting the API should be .encrypt()
and then .getvalue()
at the end to get the full string? We discussed this, and the conclusion was that buffering should be provided at a higher level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I haven't seen that discussion. Ignore me.
[...] | ||
|
||
Here ``key`` is the encryption key (which must be kept secret), and ``iv`` is | ||
the initialization vector (which should be random). Exactly what form these |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not should; must
|
||
``encrypt()`` should be called repeatedly with additional plaintext, and it | ||
will return the encrypted bytes, if there isn't enough data, it will buffer it | ||
internally. ``finalize()`` should be called at the end, and will return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about buffering internally. I think that padding will take care of the difference but I could be wrong about that. (E.g. a block that is too small will be padded out to be the right size). We should make sure about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They way it should work is encrypt(thing-half-as-long-as-block_size)
returns b""
, and then encrypt(another-thing-half-as-long)
, returns the encrypted block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should be a higher level thing. Padding should handle the difference so encypt(thing-half-as-long-as-block-size)
returns an encrypted chunk exactly as long as block size which is the encrypted version of thing-half-as-long-as-block-size
padded out using the padding scheme. Perhaps @lvh can clarify? :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Padding handles the issue of your plaintext being potentially shorter than a multiple of the block size. So you add as much padding to the end of the entire plaintext as is necessary to bring it up to the desired length or an entire block and you include information about how to remove it. Imagine for a moment .encrypt() would add padding to the given piece of plain text, how would you, given the entire plain text, decide what is padding and what isn't?
:param bytes initialization_vector: Must be random bytes. They do not need | ||
to be kept secret (they can be included | ||
in a transmitted message). Should be | ||
the same number of bytes as the ``key`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IV is the block size, not the key size.
I'm liking the way this is looking. I noticed the |
.. class:: cryptography.primitives.block.BlockCipher(cipher, mode) | ||
|
||
Block ciphers work by encrypting content in chunks, often 64- or 128-bits. | ||
Theycombine an underlying algorithm (such as AES), with a mode (such as CBC, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Theycombine" -> "They combine"
@dstufft Not so much intentional as I wanted to start with the minimal bits to be able to write code. |
This is probably as good as place as any to start moving forward with. It looks like a reasonable API and seems to have everything we need for encryption and we can always iterate from here. |
[WIP] Started trying to document symmetric encryption
Pass a Connection instance to the info callback (instead of a Context instance). Fixes pyca#16 (a regression introduced by the Python rewrite).
No description provided.