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] use setting security_ciphers_compatibility to define security configu… #640

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

Conversation

Projects
None yet
2 participants
@rds13
Copy link

rds13 commented Feb 4, 2019

This PR is proposed following discussion in #581

The problem

Try to support modern configuration for admin willing to.

Solution

Define settings 'security.ciphers.compatibility'

PR Status

Tested

How to test

Modern settings for web

  • Change default security policy
    yunohost settings set security.ciphers.compatibility -v modern

  • Add a domain to have yunohost build nginx conf file
    yunohost domain add yunohostdev1.host

  • Ensure you already have an application or install one
    yunohost app install opensondage

  • Check configuration is applied on nginx
    cat /etc/nginx/conf.d/yunohostdev1.host.conf
    Ensure only TLS v1.2 is allowed.

Intermediate settings for web

  • Reverse security policy to default
    yunohost settings set security.ciphers.compatibility -v intermediate

  • Check yunohost have rebuild nginx conf file

  • Check configuration is applied on both nginx configuration
    cat /etc/nginx/conf.d/yunohostdev1.host.conf
    Ensure TLSv1 TLSv1.1 TLSv1.2 are supported.

Intermediate settings for ssh

  • Set intermediate policy for ssh security
    yunohost settings set service.ssh.ciphers.compatibility -v intermediate

  • Check SSH configuration have been regenerated

  • Check new SSH configuration
    cat /etc/sshd/sshd_config

Validation

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

@rds13 rds13 changed the title use setting security_ciphers_compatibility to define security configu… [enh] use setting security_ciphers_compatibility to define security configu… Feb 7, 2019

Show resolved Hide resolved locales/fr.json Outdated
Show resolved Hide resolved src/yunohost/settings.py Outdated
Show resolved Hide resolved data/templates/ssh/sshd_config Outdated
Show resolved Hide resolved data/templates/nginx/server.tpl.conf Outdated
Show resolved Hide resolved data/hooks/conf_regen/15-nginx Outdated
("service.ssh.ciphers.compatibility", {"type": "enum", "default": "modern",
"choices": ["intermediate", "modern"]}),
("security.ciphers.compatibility", {"type": "enum", "default": "intermediate",
"choices": ["intermediate", "modern"]}),

This comment has been minimized.

@alexAubin

alexAubin Feb 14, 2019

Member

Oh and then the question which goes beyond this PR is : what do we want to do when those settings are modified ... So e.g. when calling yunohost setting set security.ciphers.compatibility modern (whatever the exact syntax), ssh or nginx conf shall be regenerated ... we can't expect the user to guess that.

So, when this setting system was first designed, we though of using a decorator for this to create something like :

@when_changing("some_setting_name")
def some_function_name():
   # [... stuff to do after the setting gets changed ...]

so that for this case that would be something like :

@when_changing("security.ciphers.compatibility")
def some_function_name():
    service_regen_conf("nginx")

So this @when_changing decorator needs to be defined somewhere so that after using setting_set, the functions decorated in that way are called 😉

Dunno if you are at ease with decorators and understand this whole thing, otherwise I can work on it at some point.

This comment has been minimized.

@rds13

rds13 Feb 14, 2019

Author

I understand the mecanism but I'm not confortable enough with Yunohost stack to handle that at the moment.
If it's a new mecanism to design I bet you or someone with more experience on this subject is to be needed.

This comment has been minimized.

@alexAubin

alexAubin Feb 19, 2019

Member

So I implemented the mechanism in #654

If the design does not change, it should be as simple as adding this kind of lines :

@post_change_hook("service.ssh.ciphers.compatibility")
def foo(old_value, new_value):
    # (We don't really care about the old and new value for that use case)
    service_regen_conf(["ssh"])

This comment has been minimized.

@rds13

rds13 Feb 20, 2019

Author

As soon as I am able to test this mecanism in the context of this PR, I will report here. Thx @alexAubin for your help.

This comment has been minimized.

@rds13

rds13 Feb 21, 2019

Author

I have updated this PR to use post_change_hook. It works like a charm ! 👍

@rds13

This comment has been minimized.

Copy link
Author

rds13 commented Feb 23, 2019

Shall we need to document this feature in some documentation ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment