Rudder

I'm not sure this is really applicable to the 2.3 branch, is it? Adding new policies to the distributePolicy PT seems risky at this stage in the 2.3 lifecycle.

Converting to a user story (to improve security) for 2.4.0.

Task is over, commited here : http://www.rudder-project.org/redmine/projects/policy-templates/repository/revisions/dcac738a247455d55680b5192cc0ff944980cc73

( I had a hard time before realizing that using templates, the world is just sooooo better ...)

This is ok, however I'm a bit puzzled by the root:root 644 permission of the /etc/apache2/vhosts.d/rudder-default.conf

Shouldn't it be owned by the apache user and with permission 500 ?
( http://www.linuxforums.org/forum/servers/5270-apache-conf-html-permissions.html )

Applied in changeset commit:dcac738a247455d55680b5192cc0ff944980cc73.

• log_error is not a valid log level, only result_error exists (any error is a result)
• I do not think that we should enforce the full content of the Rudder VHost in Apache. We only provide this VHost as a convenience, and many people will modify it (to add SSL support for example, like our internal Rudder server). Therefore we mustn't control it with CFEngine completely. Can you change this to use a edit_lines body that matches a section after "<Directory /var/rudder/inventories/incoming>" and deletes any "Allow .*" lines and adds in the right ones?

Jonathan CLARKE wrote:

I am bothered by several things in this change:

• log_error is not a valid log level, only result_error exists (any error is a result)
• I do not think that we should enforce the full content of the Rudder VHost in Apache. We only provide this VHost as a convenience, and many people will modify it (to add SSL support for example, like our internal Rudder server). Therefore we mustn't control it with CFEngine completely. Can you change this to use a edit_lines body that matches a section after "<Directory /var/rudder/inventories/incoming>" and deletes any "Allow .*" lines and adds in the right ones?

OK for the log_error, I will commit a fix now.

As for the templating, Well I spent quite a lot of time trying to use anchors to no avail. That is the reason I chose this model. I can not easily use an edit lines with this file for quite a lot of reasons (convergeance principally).

However, I got an idea: I might be able to enforce the content of a specific file containing allow statements, and include this file in the main configuration. That would be the perfect fix. What do you think about that ?

This sounds like a good approach - best of all worlds.

However, this is really not imperative for our 2.4.0~beta1 release, so I suggest we revert this in the 2.4 branch and continue work on it for 2.5.

Applied in changeset commit:e11c943310e08f5df1dc03cac0968c1d0c0a3329.

Applied in changeset commit:105cf58187561f51702fa43887b98a2334d422e6.

I don't like this change to packaging: you're creating a file by cat'ing into it, which means that from the package manager's point of view, that file is not controlled by any package. This is against the principle of packaging that says that "all files created by a package should be owned by it". In particular, if you remove the rudder-webapp package, this file would be left behind.

Can you fix this to include this file as a standard source file please?

Yes, I can (tm) (r) (c)

Applied in changeset commit:2ccfcc20af4775b03d6f976db6e6e9b54c78e1c3.

This is much better, thanks.

• You haven't marked the file as a conffile in debian packaging!
• Is "Allow from all" really a sane default for a security setting?

All clear. The debian conffile has been adjusted.

As with the "Allow from all", it is necessary: the server would not be able to accept any node before a root server promises regeneration has been done...

Or maybe you think that the server should not be able to receive reports before this regeneration ?

Matthieu CERDA wrote:

All clear. The debian conffile has been adjusted.

As with the "Allow from all", it is necessary: the server would not be able to accept any node before a root server promises regeneration has been done...

Or maybe you think that the server should not be able to receive reports before this regeneration ?

I think that the default should be "closed", because that's the safest. Then, I think we should open it up when allowed networks are defined, ie in rudder-init.sh.

Okay then, a simple "Deny from all" should be ok in the packaging initial rudder-networks.conf file.

This appears to be all done and I have validated the technical review. Thank you Matthieu.