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

Document which cipher modes do not support streaming #23028

Closed
wants to merge 4 commits into from

Conversation

t8m
Copy link
Member

@t8m t8m commented Dec 13, 2023

The first commit is applicable only to master and 3.2. The second one to all 3.x branches.

Checklist
  • documentation is added or updated

@t8m t8m added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: documentation The issue/pr deals with documentation (errors) branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 tests: exempted The PR is exempt from requirements for testing branch: 3.2 Merge to openssl-3.2 labels Dec 13, 2023
@@ -63,6 +63,12 @@ FIPS provider:
This implementation supports the parameters described in
L<EVP_EncryptInit(3)/PARAMETERS>.

=head1 NOTES

The XTS, SIV and WRAP mode implementations do not support streaming. That
Copy link
Member

Choose a reason for hiding this comment

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

This is not true for XTS. It has a stream update, but it only likes full blocks.. The last update can be a partial block (Which it can then use CTS).

Copy link
Member

Choose a reason for hiding this comment

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

It probably should have supported buffering.

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you please suggest how to formulate it better then? I've basically copied what we were saying in EVP_aes_128_xts() manpage.

Copy link
Member

Choose a reason for hiding this comment

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

Something like...

The OpenSSL AES-SIV and AES-WRAP implementations do not support streaming. That means to obtain correct results there can be only one [EVP_EncryptUpdate(3)](http://man.he.net/man3/EVP_EncryptUpdate) or [EVP_DecryptUpdate(3)](http://man.he.net/man3/EVP_DecryptUpdate) call after the initialization of the context.

The OpenSSL AES-XTS implementation allows streaming to be performed, but each EVP_EncryptUpdate/EVP_DecryptUpdate requires each input to be a multiple of the blocksize. Only the final  EVP_EncryptUpdate/EVP_DecryptUpdate can optionally have an input that its not a multiple of the blocksize. (If it is not a multiple then CipherText Stealing is used).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, I'll update it.

@t8m t8m requested a review from slontis December 14, 2023 10:11
@slontis slontis removed the approval: otc review pending This pull request needs review by an OTC member label Dec 14, 2023
Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM

@tom-cosgrove-arm tom-cosgrove-arm added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Dec 20, 2023
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Dec 21, 2023
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@t8m
Copy link
Member Author

t8m commented Dec 22, 2023

Merged to the master, 3.2 (both commits) and 3.1, 3.0 (only the second commit) branches. Thank you for the reviews.

@t8m t8m closed this Dec 22, 2023
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)

(cherry picked from commit 8f0f814)
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)

(cherry picked from commit e2f9c2d)
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)

(cherry picked from commit 8f0f814)
openssl-machine pushed a commit that referenced this pull request Dec 22, 2023
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from #23028)

(cherry picked from commit 8f0f814)
wbeck10 pushed a commit to wbeck10/openssl that referenced this pull request Jan 8, 2024
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from openssl#23028)
wbeck10 pushed a commit to wbeck10/openssl that referenced this pull request Jan 8, 2024
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
(Merged from openssl#23028)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch branch: 3.0 Merge to openssl-3.0 branch branch: 3.1 Merge to openssl-3.1 branch: 3.2 Merge to openssl-3.2 tests: exempted The PR is exempt from requirements for testing triaged: documentation The issue/pr deals with documentation (errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants