Yuvipanda has uploaded a new change for review.
https://gerrit.wikimedia.org/r/322213
Change subject: toollabs: Fix maintain-kubeusers crashing
......................................................................
toollabs: Fix maintain-kubeusers crashing
Looks like if it was interrupted *once* between dir creation and
writing out the token file, it'll continue to fail forever.
Make it a little bit more robust
Change-Id: I98af8e27a328adba8daf4c039e8618456655dbce
---
M modules/toollabs/files/maintain-kubeusers
1 file changed, 19 insertions(+), 4 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/puppet
refs/changes/13/322213/1
diff --git a/modules/toollabs/files/maintain-kubeusers
b/modules/toollabs/files/maintain-kubeusers
index 3b4118c..991a097 100755
--- a/modules/toollabs/files/maintain-kubeusers
+++ b/modules/toollabs/files/maintain-kubeusers
@@ -234,7 +234,9 @@
}
dirpath = os.path.join('/data', 'project', user.name, '.kube')
path = os.path.join(dirpath, 'config')
- os.makedirs(dirpath, mode=0o775, exist_ok=False)
+ # exist_ok=True is fine here, and not a security issue (Famous last
words?).
+ # We also keep it owned by root, which is fine.
+ os.makedirs(dirpath, mode=0o775, exist_ok=True)
f = os.open(path, os.O_CREAT | os.O_WRONLY | os.O_NOFOLLOW)
try:
os.write(f, json.dumps(config, indent=4,
sort_keys=True).encode('utf-8'))
@@ -252,11 +254,24 @@
def create_homedir(user):
"""
Create homedirs for new users
+
"""
homepath = os.path.join('/data', 'project', user.name)
- os.makedirs(homepath, mode=0o775, exist_ok=False)
- os.chmod(homepath, 0o775 | stat.S_ISGID)
- os.chown(homepath, int(user.id), int(user.id))
+ if not os.path.exists(homepath):
+ # Try to not touch it if it already exists
+ # This prevents us from messing with permissions while also
+ # not crashing if homedirs already do exist
+ # This also protects against the race exploit that can be done
+ # by having a symlink from /data/project/$username point as a symlink
+ # to anywhere else. The ordering we have here prevents it - if
+ # it already exists in the race between the 'exists' check and the
makedirs,
+ # we will just fail. Then we switch mode but not ownership, so attacker
+ # can not just delete and create a symlink to wherever. The chown
+ # happens last, so should be ok.
+ self.log.info('Homedir already exists for %s', homepath)
+ os.makedirs(homepath, mode=0o775, exist_ok=False)
+ os.chmod(homepath, 0o775 | stat.S_ISGID)
+ os.chown(homepath, int(user.id), int(user.id))
def create_namespace(user):
--
To view, visit https://gerrit.wikimedia.org/r/322213
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I98af8e27a328adba8daf4c039e8618456655dbce
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Yuvipanda <yuvipa...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
