throwaway96
commented
4 months ago The script will now exit if the fallback directory can't be created (e.g., if it already exists), which should at least resolve one symlink issue.
I also replaced the chmod calls with just setting umask 077 at the beginning of the script. I think the only user-visible effect is $SESSION_TOKEN_CACHE no longer being world-readable. If that's a problem, I can save and restore the original umask.
I don't know if the warning at the top of the script is sufficient, but I'm not sure where else it should be mentioned.
I can squash these commits if desired.
Description
There is currently a race condition in
renew-script.shthat could lead to exposure of the SSH private key. This change should resolve the issue by usingmktempto create a temporary directory with an unpredictable name and mode 0700.The temporary directory (and therefore private key) will also be removed when no longer needed.
If
mktempdoes not exist or otherwise fails, a fallback directory will be created with a name based on the PID of the script. The mode will still be set to 0700 before the private key file is created. Since the name is predictable, there is a potential issue with symlink-based attacks. Perhaps the fallback should be removed or improved...Note that the symlink issue is also present with
$SESSION_TOKEN_CACHE. I'm not sure how to resolve that, since the name needs to be predictable. Checking that it's a regular file would not solve the problem but would at least make such an attack more difficult. However, I could also imagine someone wanting it to be a symlink (e.g., if/tmpis volatile and they want to maintain the cache across reboots). Anyway, this should probably be documented (with a warning to never run the script as root).Fixes #193
Type of change
Checklist:
It would be nice to have someone who uses this script test these changes.