-
-
Notifications
You must be signed in to change notification settings - Fork 11k
Description
I am working on a project where I have to integrate both OpenSSL & OpenSSH and I have been experiencing a similar symptom to what it is described with the try-ciphers script in #9524 . I can SSH into my system if it's AESGCM, but not with AESCTR.
I've been trying to debug-by-printf (ugh) this situation from OpenSSH side, and I think I have some idea of what happened.
I added some tracing to various routines within OpenSSH. I discovered that the context was getting called with some IV, then we would encrypt / decrypt a handful of times, then OpenSSH tries to marshal the state (key, IV, etc) via a blob to an inferior process. When it does this, I observed it was sending out the original IV, and not the running IV.
In the LibreSSL compatibility layer of OpenSSH , there is an EVP_CIPHER_CTX_get_iv() routine. This overlaps with an OpenSSL routine of the same name, of course... so with the right defines / configuration it calls into the OpenSSL one instead. But looking at the LibreSSL routine is instructive...
int
EVP_CIPHER_CTX_get_iv(const EVP_CIPHER_CTX *ctx, unsigned char *iv, size_t len)
{
if (ctx == NULL)
return 0;
if (EVP_CIPHER_CTX_iv_length(ctx) < 0)
return 0;
if (len != (size_t)EVP_CIPHER_CTX_iv_length(ctx))
return 0;
if (len > EVP_MAX_IV_LENGTH)
return 0; /* sanity check; shouldn't happen */
/*
* Skip the memcpy entirely when the requested IV length is zero,
* since the iv pointer may be NULL or invalid.
*/
if (len != 0) {
if (iv == NULL)
return 0;
# ifdef HAVE_EVP_CIPHER_CTX_IV
memcpy(iv, EVP_CIPHER_CTX_iv(ctx), len);
# else
memcpy(iv, ctx->iv, len);
# endif /* HAVE_EVP_CIPHER_CTX_IV */
}
return 1;
}
It seems, at least to me, that the above is meant to get the running IV and not the original IV.
When I modified the OpenSSH call to EVP_CIPHER_CTX_get_iv() and instead had it call EVP_CIPHER_CTX_get_iv_state(), it fixed the problem as observed in my setup.
There is some subsequent discussion in #9524 about the merits of changing
the name(s) before beta. I'll let project members move salient discussion points along those lines into this issue as needed...