Skip to content
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

Use mpint representation for shared_secret when deriving keys in pure-PQ key exchange, and some other bug fixes; fixes #119 #120

Merged
merged 6 commits into from Feb 17, 2022

Conversation

kevinmkane
Copy link

Other bug fixes:

  1. Increase MAX_PROP to 160. Large numbers of proposals due to all the OQS algorithms can cause a non-spec-compliant selection of algorithm if this value is too low and client and server order their proposals differently. This had been fixed in v7 by Increase MAX_PROP to 140 #85 but returned in the v8 branch.
  2. When calling ssh-keygen with the -t parameter to specify the key type for one of the ECDSA hybrids, but using the short name instead of the full ssh-ecdsa-xxxxxxx name, ECDSA keys would always be generated on P-256. Instead, always use the specified curve.
  3. kex_kem_generic_{enc,dec} could use an uninitialized variable as the return value if the call to the underlying liboqs function failed. On the small chance that variable happened to have a zero in it, this would look like a successful result to teh caller. Return SSH_ERR_LIBCRYPTO_ERROR in these cases instead.

MAX_PROP limits the number of kex algorithm proposals considered from the server by the client. With the liboqs options exceeding this number, an unfortunate ordering in the server's proposal can cause OpenSSH to pick the wrong kex in violation of the RFC.

This change increases MAX_PROP to 160, which will allow for a longer list including all liboqs options.
…gorithm, always use the curve specified by the key type

If ssh-keygen is called with the -t parameter using the short name for one of the OQS hybrid algorithms, P-256 incorrectly always ends up being used for the ECDSA part of the key.
… OQS_KEM_{en,de}caps fails

r is uninitialized otherwise.
kexoqs.c Outdated Show resolved Hide resolved
@kevinmkane kevinmkane marked this pull request as ready for review February 17, 2022 16:22
@kevinmkane
Copy link
Author

3. kex_kem_generic_{enc,dec} could use an uninitialized variable as the return value if the call to the underlying liboqs function failed. On the small chance that variable happened to have a zero in it, this would look like a successful result to teh caller. Return SSH_ERR_LIBCRYPTO_ERROR in these cases instead.

Actually I'm wrong about this: it's worse. The r variable will be initialized, but to zero based on previous calls succeeding that assign to it. So it's certain failures in the calls into liboqs won't get caught. The fix is still correct, fortunately.

@dstebila dstebila merged commit e9b0f6f into open-quantum-safe:OQS-v8 Feb 17, 2022
@dstebila
Copy link
Member

Thanks @kevinmkane!

@kevinmkane kevinmkane deleted the libssh-v8-fixes branch February 17, 2022 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants