Support for libgcrypt, and external OCB in OpenSSL and libgcrypt #924
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This code passes all Mosh tests, but fails some of the more esoteric cases in the test program at the bottom of ocb.cc (none of which Mosh uses). Includes autoconf support. But this is somewhat inefficient, it inits/resets OCB on every message. Only tested on FreeBSD. Adds --with-external-ocb configure option, default to off.
cgull
force-pushed
the
libgcrypt-external-ocb
branch
from
06aa38f to
43347dd
Compare
waltarix
added a commit
to waltarix/mosh
that referenced
this pull request
Jan 12, 2022
achernya
added a commit
to achernya/mosh
that referenced
this pull request
Jun 7, 2022
OpenSSL 3.0 deprecated many of the functions that ocb.cc used to implement OCB-AES, causing a build failure when -Wdeprecated collided with -Werror. Debian temporarily fixed this by suppressing the error in mobile-shell#1191. Since mosh 1.4 will be the next stable release of mosh, it should not depend on deprecated functions in OpenSSL. Since version 1.1.0, OpenSSL natively supports OCB-AES through the EVP_CIPHER API. @cgull started early support for this in mobile-shell#924. This change extends upon the previous work by @cgull in a few ways * EVP_EncryptInit_ex is called in ae_init to set up the EVP_CIPHER_CTX. It is later called in ae_encrypt and ae_decrypt just to load the key and nonce (IV in OpenSSL EVP parlance), which reduces the amount of initialization done per-packet. * Adds missing support for an external tag, rather than just one appended to the ciphertext * Support for non-default-sized tags as well as some improved error handling. Note that this change raises the minimum OpenSSL version for Mosh to 1.1.0. OpenSSL does not provide security support for versions prior to 1.1 at this time, so this is in principle reasonable dependency. If we want to continue to support distributions (such as RHEL7) which continue to be supported by their vendor but use an unsupported OpenSSL, then some future work will have to restore the ocb.cc implementation that uses the deprecated functions. Bugs: mobile-shell#1174
achernya
added a commit
to achernya/mosh
that referenced
this pull request
Jun 7, 2022
OpenSSL 3.0 deprecated many of the functions that ocb.cc used to implement OCB-AES, causing a build failure when -Wdeprecated collided with -Werror. Debian temporarily fixed this by suppressing the error in mobile-shell#1191. Since mosh 1.4 will be the next stable release of mosh, it should not depend on deprecated functions in OpenSSL. Since version 1.1.0, OpenSSL natively supports OCB-AES through the EVP_CIPHER API. @cgull started early support for this in mobile-shell#924. This change extends upon the previous work by @cgull in a few ways * EVP_EncryptInit_ex is called in ae_init to set up the EVP_CIPHER_CTX. It is later called in ae_encrypt and ae_decrypt just to load the key and nonce (IV in OpenSSL EVP parlance), which reduces the amount of initialization done per-packet. * Adds missing support for an external tag, rather than just one appended to the ciphertext * Support for non-default-sized tags as well as some improved error handling. Note that this change raises the minimum OpenSSL version for Mosh to 1.1.0. OpenSSL does not provide security support for versions prior to 1.1 at this time, so this is in principle reasonable dependency. If we want to continue to support distributions (such as RHEL7) which continue to be supported by their vendor but use an unsupported OpenSSL, then some future work will have to restore the ocb.cc implementation that uses the deprecated functions. Bugs: mobile-shell#1174
achernya
added a commit
to achernya/mosh
that referenced
this pull request
Jun 14, 2022
OpenSSL 3.0 deprecated many of the functions that ocb.cc used to implement OCB-AES, causing a build failure when -Wdeprecated collided with -Werror. Debian temporarily fixed this by suppressing the error in mobile-shell#1191. Since mosh 1.4 will be the next stable release of mosh, it should not depend on deprecated functions in OpenSSL. Since version 1.1.0, OpenSSL natively supports OCB-AES through the EVP_CIPHER API. @cgull started early support for this in mobile-shell#924. This change extends upon the previous work by @cgull in a few ways * EVP_CipherInit_ex is called in ae_init to set up the EVP_CIPHER_CTX. It is later called in ae_encrypt and ae_decrypt just to load nonce (IV in OpenSSL EVP parlance), which reduces the amount of initialization done per-packet. However, due to OpenSSL API limitations, two copies of the EVP_CIPHER_CTX are kept: one for encryption, and one for decryption. * Adds missing support for an external tag, rather than just one appended to the ciphertext * Support for non-default-sized tags as well as some improved error handling. Note that this change raises the minimum OpenSSL version for Mosh to 1.1.0. OpenSSL does not provide security support for versions prior to 1.1 at this time, so this is in principle reasonable dependency. If we want to continue to support distributions (such as RHEL7) which continue to be supported by their vendor but use an unsupported OpenSSL, then some future work will have to restore the ocb.cc implementation that uses the deprecated functions. Bugs: mobile-shell#1174
achernya
added a commit
that referenced
this pull request
Jun 14, 2022
OpenSSL 3.0 deprecated many of the functions that ocb.cc used to implement OCB-AES, causing a build failure when -Wdeprecated collided with -Werror. Debian temporarily fixed this by suppressing the error in #1191. Since mosh 1.4 will be the next stable release of mosh, it should not depend on deprecated functions in OpenSSL. Since version 1.1.0, OpenSSL natively supports OCB-AES through the EVP_CIPHER API. @cgull started early support for this in #924. This change extends upon the previous work by @cgull in a few ways * EVP_CipherInit_ex is called in ae_init to set up the EVP_CIPHER_CTX. It is later called in ae_encrypt and ae_decrypt just to load nonce (IV in OpenSSL EVP parlance), which reduces the amount of initialization done per-packet. However, due to OpenSSL API limitations, two copies of the EVP_CIPHER_CTX are kept: one for encryption, and one for decryption. * Adds missing support for an external tag, rather than just one appended to the ciphertext * Support for non-default-sized tags as well as some improved error handling. Note that this change raises the minimum OpenSSL version for Mosh to 1.1.0. OpenSSL does not provide security support for versions prior to 1.1 at this time, so this is in principle reasonable dependency. If we want to continue to support distributions (such as RHEL7) which continue to be supported by their vendor but use an unsupported OpenSSL, then some future work will have to restore the ocb.cc implementation that uses the deprecated functions. Bugs: #1174
|
This work has been mostly superseded by newer, better OpenSSL OCB support in #1196 by @achernya, and later work, included (but not default) in Mosh 1.4.0. The libgcrypt support here might still be of interest to some, but given libgcrypt's unpopularity, isn't worth pulling into Mosh. After the problems discovered with OpenSSL's OCB, I now better understand @keithw's conservative position on using outside crypto implementations, and mostly agree with it. I think alternate crypto implementations still have some value for development and testing. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This supersedes #790.
It adds support for libgcrypt's AES implementation. It also supports the OCB implementations in libgcrypt and OpenSSL 1.1+, in place of the bundled Rogaway/Krovetz implementation. There's autoconf support for all these permutations. Unlike #790, the autoconf code defaults to the current behavior, and libgcrypt and/or external OCB must be manually selected.
The external-OCB support is done by adding shims that support the
ae_encrypt/ae_decryptAPI presented by the bundled code. The shims are incomplete and do not support the full range of possible OCB encryption/decryption actions available in that API, or the full range of OCB cipher configurations. However, they do support the straightforward OCB requests that Mosh makes, and likely many of the requests that other practical OCB-using programs will make. The code passes all of Mosh's OCB tests, but does not pass the Krovetz/Rogaway test code at the bottom ofsrc/crypto/ocb.cc. I think this code might be useful for other projects even if Mosh never uses it.The libgcrypt support has seen some testing on Linux, FreeBSD, and macOS. The OpenSSL OCB code has only been tested on FreeBSD and needs verification on other platforms, once they take up OpenSSL 1.1+ more seriously. So this is not yet really ready to pull into Mosh.