Request for new upstream version 1.2 upgrade.

Debdiff is attached here.

Most recent portion of changelog:

wakeup (1.2-0ubuntu1) precise; urgency=low

  * New upstream release (LP: #909189).
   - Changed weather source to google using python-pywapi
   - Added location.py in wakeup directory as plugin helper
   - Added plugin "Commands" which allows arbitrary user dataitems
   - Changed HebrewCalendar to use location from location.py
   - fixed problems to do with hard-coded DISPLAY variable
   - Small bug fixes
  * Removed all perl dependencies

thanks for your contribution to ubuntu.

I modified the debdiff a bit adding this:
  * wrap-and-sort debian/
  * convert copyright to dep5 format
  * use dh_python2 instead of pysupport

please review the changes I made and check if the package still works correctly. the largest change is the use of dh_python2 instead of the deprecated pysupport.

I don't like the uses of os.system in the source e.g.
os.system(wakeup_script + " $USER " + re.search("\d+",folder).group(0) + " test &")
and many more.
I'm not sure if those can be used to for priviledge escalations but I still prefer those unsafe system calls to be fixed by using the subprocess module (without shell=True) before I upload this.

Okay, so the changes now are:

wrap-and-sort debian/
converted to dep5 format
used dh_python2
removed all calls to os.system and commands.get(status)output
added some other bug fixes

The new changelog is as follows:

wakeup (1.2-0ubuntu1) precise; urgency=low

  * New upstream release (LP: #909189).
   - Changed weather source to google using python-pywapi
   - Added location.py in wakeup directory as plugin helper
   - Added plugin "Commands" which allows arbitrary user dataitems
   - Changed HebrewCalendar to use location from location.py
   - fixed problems to do with hard-coded DISPLAY variable
   - fixed issues with stopping the alarm
   - removed calls to os.system and commands.get(status)output
   - small bug fixes
  * Updated packaging
   - Removed all perl dependencies
   - wrap-and-sort debian/
   - converted copyright to dep5 format
   - use dh_python2 instead of pysupport

The new debdiff is attached.

thanks for incorporating my suggestions. (note subprocess.check_ouput only works with python2.7 which is fine in ubuntu but maybe not for all your other users)

I saw another issue with insecure temporary file use in setnextalarm.py and alarm.py and most scripts.
please use tempfile.TemporaryFile in python and mkstemp in shellscripts so the tempfiles cannot be abused via race conditions.
E.g. the tmpfile data/scripts/wakeup is exploitable for privilige escalation and needs a security update in ubuntu oneiric.

Also please make sure the debdiff applies against the package currently in ubuntu, your last diff does not apply against debian/changelog and debian/control

Thanks for looking through this all carefully, I really appreciate it.

Does this apply to temporary files kept within the user's home directory? The temporary file created in wakeup-settings (tmpPlayFile), for instance is contained in ~/.wakeup/ and should be accessible only by the user and root.

tempfiles in ~ are ok if they have correct permissions. But their content should not be used by privileged processes unverified (especially not as roots crontab).

Okay, thanks. Just for clarification, do such temporary files need to be generated randomly? There are 2 files I am specifically concerned about. These are the files which play the actual alarms (ie, the alarms are executable bash scripts called by /usr/bin/wakeup):

1. ~/.wakeup/playable_tmp: this is generated by wakeup-settings via alarm.py and is made executable, but is never run with elevated access. It is deleted as soon as the wakeup-settings window is closed. Does this have to have a random file name?
2. ~/.wakeup/alarm[#]/playable_text: this is also generated by wakeup-settings via alarm.py and made executable, but is run from cron, either root's or user's depending on whether the boot-for-alarm option is checked. This must be generated in a random file every time the alarm is run?

within home the filename is not important as only the user and root should have access to it. So 1. should be no problem
2. on the other hand can be if the script, which will be executed by root, can be edited without root rights.
the script run by root must only be writeable by root.

Hm, okay. A sudo password is requested to create the file if it will be executed by root. Does this mean that as long as I alter the permissions of the file upon creation such that only root can write to it later (chown root; chmod 700), then it should be secure?

you must create it with the right permissions to begin with.
Else you have race condition where a unprivileged user could write to the file before the permissions are changed.

Okay, the temporary files I believe are now secure. Any files created in /tmp are made using mktemp (in bash scripts) or tempfile (in python scripts). The playable_text file is created (owned) by root and chmod 700 prior to writing if it will be run from root's cron.

I've made a new release on launchpad, with the source tarball. The changelog now reflects that this is now responding also to bug 912762 (see below). Debdiff is attached.

wakeup (1.2-0ubuntu1) precise; urgency=low

  * New upstream release (LP: #909189).
   - Changed weather source to google using python-pywapi
   - Added location.py in wakeup directory as plugin helper
   - Added plugin "Commands" which allows arbitrary user dataitems
   - Changed HebrewCalendar to use location from location.py
   - fixed problems to do with hard-coded DISPLAY variable
   - fixed issues with stopping the alarm
   - removed calls to os.system and commands.get(status)output
   - use secure temp files (LP: #912762)
   - root-owned chmod 700 playable_text file for boot alarms
   - small bug fixes
  * Updated packaging
   - Removed all perl dependencies
   - wrap-and-sort debian/
   - converted copyright to dep5 format
   - use dh_python2 instead of pysupport

I still don't like it, there are race conditions between opening files and making them root only all over the place.
also you create tempfiles with user permissions but which are then used by root.
sudo mktemp will create files with proper permissions.

For the crontab updating I would not go to the filesystem at all, instead I would pipe in the new result via stdin:
(sudo crontab -l; echo "some new cron") | sudo crontab -

or the equivalent with python subprocess (w/o sudo)
import subprocess
curcron = subprocess.check_output(["crontab", "-l"])
# on non-debian systems you might get 3 lines of headers out of crontab -l
strip_headers(curcron)
updatecron = subprocess.Popen(["crontab", "-"], stdin = subprocess.PIPE)
updatecron.communicate(curcron+"1 0 * * * echo\n")

just to illustrate why creating a file and then chmod'ing it is wrong.

I by accident start a script with my normal user permissions which contains this:
file = inotifywait -e create /path/to/wakup/tmpdir | grep CREATE | awk '{$3}'
echo "... do-bad-stuff" >> file

this can now win the race condition and write arbitrary data to the file before wakup chmod's the file to be only root rightable.
now the script gained root access while it before only had normal user access.

better create the file with the correct permissions in the first place, or truncate the file when root writes to it.

Okay, I can remove as many temporary files as possible. I'm not sure which temporary files you're still worried about, though. I thought that the way I have it temp files are only used by root if they are created by root. For instance, setnextalarm.py is only ever called by root.

this part looks sketchy, if it is not executed as root in total:
+ if self.wakecomputer and not isTmpFile:
+ f = tempfile.NamedTemporaryFile()
+ f.write(final_text)
+ f.seek(0)
+ #subprocess.call(['gksudo', '--message', 'testing', 'echo'])
+ subprocess.call(['sudo', 'cp', '--remove-destination', f.name, filename])
+ subprocess.call(['sudo', 'chmod', '700', filename])
+ f.close()

I believe I have fixed these - the only temporary files still used in /tmp are never used as executables (made by voice_list.sh). For the sketchy creation of the root playfile, I now have a separate script to create and write to the file which is run as root so that the file is created with root permissions before writing to.

In fixing these, however, I noticed that gksudo/sudo has been giving recent problems. In particular, calls to sudo fail even after calls to gksudo IF wakeup-settings is not run through a terminal. I decided to try to migrate to using pkexec, since this seems the more up-to-date approach anyway, but it is poorly documented. I tried creating a .policy file which would allow only the commands needed (crontab, setalarm, setnextalarm.py, createRootPlayfile.py) with an allow_active default as auth_admin_keep for a "meta-action" defined using annotation org.freedesktop.policykit.imply, but every action defined using this method needs to be separately authenticated.

I've attached the .policy file. Do you have suggestions on how to fix this? The behavior I get is:
1. authenticate com.ubuntu.wakeup.exec using (prompts for password for com.ubuntu.wakeup.exec, is_auth returns True):
pid=os.getpid()
action_id='com.ubuntu.wakeup.exec'
service = dbus.SystemBus().get_object('org.freedesktop.PolicyKit1', '/org/freedesktop/PolicyKit1/Authority')
policy_kit=dbus.Interface(service, 'org.freedesktop.PolicyKit1.Authority')
(granted, _, details) = policy_kit.CheckAuthorization(('unix-process', {'pid':dbus.UInt32(pid, variant_level=1), 'start-time':dbus.UInt64(0,variant_level=1)}),action_id,{},dbus.UInt32(1),'',timeout=600)
(is_auth, _, details) = policy_kit.CheckAuthorization(('unix-process', {'pid':dbus.UInt32(pid, variant_level=1), 'start-time':dbus.UInt64(0,variant_level=1)}),action_id,{},dbus.UInt32(0),'',timeout=600)
2. try calling subprocess.call(['pkexec', 'setnextalarm.py'])
    ---> get pkexec popup asking for authentication for com.ubuntu.wakeup.exec.setnextalarm

You can more or less ignore the last post. I'm attaching the debdiff. Note that I have moved from gksudo to pkexec. Let me know if there are any errors there.

thanks for doing all the fixes.
As feature freeze is nearing I have uploaded it to the archive. We can still fix potential reggressions after that.
Unfortunatly I don't know much about pkexec.

This bug was fixed in the package wakeup - 1.2-0ubuntu1

---------------
wakeup (1.2-0ubuntu1) precise; urgency=low

  * New upstream release (LP: #909189).
   - Moved from using gksudo to using pkexec. Added policy file.
   - Changed weather source to google using python-pywapi
   - Added location.py in wakeup directory as plugin helper
   - Added plugin "Commands" which allows arbitrary user dataitems
   - Changed HebrewCalendar to use location from location.py
   - fixed problems to do with hard-coded DISPLAY variable
   - fixed issues with stopping the alarm
   - removed calls to os.system and commands.get(status)output
   - use secure temp files (LP: #912762)
   - root-owned chmod 700 playable_text file for boot alarms
   - small bug fixes
  * Updated packaging
   - replaced gksu with python-dbus in debian/control
   - Removed all perl dependencies
   - wrap-and-sort debian/
   - converted copyright to dep5 format
   - use dh_python2 instead of pysupport
 -- David Glass <email address hidden> Tue, 07 Feb 2012 10:36:30 -0800

Thanks for all the help