I am always looking for bugs in OpenSSH as it is written in clear to read source code and has very strong security. Somehow I was suprised when I saw the new ecdsa authentication checks in OpenSSH and especially the transition to OpenSSL.
I did read the code and tested it and I doubt that OpenSSH or OpenSSL is vulnerable at all. I cannot verify this without a strong understanding of the math involved. Please don’t laugh at me if this leads to thin air.
Ecdsa is the authentication method used by newer OpenSSH versions aside with RSA and DSA public key authentication.
It uses elliptic curves and points on these elliptic curves to verify the connection is valid and authenticate. Inside the OpenSSH code I see that it will accept at least three curve types from the client. These are nistp256,nistp384 and nistp521 being identified by their bit count. nistp256 is used as a default curve. I could not validate if other curve types that are supported by OpenSSL can be used by a remote connection to go through OpenSSH authentication based on their bit count, as far as I have seen OpenSSH checks the bit count of the requested curve. I tried to supply OpenSSH with two dimensional curves that are supported by OpenSSL but it didn’t exactly succeed. Why is this important in a potential attack?
Because OpenSSH calls two functions in the transition to OpenSSL to validate authentication based on group types and more identifiers, these are:
EC_GROUP_cmp
EC_POINT_cmp
EC_GROUP_cmp should validate if two groups (the curves) are equal to each other.
Fun is that the top check done is checking the curve name, which again is the bit count of the curves supplied:
int EC_GROUP_cmp(const EC_GROUP *a,const EC_GROUP *b,
BN_CTX *ctx) 

…  
479  /* compare the field types*/ 
480  if (EC_METHOD_get_field_type(EC_GROUP_method_of(a)) != 
481  EC_METHOD_get_field_type(EC_GROUP_method_of(b))) 
482  return 1; 
483  /* compare the curve name (if present) */ 
484  if (EC_GROUP_get_curve_name(a) && EC_GROUP_get_curve_name(b) && 
485  EC_GROUP_get_curve_name(a) == EC_GROUP_get_curve_name(b)) 
486  return 0; 
If the bit count is equal then OpenSSL will return zero as a value for success. This is not particular wrong behaviour, still akward that it only checks for the bit count when it should check the curve itself. For me a curve is like a graph and the bit count is only a small factor of the real information that should be checked before returning success.
The second function OpenSSH calls for verifiying authentication is EC_POINT_cmp. This function compares two EC_POINT values that encode x,y and z coordinates as OpenSSL BIGNUM types, the first originates from the authorized_keys file, the second from the client.
This function has more suspicious code flow. It returns success in comparison of the points if the curves method doesn’t have a point compare function. I know that this is internal audit code but still it shouldn’t return zero indicating success to OpenSSH, a return value of 1 should be fine.
The second check in EC_POINT_cmp will return success in comparison if the curves method is different from the first points method or if the first points method is different from the second points method. Especially the second check is interesting because point two is user supplied. Again a return value of 1 would be secure even if this might be internal audit code.
int EC_POINT_cmp(const EC_GROUP *group,const EC_POINT *a,
const EC_POINT *b, BN_CTX *ctx) 

992  { 
993  if (group>meth>point_cmp == 0) 
994  { 
995  ECerr(EC_F_EC_POINT_CMP, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED); 
996  return 0; 
997  } 
998  if ((group>meth != a>meth)  (a>meth != b>meth)) 
999  { 
1000  ECerr(EC_F_EC_POINT_CMP, EC_R_INCOMPATIBLE_OBJECTS); 
1001  return 0; 
1002  } 
1003  return group>meth>point_cmp(group, a, b, ctx); 
1004  } 
When all these checks are passed it will call the curves point compare routine that is bound to the curve type.
The default case is that ec_GFp_simple_cmp function gets called for a nistp256 curve.
This function checks at the very top if point a is at its infinity. If that is the case and point b is at its infinity too than zero is returned again as success. It might sound undoable but what if somebody manages to set both points to its infinity by manipulating either one of the points or the curve. A point is at infinity if its Z coordinate is zero.
Here is an interesting commit involving the function:
http://rt.openssl.org/Ticket/Display.html?id=1612&user=guest&pass=guest
These are the two authentication checks done in OpenSSH to verify a users certificate, here is the check in key.c:
299  if (EC_GROUP_cmp(EC_KEY_get0_group(a>ecdsa), 
300  EC_KEY_get0_group(b>ecdsa), bnctx) != 0  
301  EC_POINT_cmp(EC_KEY_get0_group(a>ecdsa), 
302  EC_KEY_get0_public_key(a>ecdsa), 
303  EC_KEY_get0_public_key(b>ecdsa), bnctx) != 0) { 
304  BN_CTX_free(bnctx); 
305  return 0; 
306  } 
It much depends on the order the arguments are given to the OpenSSL functions because if EC_POINT_cmp takes the curve (named group here) as first argument it will do run authentication checks with this supplied curve. OpenSSH does that correctly by supplying the curve from the authorized_keys file as first argument whereas proftpd mod_sftp does supply the clients curve as the first argument and moreover the clients point BIGNUM coordinates as the first point in the arguments.
I personally do not trust this code, not because i don’t believe it’s secure but because it bluntly returns authentication success to OpenSSH when making internal audit checks. My personal opinion is that OpenSSH does not deserve this entry point of insecurity.
Cheers,
Kingcope
Hi, Thanks for looking. Fortunately, none of worrisome code should be reachable in OpenSSH.
From the top:
1) The EC_GROUP_cmp checks.
OpenSSH will only create EC public keys from a small, predefined set of curves. There are no two different curves with the same bitcount that could trick this code into erroneously returning 0 when comparing them.
2) (group>meth>point_cmp == 0)
Again, we only use the standard predefined NIST 256, 384 and 521bit curves so these will all have a point_cmp function defined.
3) ((group>meth != a>meth)  (a>meth != b>meth))
All the curves used by OpenSSH are GF(p) curves, so they will all have the same group>meth. We refuse to parse EC public values of other curve types (of which only GF(2^m) are supported by OpenSSL) so this case can’t be hit either.
4) ec_GFp_simple_cmp / points at infinity
We refuse any public EC value that is a point at infinity. See key.c key_from_blob => key_ec_validate_public() This is called both for keys from the network and from authorized_keys.
5) Cert verification.
The code you list from key.c is just key equality; keys from the network or authorized_keys still have to pass validation and the client must produce a valid signature using their private key. IMO this is a much harder path to get through than just the equality test.
d
ab 4 lyfe