/yunohost

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] Generate custom Diffie-Hellman primes #621

Open
wants to merge 19 commits into
base: stretch-unstable
from

Conversation

Reviewers

@Psycojoker Psycojoker

@alexAubin alexAubin

@Josue-T Josue-T

Assignees
No one assigned
Labels
test needed work needed
Projects
None yet
Milestone
No milestone
4 participants
@frju365 @Psycojoker @alexAubin @Josue-T
@frju365
Copy link
Member

frju365 commented Jan 19, 2019 •
edited

The problem

Weak dh_params == weak security

Solution

Add dh_params in background

PR Status

How to test

Install Yunohost / ynh-dev

Validation

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

frju365 added some commits Jan 19, 2019

@frju365
dh_params
Verified
This commit was signed with a verified signature.
@frju365 frju365 frju365
GPG key ID: C892BE847FF5635A Learn about signing commits
Loading status checks…
be53f76
@frju365
dh_params (forget :) )
Verified
This commit was signed with a verified signature.
@frju365 frju365 frju365
GPG key ID: C892BE847FF5635A Learn about signing commits
Loading status checks…
077f413
@frju365
typo
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
3bc82d6
@Psycojoker

Psycojoker reviewed Jan 20, 2019
View changes

Show resolved Hide resolved src/yunohost/tools.py
@Psycojoker

Psycojoker reviewed Jan 20, 2019
View changes

Show resolved Hide resolved src/yunohost/tools.py Outdated
@frju365
Update tools.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
7dcef3a
@Psycojoker

Psycojoker reviewed Jan 20, 2019
View changes

Show resolved Hide resolved src/yunohost/tools.py Outdated
@Psycojoker

This comment has been minimized.

Sign in to view
Copy link
Member

Psycojoker commented Jan 20, 2019

This PR misses relaunching the regen-conf or equivalent mechanism for including this custom dhparam in nginx confs.
@frju365
avoid error log
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4d446b2
@frju365

This comment has been minimized.

Sign in to view
Copy link
Member Author

frju365 commented Jan 20, 2019

I add regen-conf at the end of postinstall but I can't be sure it will be generated at the end of it (surely not).

frju365 added some commits Jan 20, 2019

@frju365
redirect log to file /var/log/yunohost/dhparam_generation.log
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9789b9c
@frju365
add render for Yunohost-admin
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
f16d42e

@alexAubin alexAubin changed the title dh_params [enh] Generate custom Diffie-Hellman primes Jan 26, 2019

@alexAubin

alexAubin reviewed Feb 5, 2019
View changes

Show resolved Hide resolved data/hooks/conf_regen/15-nginx Outdated
src/yunohost/tools.py Outdated
@@ -322,6 +322,17 @@ def tools_postinstall(operation_logger, domain, password, ignore_dyndns=False,
else:
dyndns = False

cron_job_file = "/etc/cron.hourly/yunohost-generate-dh-params"

command = "nice -n 19 openssl dhparam -out /etc/ssl/private/dh2048.pem -outform PEM -2 2048 -dsaparam 2> /var/log/yunohost/dhparam_generation.log && rm /etc/cron.hourly/yunohost-generate-dh-params\n"

This comment has been minimized.

Sign in to view
Copy link
@alexAubin

alexAubin Feb 5, 2019

Member

Note that a migration should be implemented so that it also gets applied to existing instances

This comment has been minimized.

Sign in to view
Copy link
@Josue-T

Josue-T Feb 8, 2019

Contributor

You also probably need to change the owner for this file like : root:ssl-cert

This comment has been minimized.

Sign in to view
Copy link
@frju365

frju365 Feb 8, 2019

Author Member

mh ? On my instance it's root:root, and it works. If you think it's better root:ssl-cert... but it works like that (perhaps I miss something).

This comment has been minimized.

Sign in to view
Copy link
@alexAubin

alexAubin Feb 8, 2019

Member

That essentially depends of wether or not we will need some other service to use this DH param. For instance, SSL certificates have root:ssl-cert as permission because metronome needs access to it while not having root privileges... I don't know if dovecot or postfix or metronome or whatever service needs DH params but having ssl-cert as group for this file provides more flexibility I guess

This comment has been minimized.

Sign in to view
Copy link
@Josue-T

Josue-T Feb 8, 2019

Contributor

I say this because I'll need this for synapse 😃

This comment has been minimized.

Sign in to view
Copy link
@frju365

frju365 Feb 8, 2019

Author Member

normalement corrigé.

This comment has been minimized.

Sign in to view
Copy link
@alexAubin

alexAubin Feb 17, 2019

Member

Note that the original comment is still valid : a migration should be implemented so that it also gets applied to existing instances

This comment has been minimized.

Sign in to view
Copy link
@frju365

frju365 Feb 18, 2019

Author Member

yes, I know. I just didn't have time to do it :/ Let's go to try.

This comment has been minimized.

Sign in to view
Copy link
@alexAubin

alexAubin Feb 18, 2019

Member

That's okay 😉 Just be aware of it 😛

This comment has been minimized.

Sign in to view
Copy link
@frju365

frju365 Feb 18, 2019

Author Member

Normally, quite done :/ Not tested as always (I'm at work for now).

@alexAubin alexAubin added work needed test needed labels Feb 5, 2019

frju365 added some commits Feb 5, 2019

@frju365
Update 15-nginx
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
761a01b
@frju365
Update tools.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
b7e97a7
@frju365
Strange migration
Verified
This commit was signed with a verified signature.
@frju365 frju365 frju365
GPG key ID: C892BE847FF5635A Learn about signing commits
Loading status checks…
c4cd135
@frju365
Update 0009_dh_params.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
ed85df2
@frju365
indentation
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
b971fff
@frju365
remove useless import
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
d820cd8
@alexAubin

alexAubin requested changes Feb 18, 2019
View changes

Show resolved Hide resolved src/yunohost/data_migrations/0009_dh_params.py Outdated
Show resolved Hide resolved src/yunohost/data_migrations/0009_dh_params.py Outdated
Show resolved Hide resolved src/yunohost/tools.py Outdated
Show resolved Hide resolved data/hooks/conf_regen/15-nginx Outdated

frju365 added some commits Feb 18, 2019

@frju365
Update 0009_dh_params.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
04fdb9a
@frju365
Update tools.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
d9f19dd
@alexAubin

alexAubin reviewed Feb 18, 2019
View changes

Show resolved Hide resolved src/yunohost/data_migrations/0009_dh_params.py Outdated

frju365 added some commits Feb 18, 2019

@frju365
Update 0009_dh_params.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
29bc912
@frju365
Do some change
Verified
This commit was signed with a verified signature.
@frju365 frju365 frju365
GPG key ID: C892BE847FF5635A Learn about signing commits
f2ad8cb
@frju365

This comment has been minimized.

Sign in to view
Copy link
Member Author

frju365 commented Feb 18, 2019

Why is there conflict ?
@frju365
correc
Verified
This commit was signed with a verified signature.
@frju365 frju365 frju365
GPG key ID: C892BE847FF5635A Learn about signing commits
99c83f5
@alexAubin

alexAubin reviewed Feb 18, 2019
View changes

Show resolved Hide resolved src/yunohost/data_migrations/0009_dh_params.py Outdated
@frju365
Update 0009_dh_params.py
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
4660c60
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment