Skip to content

ssl: fix elliptic curve selection in server mode #183

Closed
wants to merge 1 commit into from

3 participants

@RoadRunnr

The server code erroneously took the list of curves supported by the
client from it's own hello extension, effectively break selection
all together.

Also the default fallback secp256k1 curve is not supported by
all clients. secp256r1 is recommended as part of the NIST Suite B
cryptographic suites. The changes are much better that all clients
support it, so use that as fallback.

@OTP-Maintainer

Patch has passed first testings and has been assigned to be reviewed

@RoadRunnr RoadRunnr ssl: fix elliptic curve selection in server mode
The server code erroneously took the list of curves supported by the
client from it's own hello extension, effectively breaking curve
selection all together.

Also the default fallback secp256k1 curve is not supported by
all clients. secp256r1 is recommended as part of the NIST Suite B
cryptographic suites. The chances are much better that all clients
support it, so use that as fallback.
6f4f02e
@RoadRunnr
  • updated with a slightly reworded commit message
  • fixed the change to select_curve/1 (matching the record in the older version was not intended)
@OTP-Maintainer

Patch has passed first testings and has been assigned to be reviewed

@proxyles

aha! was just going to point out the warning in ssl_connection.erl

Next time, update the original branch instead of making a new one. It took me a little while before i found it.

When a pullrequest is handled by OTP-Maintainer, it is automatically added into our system, with links to git and fetch links, so if you make a new branch i will have to find in manually and update, instead of just using the ones already in the system.

Thanks!

@RoadRunnr

Next time, update the original branch instead of making a new one. It took me a little while before i found it.

That's strange, it did not create a new branch, I force pushd the updated cset to my original branch.

@proxyles

aha.

This commit "ssl: fix elliptic curve selection in server mode" is also part of #120
which is the branch I was testing.

It is also present here, but in a updated way. so the branch "ecdh_crypto" contains a old version of this commit.

Can we stick to having commits up to date and in one place? ;)

Will you update #120 with this version of this commit, or remove it from #120 and have it separately in this PR

@RoadRunnr

Can we stick to having commits up to date and in one place? ;)

definitely! having it in #120 was a mistake, probably got in during testing.

I have removed that cset from #120. Having it separately makes more sense, it really is unrelated to the new functionality in #120.

@proxyles

Graduated

@proxyles proxyles closed this Jan 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.