On 06/18/2013 06:04 AM, Christian Boltz wrote:
> Hello,
>
> (with >34 °C outside, I decided to spend some hours in the office ;-)
>
> Am Montag, 17. Juni 2013 schrieb nore...@launchpad.net:
>> ------------------------------------------------------------
>> revno: 7
>> committer: Kshitij Gupta <kgupta8...@gmail.com
>> branch nick: apparmor-profile-tools
>> timestamp: Tue 2013-06-18 03:49:05 +0530
>> message:
>> added severity.py with tested convert_regex and the old and new
>> config added:
>> lib/configbkp.py
>> lib/severity.py
>> modified:
>> lib/config.py
>
> Your code looks quite good, but let me add some random thoughts about
> it nevertheless ;-)
>
> === lib/config.py ===
>
>> def write_config(filename, config):
>> """Writes the given configparser to the specified file"""
>> filepath = confdir + '/' + filename
>> try:
>> with open(filepath, 'w') as config_file:
>> config.write(config_file)
>> except IOError:
>> raise IOError("Unable to write to %s"%filename)
>> else:
>> permission_600 = stat.S_IRUSR | stat.S_IWUSR # Owner read and
>> write
>> # Set file permissions as 0600
>> os.chmod(filepath, permission_600)
>
> This code has two small bugs, at least for nitpickers ;-)
> - it should write to a tempfile, check for errors and then replace the
> config_file atomically (to avoid breaking the configfile when your
> disk is full ;-)
> - the correct order is: create file, set permissions, write the content
> (your code leaves the file readable for others for a short moment -
> except if the python default permissions are 600 of course ;-)
>
> From users' POV, it might be better to keep the previous permissions
> instead of using hardcoded 600 - but 600 is more secure.
> @John: what is your opinion about this?
>
Hrmmm, being a config file that is going to be used by root/admin programs
I'd default to being as secure as possible. Not that an attack is all that
feasible since it would require launching when an admin is using the
tool, but better to be safe than sorry.
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
