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

[enh] ECDH Curves #454

Merged
merged 10 commits into from
May 2, 2018
Merged

[enh] ECDH Curves #454

merged 10 commits into from
May 2, 2018

Conversation

frju365
Copy link
Member

@frju365 frju365 commented Apr 28, 2018

The problem

  • Security problem which affect Diffie-Hellman (DH) key exchange parameters

Solution

  • Add ECDH Curves

PR Status

  • Work Finished/Review needed

How to test

  • Install Yunohost :P

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

@@ -19,6 +19,7 @@ server {
ssl_certificate_key /etc/yunohost/certs/yunohost.org/key.pem;
ssl_session_timeout 5m;
ssl_session_cache shared:SSL:50m;
ssl_ecdh_curve secp521r1:secp384r1:prime256v1;
Copy link
Member

Choose a reason for hiding this comment

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

Uh could you elaborate on where those come from ? For instance for the ciphers we cite the mozilla source which is kinda trustable, since all those things are touchy security stuff...

Also how do you know this related to the DH params ? Does that mean we don't need the ssl_dhparam thingy at all ? Or are those DH params but only for eliptic curve ciphers ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

It was just to change the description from just "security problem"...

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not cyphers btw....

Copy link
Member

Choose a reason for hiding this comment

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

Alrighty, I just moved the instructions right next to the other cipher-related things, since that's the same reference ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

euh... but these are the same ecdh crurves for the two... modern or intermediate... so I thought/think it's better to set it at the top. like this the final user will be completely lost and perhaps not enable this.

@frju365
Copy link
Member Author

frju365 commented Apr 28, 2018

I need test on jessie server for the X25519 curve.

@frju365
Copy link
Member Author

frju365 commented Apr 30, 2018

I don't think the X25519 works on Jessie. Can someone just add this line on his/her server just to check. It won't break installation for information.

@frju365
Copy link
Member Author

frju365 commented May 1, 2018

ok, it's ok now.

Copy link
Member

@Psycojoker Psycojoker left a comment

Choose a reason for hiding this comment

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

LGTM

@Psycojoker Psycojoker merged commit c36431c into YunoHost:unstable May 2, 2018
@frju365 frju365 deleted the patch-2 branch May 2, 2018 11:28
@alexAubin alexAubin added this to the 2.7.x milestone May 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants