*** mattw4 has joined #zuul | 00:01 | |
clarkb | corvus: I'm not sure I understand the nodepool question | 00:03 |
---|---|---|
clarkb | the 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 |
fungi | oh, or do you mean it may allow them to run jobs on the executor which listen to arbitrary ports on the pods? | 00:04 |
clarkb | fungi: 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 pods | 00:05 |
clarkb | I think that scenario should be avoided | 00:05 |
clarkb | job workloads are untrusted by zuul | 00:05 |
clarkb | if we allow ^ we've created an unwanted trust relationship I think | 00:05 |
fungi | i still don't get why adding pods/portforward to the kubernetes rbac would grant access to firewalled ports on executors | 00:06 |
corvus | sorry, i'll try to clarify | 00:06 |
fungi | i'm probably just a bit dense at this time of night after a long week | 00:07 |
*** mattw4 has quit IRC | 00:07 | |
corvus | kubectl port-forward sets up a port forward on the executor and a pod running on kubernetes | 00:07 |
corvus | the part that happens on the executor is basically just what anyone could do with socat | 00: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 |
clarkb | ah ok. Its not arbitrary though right? it would be from finger gateway to $kubeport? | 00:09 |
fungi | oh. 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 change | 00:09 |
corvus | clarkb: i'm not worried about the one that zuul runs | 00:09 |
corvus | no, it's relevant to the nodepool change, i'm still not expressing my concern well | 00:09 |
corvus | (mostly because i don't actually know how to exploit it -- if i did, i would just say :) | 00:10 |
corvus | lemme do 2 things: explain what the changes do, then a crayon sketch of the vulnerability i'm worried about | 00:10 |
fungi | that sounds helpful, thanks | 00:10 |
clarkb | sounds good thanks | 00:11 |
corvus | the 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 |
corvus | that kubectl command is run by the zuul executor process itself -- not in a playbook or anything | 00:12 |
fungi | what is port 19885? it's not in my /etc/services | 00:13 |
clarkb | fungi: thats zuul's log streamer port | 00:13 |
corvus | but 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 |
fungi | aha, got it. thanks. i'd forgotten ;) | 00:13 |
clarkb | fungi: it is the port the job contexts publish data over | 00:13 |
corvus | so that's more or less the change in a nutshell. make sense? | 00:14 |
clarkb | corvus: yup | 00:14 |
fungi | i 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 |
corvus | the 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_port | 00:16 |
fungi | ahh | 00:16 |
corvus | fungi: more specifically, the thing running would be on k8s (but hey, it could just be another tcp proxy), but yes | 00:16 |
corvus | it's easier to do this just by running socat | 00:17 |
*** rfolco has quit IRC | 00:17 | |
fungi | running socat in a container on the kubernetes pod, or in a job on the executor? | 00:17 |
corvus | on the executor | 00:17 |
corvus | but 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 |
fungi | right, okay | 00:17 |
clarkb | corvus: it would have to be a trusted job context for both correct? | 00:17 |
clarkb | I think in that case it is probably ok | 00:18 |
corvus | so 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-forward | 00:18 |
fungi | so someone would have to actually make it possible for a job to call $dangerous_command on the executor either way | 00:18 |
corvus | fungi: yeah -- so i guess i'm wondering if there's an existing ansible construct that makes that possible | 00:18 |
fungi | or $dangerous_module i guess | 00:18 |
corvus | i don't think you can set up a port forward just with k8s yaml | 00:19 |
fungi | in 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 possibility | 00: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 |
corvus | and, 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 |
clarkb | corvus: left a note on https://review.opendev.org/#/c/709261/1 for an unrelated item | 00:21 |
corvus | so i'm pretty sure this is okay; but it's a "i don't know what i don't know" situation :) | 00:21 |
clarkb | corvus: it would probably also be difficult to inject the port in the vars dict early enough that the actual streamer port isn't used | 00:21 |
clarkb | so you'd be left with a port forward that you can't do much with | 00:22 |
clarkb | you could use http module to do requests against it I guess | 00:22 |
clarkb | this looks ok to me. but should probably sleep on it to see if any bad cases can be contrived | 00:23 |
corvus | clarkb: 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 something | 00:23 |
clarkb | does bwrap create a network namespace the way we are using it? | 00:25 |
clarkb | if so I think the risk is even lower since we'll have semi isolated the network stack for 127.0.0.1 | 00:25 |
corvus | clarkb: no -- i think that's incompatible with allowing network traffic out of the host? | 00:25 |
clarkb | corvus: you would need NAT or routable IPs | 00:26 |
clarkb | NAT being more likely in this case | 00:26 |
corvus | we'd have to set that up though, right? | 00:26 |
clarkb | probably, docker does it for you somewhat automagically, but bwrap is a bit less magic | 00:26 |
clarkb | and you'd likely have to do it at brwrap startup so ya that gets messy | 00:27 |
*** Goneri has quit IRC | 00:28 | |
clarkb | thinking 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 ssh | 00:33 |
clarkb | that makes me think being able to port forward the k8s pod is less of a concern | 00:33 |
clarkb | granted we don't do system level port forwards | 00:33 |
clarkb | but I think that does mean the risk is in impersonating a log stream. And that seems unlikely to be abused since finger is so simple | 00:34 |
fungi | yeah, i'm not coming up with anything, but should revisit when i'm not braindead on a friday night | 00:35 |
mordred | corvus: neat! also not coming up with anything immediately | 00:36 |
*** rfolco has joined #zuul | 01:54 | |
*** jamesmcarthur has joined #zuul | 02:09 | |
*** jamesmcarthur has quit IRC | 02:43 | |
*** jamesmcarthur has joined #zuul | 02:43 | |
*** jamesmcarthur has quit IRC | 02:48 | |
*** jamesmcarthur has joined #zuul | 02:57 | |
*** jamesmcarthur has quit IRC | 03:07 | |
*** jamesmcarthur has joined #zuul | 03:42 | |
*** igordc has quit IRC | 03:45 | |
*** rfolco has quit IRC | 03:52 | |
*** jamesmcarthur has quit IRC | 04:48 | |
*** evrardjp has quit IRC | 05:34 | |
*** evrardjp has joined #zuul | 05:35 | |
*** rishabhhpe has joined #zuul | 06:03 | |
*** Defolos has joined #zuul | 07:31 | |
*** rishabhhpe has quit IRC | 08:00 | |
*** avass has joined #zuul | 11:24 | |
*** tosky has joined #zuul | 11:43 | |
*** armstrongs has joined #zuul | 12:46 | |
armstrongs | how 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 streamer | 12:50 |
armstrongs | have 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 on | 12:52 |
avass | armstrongs: I think the only way to do that at the moment is running it in the foreground | 13:08 |
avass | armstrongs: There are a couple of changes on the way to decouple that though | 13:09 |
avass | see: https://review.opendev.org/#/c/705185/3 | 13:10 |
*** zenkuro has quit IRC | 13:31 | |
*** Tahvok has quit IRC | 13:35 | |
fungi | or using the python-logging module, which requires its own separate config | 13:41 |
fungi | armstrongs: https://opendev.org/opendev/puppet-zuul/src/branch/master/files/executor-logging.conf | 13:43 |
fungi | it's ugly, which is why doing it by running in the foreground is the usual suggestion | 13:43 |
avass | fungi: ah TIL | 13:47 |
armstrongs | thank you :) | 13:49 |
armstrongs | any clues on why the zuul-executor would start then stop? | 13:58 |
fungi | stale pidfile? | 14:01 |
fungi | how long does it run before it stops? | 14:01 |
*** sgw has quit IRC | 14:04 | |
openstackgerrit | Albin Vass proposed zuul/zuul-jobs master: Adds role to install hashicorp packer https://review.opendev.org/709292 | 14:04 |
armstrongs | it immediately exits has an exception on executor.py | 14:07 |
armstrongs | http://paste.openstack.org/show/789892/ | 14:08 |
armstrongs | debug logs dont show anything more than info do. streamer starts then stops immediately | 14:09 |
fungi | does the directory where it wants to create the command socket exist? | 14:14 |
fungi | and does the user running the executor process have access to it? | 14:15 |
armstrongs | yeah directory exists and user has access to it ... | 14:16 |
armstrongs | initially it was failing as urlib3 wasnt correct version but i upgraded | 14:17 |
armstrongs | but that was requests package | 14:17 |
armstrongs | so it starts clean now then falls over | 14:17 |
clarkb | is an executer daemon running? | 14:17 |
clarkb | that command is a cli command that talks to a running daemon via a unix soclet | 14:18 |
armstrongs | ah ok so then it isnt running at all | 14:18 |
armstrongs | as it stops immediately the daemon | 14:18 |
armstrongs | with the starting log streamer then says stopping | 14:18 |
armstrongs | and daemon stops | 14:19 |
armstrongs | im gona try and redeploy clean with the bumped requests i may have got this deploy into a mess will report back | 14:19 |
armstrongs | redeploy fixed :) | 14:40 |
*** rfolco has joined #zuul | 14:54 | |
*** rfolco has quit IRC | 14:59 | |
mordred | armstrongs: \o/ | 15:00 |
*** rfolco has joined #zuul | 15:05 | |
*** armstrongs has quit IRC | 15:33 | |
*** Goneri has joined #zuul | 15:55 | |
mordred | corvus: 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 work | 16:14 |
*** zenkuro has joined #zuul | 16:19 | |
corvus | mordred: maybe this? https://pypi.org/project/enum34/#history | 16:25 |
corvus | mordred: issue https://bitbucket.org/stoneleaf/enum34/issues/27/enum34-118-broken | 16:26 |
*** zenkuro has quit IRC | 16:27 | |
mordred | corvus: yay | 16:28 |
mordred | corvus: I'll see if I can find a pinning something to work locally for now | 16:28 |
corvus | mordred: yeah, i reckon we can pin it to 1.1.6 at the top of our list and probably be ok | 16:28 |
corvus | (for python < 3.4) | 16:29 |
mordred | ++ | 16:29 |
mordred | corvus: also - hrm - how does this work at all? | 16:30 |
mordred | corvus: we have zuul in the test-requirements.txt | 16:30 |
mordred | how are py27 jobs working? | 16:30 |
corvus | i think the syntax would be: enum34==1.1.6; python_version < "3.4" | 16:31 |
mordred | yeah - but like - zuul isn't installable on py27 | 16:31 |
corvus | mordred: not sure i understand the q | 16:31 |
corvus | oh i get it | 16:31 |
corvus | mordred: i think it's maybe installable but doesn't work? | 16:32 |
mordred | well - 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 laptop | 16:33 |
mordred | corvus: I'm finding a super fun rabbit hole here | 16:33 |
corvus | mordred: or you could just throw up a patch and see if it works :) | 16:33 |
mordred | corvus: I think I've got a fix ... | 16:35 |
*** tosky has quit IRC | 16:37 | |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Add pause-buildset-registry role https://review.opendev.org/709256 | 16:38 |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2 https://review.opendev.org/709302 | 16:38 |
mordred | corvus: turns out we don't need zuul for python2 | 16:38 |
corvus | just for linting? | 16:38 |
mordred | I believe that's why it's there | 16:39 |
mordred | we get things like "zuul_return not found" otherwise | 16:39 |
corvus | oh but it's used in the tox siblings test? | 16:39 |
mordred | yeah | 16:39 |
corvus | well, since it didn't seem to be "broken" can we avoid "fixing" it? :) | 16:40 |
corvus | mordred: is that test just picking "zuul" at random? can we pick another package? | 16:41 |
mordred | corvus: 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 frequently | 16:41 |
mordred | corvus: yeah - let me try that | 16:41 |
mordred | also - the unittests leave stuff behind - you can't run them twice without cleaning some stuff up | 16:41 |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2 https://review.opendev.org/709302 | 16:47 |
mordred | corvus: how's that look? | 16:47 |
corvus | mordred: seems reasonable; some nit-level comments that probably warrant a respin | 16:50 |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Fix unittests for python2 https://review.opendev.org/709302 | 16:57 |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Add pause-buildset-registry role https://review.opendev.org/709256 | 16:57 |
openstackgerrit | Monty Taylor proposed zuul/zuul-jobs master: Fix cleanup of symlink fixtures https://review.opendev.org/709306 | 16:57 |
mordred | corvus: fixed. also - followup patch that fixes the issue with files being left behind after a run | 16:57 |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Add foreground option https://review.opendev.org/635649 | 17:03 |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Deprecate -d switch for running in foreground https://review.opendev.org/705185 | 17:03 |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Don't enforce foreground with -d switch https://review.opendev.org/705189 | 17:03 |
*** Goneri has quit IRC | 17:05 | |
*** zenkuro has joined #zuul | 17:27 | |
*** evrardjp has quit IRC | 17:34 | |
*** evrardjp has joined #zuul | 17:35 | |
*** openstackstatus has joined #zuul | 17:47 | |
*** ChanServ sets mode: +v openstackstatus | 17:47 | |
openstackgerrit | Tobias Henkel proposed zuul/zuul master: Recover from broken process pool https://review.opendev.org/709307 | 17:53 |
*** jamesmcarthur has joined #zuul | 18:48 | |
*** jamesmcarthur has quit IRC | 19:01 | |
*** jamesmcarthur has joined #zuul | 19:02 | |
*** mugsie has quit IRC | 19:25 | |
*** mugsie has joined #zuul | 19:27 | |
*** rfolco has quit IRC | 20:02 | |
*** rfolco has joined #zuul | 20:07 | |
*** rfolco has quit IRC | 20:23 | |
*** jamesmcarthur has quit IRC | 20:44 | |
*** jamesmcarthur has joined #zuul | 21:09 | |
*** jamesmcarthur has quit IRC | 21:19 | |
*** jamesmcarthur has joined #zuul | 21:54 | |
*** jamesmcarthur has quit IRC | 22:00 | |
*** jamesmcarthur has joined #zuul | 22:40 | |
*** jamesmcarthur has quit IRC | 22:45 | |
*** Defolos has quit IRC | 23:13 | |
*** jamesmcarthur has joined #zuul | 23:41 | |
*** jamesmcarthur has quit IRC | 23:46 | |
*** jamesmcarthur has joined #zuul | 23:54 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!