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
Restore EVP_CIPHER_CTX "running IV" access
#12233
Conversation
|
Note that I did not (yet) do so, but I believe it would be permitted to remove the |
|
One thing I will note is that kTLS on FreeBSD does indeed support some cipher suites using AES-CBC. However, the kTLS hooks use the IV directly from the PRF result at the start of a key session rather than fetching it from the context, so I believe they are not affected by this change. For an example of kTLS fetching the IV, see https://github.com/openssl/openssl/blob/master/ssl/t1_enc.c#L446 where |
EVP_CIPHER_CTX "original IV" accessEVP_CIPHER_CTX "running IV" access
|
Thanks for the insight, John! |
|
[travis is quite lit up about the part where we use now-deprecated functions internally; I'll have to look up the incantation to appeast it] |
|
|
EVP_CIPHER_CTX "running IV" accessEVP_CIPHER_CTX "running IV" access
|
Marking as WIP until I have a chance to address the no-deprecated stuff |
|
I pushed quite a few additional commits and rebased onto the latest master (only one minor merge conflict, over whitespace in core_names.h). For most of the libcrypto EVP implementations it seemed better to just inline the (old) @levitte please take another look when you get a chance. |
EVP_CIPHER_CTX "running IV" accessEVP_CIPHER_CTX "running IV" access
|
(Also, took out of WIP) |
|
After what @bsdjhb wrote, I'm at a loss as to why the IV state is still needed. |
The proxmiate cause is that I have need to serialize the cipher state; it happens to be for a non-kTLS purpose, but it's not clear that the exact use-case matters. (I can talk about it if you want, of course, but is it needed?) I can do this in 1.1.1 with the existing public APIs. However, it is quite clear that the current state of master is an API break from 1.1.1, since the data that you can access from the Now, I could replicate access to that data by caching the last blocks of (plaintext and) ciphertext [0], and then grabbing those off from the side where I currently use the IV state, but that's both needlessly complicated and duplicates what's already being done by the implementation. [0] The CBC IV state is the last full block of ciphertext, and the OFB state is the keystream used to encrypt the last block of ciphertext, thus recoverable by (plaintext xor ciphertext) |
|
Thank you. A reason I'm asking these things is the unfortunate fact that before 1.1.0, almost everything was public. I have always thought of the running IV as internal, and belonging with the backends, and use of it by any other a kinda dubious hack. But if there are legitimate uses of it after all, I can understand. |
|
FWIW I'm going to have to rebase past #12226 in order to get a clean no-deprecated build. |
|
CI looks clean (the one failed Travis job is just a timeout, that I think we've been seeing fairly regularly). |
|
I'd be interested to know what @p-steuer thinks about ec681a305ee58c70c9a1a319a104ba4df9abc9d1 -- I think the best thing to do would be to teach the appropriate |
800db05
to
c4764e8
Compare
|
Rebased past the libcrypto.num conflict. |
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.
Good work! Approved if CI agrees.
I think theres nothing wrong with sync-ing the PROV_CIPHER_CTX for now. If the additional memcpy turns out to affect performance, i ll look into a get_ctx_params solution. At least it looks to me like the sync is missing in case of s390x's ccm and gcm though. Lets see if CI agrees. |
The CCM and GCM modes don't have any "IV state" that gets updated after each operation, so the CI should be okay (in that regard at least). Whether or not I consciously thought about that and decided to not put in "useless" memcpys is another matter, though... |
Some modes (e.g., CBC and OFB) update the effective IV with each block-cipher invocation, making the "IV" stored in the (historically) EVP_CIPHER_CTX or (current) PROV_CIPHER_CTX distinct from the initial IV passed in at cipher initialization time. The latter is stored in the "oiv" (original IV) field, and has historically been accessible via the EVP_CIPHER_CTX_original_iv() API. The "effective IV" has also historically been accessible, via both EVP_CIPHER_CTX_iv() and EVP_CIPHER_CTX_iv_noconst(), the latter of which allows for *write* access to the internal cipher state. This is particularly problematic given that provider-internal cipher state need not, in general, even be accessible from the same address space as libcrypto, so these APIs are not sustainable in the long term. However, it still remains necessary to provide access to the contents of the "IV state" (e.g., when serializing cipher state for in-kernel TLS); a subsequent reinitialization of a cipher context using the "IV state" as the input IV will be able to resume processing of data in a compatible manner. This problem was introduced in commit 089cb62, which effectively caused all IV queries to return the "original IV", removing access to the current IV state of the cipher. These functions for accessing the (even the "original") IV had remained undocumented for quite some time, presumably due to unease about exposing the internals of the cipher state in such a manner. Note that this also as a side effect "fixes" some "bugs" where things had been referring to the 'iv' field that should have been using the 'oiv' field. It also fixes the EVP_CTRL_GET_IV cipher control, which was clearly intended to expose the non-original IV, for use exporting the cipher state into the kernel for kTLS. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
The EVP_CIPHER_CTX_iv() family of functions are incompatible with
the libcrypto/provider separation, since the implied API contract
(they are undocumented) involves a pointer into the active cipher
context structure. However, the active IV data in a provider-side
context need not even be in the same address space as libcrypto,
so a replacement API is needed.
The existing functions for accessing the (even the "original") IV had
remained undocumented for quite some time, presumably due to unease
about exposing the internals of the cipher state in such a manner.
Provide more maintainable new APIs for accessing the initial ("oiv") and
current-state ("iv") IV data, that copy the value into a caller-provided
array, eliminating the need to provide a pointer into the internal
cipher context, which accordingly no longer provides the ability to
write to the internal cipher state.
Unfortunately, in order to maintain API compatibility with OpenSSL
1.1.1, the old functionality is still available, but is marked as
deprecated for future removal. This would entail removing the "octet
pointer" parameter access, leaving only the "octet string" parameter
type.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from openssl#12233)
Test that EVP_CIPHER_CTX_get_iv() returns the same IV that was given at initialization time, and that EVP_CIPHER_CTX_get_iv_state() returns the expected value after performing an encryption operation (which will differ from the previous value for CBC and OFB modes), for various modes of AES. Do this both for the implicit fetch and explicit fetch paths, at the cost of a slightly more complicated switch statement. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
The current check for iv_gen and iv_gen_rand only lets you fetch the IV for the case when it was set internally. It might also make sense to fetch the IV if one was set at cipher-context creation time, so switch to checking the iv_state, which should be enough to ensure that there is valid data in the context to be copied out. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Including the ones that were added in commit 83b0634 with a note that they "may go away" and are now deprecated. Remove the missingcrypto.txt entries for the now-deprecated functions. [extended tests] Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
It is superseded by EVP_CIPHER_CTX_get_iv(), is only present on master, and had only a couple of in-tree callers that are easy to convert. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_aes.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_aes_cbc_hmac_sha1.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_aes_cbc_hmac_sha256.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_aria.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_camellia.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_des.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_des3.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_sm4.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_xcbc_d.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in e_rc2.c. For the legacy implementations, there's no need to use an in-provider storage for the IV, when the crypto operations themselves will be performed outside of the provider. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Inline the pre-13273237a65d46186b6bea0b51aec90670d4598a versions of EVP_CIPHER_CTX_iv(), EVP_CIPHER_CTX_original_iv(), and EVP_CIPHER_CTX_iv_noconst() in evp.h. These macros are internal-only, used to implement legacy libcrypto EVP ciphers, with no real provider involvement. Accordingly, just use the EVP_CIPHER_CTX storage directly and don't try to reach into a provider-side context. This does necessitate including evp_local.h in several more files. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Use EVP_CIPHER_CTX_get_iv() to implement EVP_CIPHER_set_asn1_iv(), rather than the deprecated EVP_CIPHER_CTX_original_iv(). Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
Use EVP_CIPHER_CTX_get_iv_state() in cipher_test_enc() rather than the deprecated EVP_CIPHER_CTX_iv(). [extended tests] Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
The S390x hardware-accelerated cipher implementations keep their IV state in an internal structure tied to the underlying implementation. However, the provider itself needs to be able to expose the IV state to libcrypto when processing the "iv-state" parameter. In the absence of a S390x hardware-specific get_ctx_params() implementation, be sure to copy the IV state from the hw-specific structure back to the generic PROV_CIPHER_CTX object after each cipher operation in order to synchronize the internal and fetchable state. [extended tests] Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from openssl#12233)
During OpenSSL 3.0 development since OpenSSL commits:
| 718b133a5328 Implement AES CBC ciphers in the default provider
| 819a7ae9fc77 Implement AES CTR ciphers in the default provider
the dhgex tests ("make t-exec LTESTS=dhgex") are failing.
OpenSSH needs the "current" IV state, which is aquired with the accessor
function EVP_CIPHER_CTX_get_iv(). The libressl compat layer uses
EVP_CIPHER_CTX_iv() to implement EVP_CIPHER_CTX_get_iv(), see:
| 482d23b upstream: hold our collective noses and use the openssl-1.1.x API in
| 48f54b9 adapt -portable to OpenSSL 1.1x API
Duing OpenSSL 3.0 development EVP_CIPHER_CTX_iv() was deprecated, and later
OpenSSL re-added the functionality: EVP_CIPHER_CTX_get_iv() and
EVP_CIPHER_CTX_get_iv_state() were introduced. However,
EVP_CIPHER_CTX_get_iv() returns the original IV, while
EVP_CIPHER_CTX_get_iv_state() returns the current IV. See openssl PR #12233 for
additional discussion.
This is a API clash, since OpenSSH expects EVP_CIPHER_CTX_get_iv() to return
the running IV. See OpenSSL issue #13411 for an ongoing discussion on how to
fix the problem, by renaming the functions.
This patch works around the problem in the libressl compat layer, by providing
a EVP_CIPHER_CTX_get_iv() function, that calls EVP_CIPHER_CTX_get_iv_state(),
only if EVP_CIPHER_CTX_get_iv_state() is available. This internal
EVP_CIPHER_CTX_get_iv() will be used by OpenSSH instead of the
EVP_CIPHER_CTX_get_iv() provided by OpenSSL-3.0.
The latest changes in OpenSSL 3.0 in combination with this patch fixes the
non-GCM ciphers. All but the chacha20-poly1305 test are working again:
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 3des-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 3des-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-cbc
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-ctr
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-ctr
| dhgex bits 3072 diffie-hellman-group-exchange-sha1 aes128-gcm@openssh.com
| dhgex bits 3072 diffie-hellman-group-exchange-sha256 aes128-gcm@openssh.com
| dhgex bits 7680 diffie-hellman-group-exchange-sha1 aes192-cbc
| dhgex bits 7680 diffie-hellman-group-exchange-sha256 aes192-cbc
| dhgex bits 7680 diffie-hellman-group-exchange-sha1 aes192-ctr
| dhgex bits 7680 diffie-hellman-group-exchange-sha256 aes192-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-cbc
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-cbc
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-ctr
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 aes256-gcm@openssh.com
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 aes256-gcm@openssh.com
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 rijndael-cbc@lysator.liu.se
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 rijndael-cbc@lysator.liu.se
| dhgex bits 8192 diffie-hellman-group-exchange-sha1 chacha20-poly1305@openssh.com
| ssh failed ()
| dhgex bits 8192 diffie-hellman-group-exchange-sha256 chacha20-poly1305@openssh.com
| ssh failed ()
Cc: Thomas Dwyer III <tomiii@tomiii.com>
Link: https://www.spinics.net/lists/openssh-unix-dev/msg06860.html
Link: openssl/openssl#12233
Link: openssl/openssl#13411
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In #10026 it was asserted that there's no need to expose the "running IV" to applications (note: not strictly an IV anymore) and that there should only be a need for the "original IV" in applications, and that if there's a need for the running IV that can be added later.
It turns out there is a need for the "running IV", for when a cipher context needs to be serialized, as is done by kTLS (though note that AFAIK kTLS only currently supports AEAD cipher modes, for which there is not a distinction between "original" and "running" IV).
However, the existing APIs that have (historically, prior to #10026) exposed the "running IV" are bad and deliberately undocumented -- they provide a pointer directly into the internal implementation buffer. When this buffer was in the
EVP_CIPHER_CTXitself, that was workable (but still unpleasant) since it was in a known location and always available if theEVP_CIPHER_CTXwas non-NULL, but in a provider-centric model this can only make sense if the returned pointer is into the provider's internal data store, and that is flat-out unsustainable. For one, it requires that the provider implements things with the IV stored as a contiguous byte array, which is a weird and opaque ABI constraint between libcrypto and provider, and additionally, there's not even a requirement that the provider's internal data store be accessible from the same address space as libcrypto!To make the best of a bad situation, introduce a pair of new APIs that copy the initial or "running" IV into a caller-supplied buffer, which will eventually let us remove the octet-pointer parameter usage for the IV data, and document the old APIs as deprecated.
Test the new APIs for several cipher modes of AES, confirming that the "running IV" or, as I call it for the new APIs, the "IV state", diverges from the initial IV for CBC and OFB, but not for GCM, CCM, and OCB.
In order to test GCM, adjust
gcm_get_ctx_params()to use a slightly different check for when it is safe to return a value: previously we could only retrieve an IV value that was generated internally to the library, but not one that was explicitly provided at context initialization.