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

Removed DHE SSL ciphers in TLS RSA key exchange #274

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

Rocketct
Copy link
Contributor

Removed DHE SSL ciphers to fix issue #270

cc:\ @sandeepmistry tested

@sandeepmistry
Copy link
Contributor

Let's see what feedback Microchip has on these changes.

@adriens
Copy link

adriens commented Jun 27, 2019

🙏 I'll give it a try pretty soon as I jave received some new hardware (I don't want to break the actual working one) . Thanks for this 👍

Copy link
Contributor

@sandeepmistry sandeepmistry left a comment

Choose a reason for hiding this comment

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

Tested with the following domains + WiFiSSLClient example sketch:

Looks good!

@sandeepmistry
Copy link
Contributor

@gvarisco any opinions on this change?

With the DHE ciphers enabled the connection to maker.ifttt.com would fail because the server had a 512 byte public key which the WINC1500 firmware can't handle.

Michele from Microchip is also in agreement with this change:

Anyway it looks to be good to me.

I believe that, originally, the TLS client was somehow limited to use cipher suites that it could not actually support.

By adding the RSA for authentication mechanism, 128 AES for encryption and 256 SHA for integrity we are well in between the winc1500 boundaries.

@adriens
Copy link

adriens commented Jun 28, 2019

@sandeepmistry : could you give a try to https://www.heroku.com/ ?... I have some REST services ther I'd like to consume ;-p

@gvarisco
Copy link

@sandeepmistry +1, I'm happy to remove DHE-based ciphers. LGTM.

@sandeepmistry sandeepmistry merged commit 3301d03 into arduino-libraries:master Jul 2, 2019
@sandeepmistry
Copy link
Contributor

@gvarisco great, thanks for the feedback!

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.

4 participants