Merge lp:~andreserl/maas/maas_correct_paths into lp:maas/trunk

Proposed by Andres Rodriguez on 2012-09-19
Status: Merged
Approved by: Andres Rodriguez on 2012-09-19
Approved revision: 1030
Merged at revision: 1030
Proposed branch: lp:~andreserl/maas/maas_correct_paths
Merge into: lp:maas/trunk
Diff against target: 14 lines (+2/-2)
1 file modified
src/maasserver/models/node.py (+2/-2)
To merge this branch: bzr merge lp:~andreserl/maas/maas_correct_paths
Reviewer Review Type Date Requested Status
Scott Moser 2012-09-19 Approve on 2012-09-19
Review via email: mp+125286@code.launchpad.net

Commit Message

Set correct paths for virsh and ipmipower

To post a comment you must log in.
Scott Moser (smoser) wrote :

I grudgingly approve this.

However, the right fix to this problem is to *NOT* specify full paths.
Trust your environment instead. By trusting the PATH environment variable to contain the right paths, you get:
 * the ability to easily replace a program with a preferred version in a different path
 * the stability to not fail when a program is not in the hard coded path
 * no reduced security

I'm sure that the reader of this rant might for some strange reason believe that hard coding paths provides a sense of security. That is, however, extremely flawed belief. If a attacker has influenced the environment of your process, you're already compromised and hard coding strings will not help you. If I could influence the environment of your process, i could set LD_PRELOAD and modify 'open' or 'execve' to do what i wanted.

Please stop doing silly things.

Scott Moser (smoser) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/node.py'
2--- src/maasserver/models/node.py 2012-09-18 18:25:00 +0000
3+++ src/maasserver/models/node.py 2012-09-19 17:37:24 +0000
4@@ -581,8 +581,8 @@
5 power_params = {}
6
7 power_params.setdefault('system_id', self.system_id)
8- power_params.setdefault('virsh', '/usr/sbin/virsh')
9- power_params.setdefault('ipmipower', '/usr/bin/ipmipower')
10+ power_params.setdefault('virsh', '/usr/bin/virsh')
11+ power_params.setdefault('ipmipower', '/usr/sbin/ipmipower')
12 power_params.setdefault('power_address', 'qemu://localhost/system')
13 power_params.setdefault('username', '')
14 power_params.setdefault('power_id', self.system_id)