Saturday, 2020-02-22

*** mattw4 has joined #zuul00:01
clarkbcorvus: I'm not sure I understand the nodepool question00:03
clarkbthe executor isn't running under the same user as the job context is it?00:03
clarkb(I don't think it should)00:03
fungioh, or do you mean it may allow them to run jobs on the executor which listen to arbitrary ports on the pods?00:04
clarkbfungi: I read it as executors may run as the same user as job workloads and therefore the job workloads would be able to modify the executor pods00:05
clarkbI think that scenario should be avoided00:05
clarkbjob workloads are untrusted by zuul00:05
clarkbif we allow ^ we've created an unwanted trust relationship I think00:05
fungii still don't get why adding pods/portforward to the kubernetes rbac would grant access to firewalled ports on executors00:06
corvussorry, i'll try to clarify00:06
fungii'm probably just a bit dense at this time of night after a long week00:07
*** mattw4 has quit IRC00:07
corvuskubectl port-forward sets up a port forward on the executor and a pod running on kubernetes00:07
corvusthe part that happens on the executor is basically just what anyone could do with socat00:07
corvus(but we don't let untrusted playbooks run arbitrary commands, so you wouldn't normally be able to run socat on an executor)00:08
clarkbah ok. Its not arbitrary though right? it would be from finger gateway to $kubeport?00:09
fungioh. i see why i was confused. the comment was more to do with the zuul/zuul change even though it was in the commit message for the zuul/nodepool change00:09
corvusclarkb: i'm not worried about the one that zuul runs00:09
corvusno, it's relevant to the nodepool change,  i'm still not expressing my concern well00:09
corvus(mostly because i don't actually know how to exploit it -- if i did, i would just say :)00:10
corvuslemme do 2 things: explain what the changes do, then a crayon sketch of the vulnerability i'm worried about00:10
fungithat sounds helpful, thanks00:10
clarkbsounds good thanks00:11
corvusthe thing the changes do is: in zuul, start a kubectl port-forward command which listens on the executor on 127.0.0.1:$randomport and connects to the k8s pod port 19885.  it does that for each pod the executor connects to.  that way the log streamer can connect to localhost:$randomport and it's just like with a vm how it connects to vm:19885.00:12
corvusthat kubectl command is run by the zuul executor process itself -- not in a playbook or anything00:12
fungiwhat is port 19885? it's not in my /etc/services00:13
clarkbfungi: thats zuul's log streamer port00:13
corvusbut it does use the k8s credentials that nodepool has given to zuul for the purpose of letting ansible connect to the k8s cluster.  therefore, i also had nodepool configure that credential with the additional access needed to configure port forwarding on the k8s side.00:13
fungiaha, got it. thanks. i'd forgotten ;)00:13
clarkbfungi: it is the port the job contexts publish data over00:13
corvusso that's more or less the change in a nutshell.  make sense?00:14
clarkbcorvus: yup00:14
fungii think so. and you're saying the risk is any job which runs on the executor could then set up a port-forward to anything the executor can reach and expose that as a forwarded stream?00:15
corvusthe hand-wavey thing i'm worried about is someone being able to (in a job playbook) run "kubectl port-forward $important_port mypod/8080" and therefore essentially running something that looks like a service running on the executor on $important_port00:16
fungiahh00:16
corvusfungi: more specifically, the thing running would be on k8s (but hey, it could just be another tcp proxy), but yes00:16
corvusit's easier to do this just by running socat00:17
*** rfolco has quit IRC00:17
fungirunning socat in a container on the kubernetes pod, or in a job on the executor?00:17
corvuson the executor00:17
corvusbut we don't let users run arbitrary commands, so they can't just run socat.  that also means they can't run "kubectl port-forward"00:17
fungiright, okay00:17
clarkbcorvus: it would have to be a trusted job context for both correct?00:17
clarkbI think in that case it is probably ok00:18
corvusso to take advantage of this in an untrusted environment, i think there'd have to be some kind of ansible thing that let you set up a port-forward00:18
fungiso someone would have to actually make it possible for a job to call $dangerous_command on the executor either way00:18
corvusfungi: yeah -- so i guess i'm wondering if there's an existing ansible construct that makes that possible00:18
fungior $dangerous_module i guess00:18
corvusi don't think you can set up a port forward just with k8s yaml00:19
fungiin this case sounds like 1. maybe not a huge deal (though could be used in a social engineering attack of some kind maybe?) but 2. would be good to have our ansible gurus weigh in on the possibility00:20
corvus(i think you could use k8s yaml to set up the part that's needed on the k8s side, but you wouldn't have anything listening locally; "kubectl port-forward" runs socat to do that)00:20
corvusand, at the end of the day, if you did somehow manage to do this, you're just going to be able to listen on a high port anyway, as long as zuul-executor is not running as root (which it should not be)00:21
clarkbcorvus: left a note on https://review.opendev.org/#/c/709261/1 for an unrelated item00:21
corvusso i'm pretty sure this is okay; but it's a "i don't know what i don't know" situation :)00:21
clarkbcorvus: it would probably also be difficult to inject the port in the vars dict early enough that the actual streamer port isn't used00:21
clarkbso you'd be left with a port forward that you can't do much with00:22
clarkbyou could use http module to do requests against it I guess00:22
clarkbthis looks ok to me. but should probably sleep on it to see if any bad cases can be contrived00:23
corvusclarkb: yeah, i don't think anyone can influence the port that the real kubectl port-forward that zuul-executor runs uses.  if you could overwrite ansible variables in the 'zuul' namespace, you could change the port the streamer connects to, but yeah, that's just going to "finger" protocol something00:23
clarkbdoes bwrap create a network namespace the way we are using it?00:25
clarkbif so I think the risk is even lower since we'll have semi isolated the network stack for 127.0.0.100:25
corvusclarkb: no -- i think that's incompatible with allowing network traffic out of the host?00:25
clarkbcorvus: you would need NAT or routable IPs00:26
clarkbNAT being more likely in this case00:26
corvuswe'd have to set that up though, right?00:26
clarkbprobably, docker does it for you somewhat automagically, but bwrap is a bit less magic00:26
clarkband you'd likely have to do it at brwrap startup so ya that gets messy00:27
*** Goneri has quit IRC00:28
clarkbthinking out loud a bit more about this problem. I think VM based jobs are already in this setup if you can figure out a way to run ssh00:33
clarkbthat makes me think being able to port forward the k8s pod is less of a concern00:33
clarkbgranted we don't do system level port forwards00:33
clarkbbut I think that does mean the risk is in impersonating a log stream. And that seems unlikely to be abused since finger is so simple00:34
fungiyeah, i'm not coming up with anything, but should revisit when i'm not braindead on a friday night00:35
mordredcorvus: neat! also not coming up with anything immediately00:36
*** rfolco has joined #zuul01:54
*** jamesmcarthur has joined #zuul02:09
*** jamesmcarthur has quit IRC02:43
*** jamesmcarthur has joined #zuul02:43
*** jamesmcarthur has quit IRC02:48
*** jamesmcarthur has joined #zuul02:57
*** jamesmcarthur has quit IRC03:07
*** jamesmcarthur has joined #zuul03:42
*** igordc has quit IRC03:45
*** rfolco has quit IRC03:52
*** jamesmcarthur has quit IRC04:48
*** evrardjp has quit IRC05:34
*** evrardjp has joined #zuul05:35
*** rishabhhpe has joined #zuul06:03
*** Defolos has joined #zuul07:31
*** rishabhhpe has quit IRC08:00
*** avass has joined #zuul11:24
*** tosky has joined #zuul11:43
*** armstrongs has joined #zuul12:46
armstrongshow do I set the zuul executor logs to debug on the zuul executor? I have tried to upgrade to the latest version and the service isn't starting. Its just giving info logs where it says INFO zuul.Executor: Starting log streamer12:50
armstrongshave seen there is log_config=/etc/zuul/executor-logging.conf in zuul.conf but there isn't anything in the docs that i can see in terms of what that file needs to contain to switch debugging on12:52
avassarmstrongs: I think the only way to do that at the moment is running it in the foreground13:08
avassarmstrongs: There are a couple of changes on the way to decouple that though13:09
avasssee: https://review.opendev.org/#/c/705185/313:10
*** zenkuro has quit IRC13:31
*** Tahvok has quit IRC13:35
fungior using the python-logging module, which requires its own separate config13:41
fungiarmstrongs: https://opendev.org/opendev/puppet-zuul/src/branch/master/files/executor-logging.conf13:43
fungiit's ugly, which is why doing it by running in the foreground is the usual suggestion13:43
avassfungi: ah TIL13:47
armstrongsthank you :)13:49
armstrongsany clues on why the zuul-executor would start then stop?13:58
fungistale pidfile?14:01
fungihow long does it run before it stops?14:01
*** sgw has quit IRC14:04
openstackgerritAlbin Vass proposed zuul/zuul-jobs master: Adds role to install hashicorp packer  https://review.opendev.org/70929214:04
armstrongsit immediately exits has an exception on executor.py14:07
armstrongshttp://paste.openstack.org/show/789892/14:08
armstrongsdebug logs dont show anything more than info do. streamer starts then stops immediately14:09
fungidoes the directory where it wants to create the command socket exist?14:14
fungiand does the user running the executor process have access to it?14:15
armstrongsyeah directory exists and user has access to it ...14:16
armstrongsinitially it was failing as urlib3 wasnt correct version but i upgraded14:17
armstrongsbut that was requests package14:17
armstrongsso it starts clean now then falls over14:17
clarkbis an executer daemon running?14:17
clarkbthat command is a cli command that talks to a running daemon via a unix soclet14:18
armstrongsah ok so then it isnt running at all14:18
armstrongsas it stops immediately the daemon14:18
armstrongswith the starting log streamer then says stopping14:18
armstrongsand daemon stops14:19
armstrongsim gona try and redeploy clean with the bumped requests i may have got this deploy into a mess will report back14:19
armstrongsredeploy fixed :)14:40
*** rfolco has joined #zuul14:54
*** rfolco has quit IRC14:59
mordredarmstrongs: \o/15:00
*** rfolco has joined #zuul15:05
*** armstrongs has quit IRC15:33
*** Goneri has joined #zuul15:55
mordredcorvus: oh good! look - https://zuul.opendev.org/t/zuul/build/0b219711fbb64fcfa85bf5d66848ca65 - sometime in the last 24 hours something has happened to make the py27 test not work16:14
*** zenkuro has joined #zuul16:19
corvusmordred: maybe this? https://pypi.org/project/enum34/#history16:25
corvusmordred: issue https://bitbucket.org/stoneleaf/enum34/issues/27/enum34-118-broken16:26
*** zenkuro has quit IRC16:27
mordredcorvus: yay16:28
mordredcorvus: I'll see if I can find a pinning something to work locally for now16:28
corvusmordred: yeah, i reckon we can pin it to 1.1.6 at the top of our list and probably be ok16:28
corvus(for python < 3.4)16:29
mordred++16:29
mordredcorvus: also - hrm - how does this work at all?16:30
mordredcorvus: we have zuul in the test-requirements.txt16:30
mordredhow are py27 jobs working?16:30
corvusi think the syntax would be: enum34==1.1.6; python_version < "3.4"16:31
mordredyeah - but like - zuul isn't installable on py2716:31
corvusmordred: not sure i understand the q16:31
corvusoh i get it16:31
corvusmordred: i think it's maybe installable but doesn't work?16:32
mordredwell - the fun part is because it's not intended for py27, we don't make wheels, which means we install from tarball which means yarn is needed ... and something something is bitching about node versions on my laptop16:33
mordredcorvus: I'm finding a super fun rabbit hole here16:33
corvusmordred: or you could just throw up a patch and see if it works :)16:33
mordredcorvus: I think I've got a fix ...16:35
*** tosky has quit IRC16:37
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Add pause-buildset-registry role  https://review.opendev.org/70925616:38
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2  https://review.opendev.org/70930216:38
mordredcorvus: turns out we don't need zuul for python216:38
corvusjust for linting?16:38
mordredI believe that's why it's there16:39
mordredwe get things like "zuul_return not found" otherwise16:39
corvusoh but it's used in the tox siblings test?16:39
mordredyeah16:39
corvuswell, since it didn't seem to be "broken" can we avoid "fixing" it? :)16:40
corvusmordred: is that test just picking "zuul" at random?  can we pick another package?16:41
mordredcorvus: sure? it's _totally_ broken locally - I can't run tox -epy27 locally becuase of zuul + complaining about yarn things ... but ultimately I guess running tox -epy27 locally is not somehting people are doing very frequently16:41
mordredcorvus: yeah - let me try that16:41
mordredalso - the unittests leave stuff behind - you can't run them twice without cleaning some stuff up16:41
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2  https://review.opendev.org/70930216:47
mordredcorvus: how's that look?16:47
corvusmordred: seems reasonable; some nit-level comments that probably warrant a respin16:50
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2  https://review.opendev.org/70930216:57
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Add pause-buildset-registry role  https://review.opendev.org/70925616:57
openstackgerritMonty Taylor proposed zuul/zuul-jobs master: Fix cleanup of symlink fixtures  https://review.opendev.org/70930616:57
mordredcorvus: fixed. also - followup patch that fixes the issue with files being left behind after a run16:57
openstackgerritTobias Henkel proposed zuul/zuul master: Add foreground option  https://review.opendev.org/63564917:03
openstackgerritTobias Henkel proposed zuul/zuul master: Deprecate -d switch for running in foreground  https://review.opendev.org/70518517:03
openstackgerritTobias Henkel proposed zuul/zuul master: Don't enforce foreground with -d switch  https://review.opendev.org/70518917:03
*** Goneri has quit IRC17:05
*** zenkuro has joined #zuul17:27
*** evrardjp has quit IRC17:34
*** evrardjp has joined #zuul17:35
*** openstackstatus has joined #zuul17:47
*** ChanServ sets mode: +v openstackstatus17:47
openstackgerritTobias Henkel proposed zuul/zuul master: Recover from broken process pool  https://review.opendev.org/70930717:53
*** jamesmcarthur has joined #zuul18:48
*** jamesmcarthur has quit IRC19:01
*** jamesmcarthur has joined #zuul19:02
*** mugsie has quit IRC19:25
*** mugsie has joined #zuul19:27
*** rfolco has quit IRC20:02
*** rfolco has joined #zuul20:07
*** rfolco has quit IRC20:23
*** jamesmcarthur has quit IRC20:44
*** jamesmcarthur has joined #zuul21:09
*** jamesmcarthur has quit IRC21:19
*** jamesmcarthur has joined #zuul21:54
*** jamesmcarthur has quit IRC22:00
*** jamesmcarthur has joined #zuul22:40
*** jamesmcarthur has quit IRC22:45
*** Defolos has quit IRC23:13
*** jamesmcarthur has joined #zuul23:41
*** jamesmcarthur has quit IRC23:46
*** jamesmcarthur has joined #zuul23:54

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!