Skip to content

Commit

Permalink
Don't use CRYPTO_AES_CTR if it isn't defined.
Browse files Browse the repository at this point in the history
  • Loading branch information
@snhenson
snhenson committed Feb 18, 2014
1 parent 3c6c139 commit 6ecbc2b
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions crypto/engine/eng_cryptodev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,11 @@ static struct {
{ CRYPTO_AES_CBC, NID_aes_128_cbc, 16, 16, },
{ CRYPTO_AES_CBC, NID_aes_192_cbc, 16, 24, },
{ CRYPTO_AES_CBC, NID_aes_256_cbc, 16, 32, },
#ifdef CRYPTO_AES_CTR
{ CRYPTO_AES_CTR, NID_aes_128_ctr, 14, 16, },
{ CRYPTO_AES_CTR, NID_aes_192_ctr, 14, 24, },
{ CRYPTO_AES_CTR, NID_aes_256_ctr, 14, 32, },
#endif
{ CRYPTO_BLF_CBC, NID_bf_cbc, 8, 16, },
{ CRYPTO_CAST_CBC, NID_cast5_cbc, 8, 16, },
{ CRYPTO_SKIPJACK_CBC, NID_undef, 0, 0, },
Expand Down Expand Up @@ -602,7 +604,7 @@ const EVP_CIPHER cryptodev_aes_256_cbc = {
EVP_CIPHER_get_asn1_iv,
NULL
};

#ifdef CRYPTO_AES_CTR
const EVP_CIPHER cryptodev_aes_ctr = {
NID_aes_128_ctr,
16, 16, 14,
Expand Down Expand Up @@ -641,7 +643,7 @@ const EVP_CIPHER cryptodev_aes_ctr_256 = {
EVP_CIPHER_get_asn1_iv,
NULL
};

#endif
/*
* Registered by the ENGINE when used to find out how to deal with
* a particular NID in the ENGINE. this says what we'll do at the
Expand Down Expand Up @@ -679,6 +681,7 @@ cryptodev_engine_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
case NID_aes_256_cbc:
*cipher = &cryptodev_aes_256_cbc;
break;
#ifdef CRYPTO_AES_CTR
case NID_aes_128_ctr:
*cipher = &cryptodev_aes_ctr;
break;
Expand All @@ -688,6 +691,7 @@ cryptodev_engine_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
case NID_aes_256_ctr:
*cipher = &cryptodev_aes_ctr_256;
break;
#endif
default:
*cipher = NULL;
break;
Expand Down

7 comments on commit 6ecbc2b

@moonman
Copy link

@moonman moonman commented on 6ecbc2b Jul 5, 2014

Choose a reason for hiding this comment

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

Wondering if somebody is looking at this bug: http://rt.openssl.org/Ticket/Display.html?id=2770&user=guest&pass=guest
Bug report has been there for a while. Patches were updated yesterday and I can confirm they work on Kirkwood Armv5 SoC. Without these patches any https connectivity breaks: easily tested with wget.

@Retropotenza
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch works under the assumption that CRYPTO_AES_CTR ice declared with a define (like in https://github.com/openbsd/src/blob/master/sys/crypto/cryptodev.h).

It doesn't work correctly if it is instead declared with an enum (like in https://github.com/cryptodev-linux/cryptodev-linux/blob/master/crypto/cryptodev.h).

@levitte
Copy link
Member

@levitte levitte commented on 6ecbc2b May 8, 2019

Choose a reason for hiding this comment

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

It doesn't work correctly if it is instead declared with an enum (like in https://github.com/cryptodev-linux/cryptodev-linux/blob/master/crypto/cryptodev.h).

Very true. We've worked with the assumption that you will have to run with a modern enough cryptodev-linux. If there is some other safe way to detect what's available, PR welcome!

@Retropotenza
Copy link
Contributor

@Retropotenza Retropotenza commented on 6ecbc2b May 8, 2019 •
edited

Choose a reason for hiding this comment

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

Unfortunately, I was not able to find a good alternative here... I patched my working copy of OpenSSL with replacing CRYPTO_AES_CTR with a USE_CRYPTO_AES_CTR, defining it later at config time. But I didn't like too much the idea to patch OpenSSL at every new release.

So i raised the issue in cryptodev-linux/cryptodev-linux#44, and I proposed and alternative cryptodev-linux in https://github.com/Retropotenza/cryptodev-linux/blob/master/crypto/cryptodev.h that retain both the defines and enums. Is not elegant, but is compatible with anything out on the field (the cryptodev-linux header enums are there since 2011 at least).

@Retropotenza
Copy link
Contributor

Choose a reason for hiding this comment

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

In the latest OpenSSL releases, there is a CHECK_BSD_STYLE_MACROS that at the end control the usage of CRYPTO_AES_CTR.

Looking at https://github.com/openssl/openssl/blob/master/engines/e_devcrypto.c, if a CRYPTO_ALGORITHM_MIN define is declared CRYPTO_AES_CTR is handled as a define.

I was not able to find where this CRYPTO_ALGORITHM_MIN is defined... without it, the check on CRYPTO_AES_CTR is basically skipped (which make cryptodev-linux working fine... this approach maybe could be back ported in older OpesSSL branches).

@levitte
Copy link
Member

@levitte levitte commented on 6ecbc2b May 9, 2019

Choose a reason for hiding this comment

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

CRYPTO_ALGORITHM_MIN is defined on FreeBSD:

https://github.com/freebsd/freebsd/blob/master/sys/opencrypto/cryptodev.h#L167

But you seem to be right that it isn't on OpenBSD:

https://github.com/openbsd/src/blob/master/sys/crypto/cryptodev.h

Maybe we should look at CRYPTO_ALGORITHM_MAX instead...

BTW, discussing in a single commit is pretty fruitless. Please raise an issue.

@Retropotenza
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you are right. I created #8911.

Please sign in to comment.