15:30:39 <fao89> #startmeeting Pulp Triage 2021-01-19
15:30:39 <fao89> #info fao89 has joined triage
15:30:39 <fao89> !start
15:30:39 <pulpbot> Meeting started Tue Jan 19 15:30:39 2021 UTC.  The chair is fao89. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:30:39 <pulpbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
15:30:39 <pulpbot> The meeting name has been set to 'pulp_triage_2021-01-19'
15:30:39 <pulpbot> fao89: fao89 has joined triage
15:31:42 <daviddavis> #info daviddavis has joined triage
15:31:42 <daviddavis> !here
15:31:42 <pulpbot> daviddavis: daviddavis has joined triage
15:32:30 <ggainey> #info ggainey has joined triage
15:32:30 <ggainey> !here
15:32:30 <pulpbot> ggainey: ggainey has joined triage
15:32:36 <ppicka> !here
15:32:36 <ppicka> #info ppicka has joined triage
15:32:37 <pulpbot> ppicka: ppicka has joined triage
15:33:04 <ttereshc> #info ttereshc has joined triage
15:33:04 <ttereshc> !here
15:33:04 <pulpbot> ttereshc: ttereshc has joined triage
15:33:08 * daviddavis shouts across the post apocalyptic wastelands, "QUORUM!"
15:33:30 <ttereshc> :)
15:33:34 <ggainey> heh
15:33:47 <fao89> topic: How specific can we be in Access Denied type of errors not to give away too much information but also to make it less painful for users and devs to debug permission problems?
15:33:55 <fao89> https://paste.centos.org/view/df93d584
15:34:08 <ttereshc> it was me
15:34:20 <bmbouter> #info bmbouter has joined triage
15:34:20 <bmbouter> !here
15:34:20 <pulpbot> bmbouter: bmbouter has joined triage
15:34:25 <ttereshc> I was very frustrated with debugging and I wonder how it would be for users
15:34:39 <ttereshc> to figure out what they configured wrongly
15:35:28 <ggainey> ttereshc: detailed info into the log is prob good. From the "outside", anything other than "you do not have permissions" is prob to be avoided
15:35:36 <bmbouter> the issue is that the log isn't for the user
15:35:42 <ggainey> because you don't want to give info to an attacker
15:36:02 <ggainey> bmbouter: but a 'user' can't change perms - an admin has to do that
15:36:08 <ttereshc> what attacker can do if I say you don't have access to a requested repository?
15:36:21 <bmbouter> users can over time certainly
15:36:27 <ttereshc> without saying its name
15:36:47 <bmbouter> I think the application should raise a fatal exception that has a helpful error message
15:36:51 <ttereshc> at least I know which perms got screwed up
15:37:12 <ggainey> ah ok, that sounds reasonable
15:37:53 <ttereshc> ok, maybe I can file an issue for that
15:38:07 <ttereshc> bmbouter, could you define what helpful message means
15:38:13 <ttereshc> how helpful :)
15:39:02 <dkliban> #info dkliban has joined triage
15:39:02 <dkliban> !here
15:39:02 <pulpbot> dkliban: dkliban has joined triage
15:39:43 <bmbouter> well that's the trick of it
15:40:20 <bmbouter> right now in pulp's history I think helping users figure problems out > concerns for data leakage
15:40:43 <bmbouter> over time as the setup gets easier that concern will reverse
15:40:49 <ggainey> strongly disagree - because leaking-info never changes, until *after* it causes a security problem
15:40:52 <gerrod> #info gerrod has joined triage
15:40:52 <gerrod> !here
15:40:52 <pulpbot> gerrod: gerrod has joined triage
15:41:00 <ggainey> and then it's too late
15:41:05 <bmbouter> to be clear though it's not a security problem
15:41:09 <bmbouter> it's a data leakage problem
15:41:22 <ggainey> I don't see a difference there
15:41:25 <bmbouter> which is a problem but it's not with that info the user can sidestep authorization
15:41:47 <ggainey> "learning things you're not supposed to know" *is* a security problem
15:41:47 <bmbouter> ggainey: I get where you're coming from it's my default also
15:42:19 <bmbouter> but when asked my opinion on where to draw the line on that error message this is my answer (maybe not the right answer)
15:42:46 <ggainey> sure - which is why we're havin the discussion, is to decide where the line gets drawn in pulp :)
15:44:10 <ttereshc> we can have a setting which allows to produce error messages with some details and user can consciously turn it on knowing that some info might be revealed. Not passwords but more details than the person focused on security would consider safe.
15:44:37 <bmbouter> that would resolve our difference in opinion but I worry that's too complicated
15:44:44 <dkliban> sounds complicateed
15:44:45 <bmbouter> I'd rather agree with grant and let the user just figure it out
15:44:54 <ttereshc> I don't really like this idea  exactly for that reason
15:45:01 <ttereshc> yeah
15:45:18 <ggainey> yeah, feels like asking for edge-cases, alas
15:45:21 <ttereshc> let ggainey do all the rbac and maybe he'll change his mind :P
15:45:35 <ggainey> nope - ggainey did security for a living for too many years
15:45:39 <bmbouter> I maintain that knowing the name of a repo is not allowing a user to be authorized to operate any more than they could otherwise
15:45:53 <bmbouter> and it is practically very helpful to the user
15:46:01 <ggainey> the REST call replies "You're not allowed to do that", and the server-logs have details as to why
15:46:27 <bmbouter> we're going in a circle I think we established already that the logs are not helpful to users
15:46:29 <ggainey> we have absolutely had actual-customer-cases where "leaking repo names" was a sev-1 for the customer
15:46:51 <dkliban> i was of ggainey's opinion yesterday, but after discussing ttereshc's experience yesterday, i have decided that revealing the name is fine.
15:47:23 <bmbouter> ggainey's opinion is also my default
15:47:32 <ttereshc> is it ok to have this info in server logs at least? it can be the first step
15:47:52 <ggainey> ttereshc: yes - because server-logs don't show anything that someone with server-access couldn't find anyway
15:48:31 <ggainey> as long as we keep 'secrets' (keys and pwds and such) out of logs, we can put whatever detrail we want there
15:48:40 <ggainey> detail, even
15:49:04 <ttereshc> ok, so i'll file an issue to have all details in server logs and the type of resource for a user message. Is it ok with everyone?
15:49:13 <ggainey> sounds like a fine idea to me
15:49:17 <daviddavis> +1
15:49:18 <bmbouter> I don't think it's actually helping
15:49:31 <bmbouter> sorry because I know we want to move on but the user does not have log access
15:49:50 <ggainey> then they shouldn't have detailed-info-access on poermission-failures either
15:50:03 <bmbouter> I'm ok with that
15:50:25 <ttereshc> bmbouter, I think that user performs operation a limited set of resources and if you point which resource is the problematic one, it's already helpful
15:50:27 <ggainey> I'm not. We can do something different, but I'm not going to agree it's correct or appropriate.
15:50:45 <ttereshc> operation on a limited set of resources
15:51:12 <ttereshc> e.g. I'm doing podman push, and it involves creating 3 resources
15:51:28 <ttereshc> if I'm told which one, it's easier for me to figure out what's up
15:51:32 <bmbouter> to me the choices are a) say more in the response the uesr can read or b) do not, there isn't a c) put things into the logs
15:51:44 <fao89> next topic?
15:52:18 <ttereshc> bmbouter,  the say more part for a user is the resource type
15:52:24 <ttereshc> imo
15:52:27 <dkliban> ggainey: in ttereshc's specific use case she already knows the names of things
15:52:31 <ttereshc> not ideal but a step forward
15:52:39 <bmbouter> ttereshc: oh I see what you're saying, I'm good with that
15:52:52 <dkliban> she just wanted to know which of the resources she is lacking permissions to access
15:52:55 <bmbouter> I'm basically ok with all of these options as long as the info is going back to the user not in the logs
15:53:33 <ttereshc> they are probably unrelated. Logs will help with development/debugging.
15:53:41 <ttereshc> just not to do one instead of the other
15:54:04 <ipanova> ttereshc: dkliban FYI dockerhub does not say much, it just retuns 'insufficient permissions'
15:54:39 <ttereshc> ok, I think we can move on and I'll file an issue where we can figure out more details.
15:54:41 <ttereshc> ty ipanova
15:54:57 <ttereshc> thanks for the discussion
15:54:59 <ggainey> sure - better to have discussion in an issue, so it's available for the future!
15:56:21 <dkliban> i think we need to move on
15:56:27 <ggainey> concur
15:56:38 <fao89> +1
15:57:05 <fao89> topic: Spam script removing actual users
15:57:11 <ggainey> ouch
15:57:24 <daviddavis> this came up during our pulpcore meeting
15:57:24 <dkliban> this is problematic
15:57:35 <ggainey> yupyup
15:57:39 <fao89> disable the script?
15:57:51 <dkliban> i am wondering what kind of links they are posting
15:58:00 <dkliban> that is causing them to be flagged
15:58:11 <dkliban> dalley: do you know?
15:58:16 <dalley> it's not a matter of flagged, it's new users not going through the entire process
15:58:34 <dkliban> what do you mean?
15:58:35 <dalley> also there's manual effort on our end to verify them and add them to the whitelist
15:59:00 <dkliban> dalley: the script is supposed to look for issues/comments that contain links
15:59:10 <dkliban> and we have a white list of domains where users are allowed to link to
15:59:32 <ttereshc> so the script does its job right then, it seems that the new users do not get into this whitelist
16:00:11 <ttereshc> if I understood dalley correctly
16:00:16 <dalley> the one example I specifically remember was one of AdamW's posts that ended up deleted permanently, so I don't know where the link was to
16:00:31 <dkliban> does every new user need to be added to the whitelist?
16:00:41 <dalley> I just remember he emailed us after his account got locked and I had to unlock it
16:01:04 <dalley> and AdamW is one of our most active and helpful users
16:02:05 <dalley> but the post is gone
16:03:01 <dalley> dkliban, actually, to be honest, we've gone through a few revisions of the script, I've lost track because I was never really involved with it apart from being on the mailing list
16:03:10 <dalley> I don't know what it does right at this moment
16:03:20 <ggainey> yeah same
16:03:34 <bmbouter> same here
16:04:14 <dkliban> my understanding is that when a user posts a link to a domain we have not whitelisted, their account gets flagged and they must send us an email.
16:04:29 <dkliban> once that email is received the user needs to be added to a whitelist
16:04:42 <daviddavis> I know at least one user that didn't bother to do that and their content got removed
16:04:43 <dkliban> ideally, that domain would also get added to a whitelist
16:04:45 <daviddavis> fwiw
16:05:13 <dkliban> I am nto saying that tehre is no problem, but i just want to make sure we are all on the same page
16:05:25 <daviddavis> yes, it's appreciated
16:05:35 <ggainey> daviddavis: do we know the script did that? like, can we point to the script-log that says "I'm deleting Post X"?
16:05:41 <dkliban> and i agree that we don't want legitimate users getting deleted
16:06:08 <dkliban> ggainey: i am not sure how much is logged
16:06:17 <ggainey> have we seen a legit *user* get deleted? I thought we just locked users
16:06:27 <daviddavis> this was like months ago when I believe we were still deleting accounts before locking them
16:06:29 <daviddavis> now we lock them
16:06:29 <ggainey> dkliban: yeah same, it's been forever since I looked at a run
16:06:32 <dkliban> no, i misspoke
16:06:36 <ggainey> ah kk
16:07:24 <daviddavis> do we still get a lot of spam?
16:07:37 <ggainey> can we get an issue open, to teach the spam-filter to make issues 'private' rather than deleting them? would that accomplish our spam-goals, while still letting us recover in the case of an error?
16:08:07 <ggainey> daviddavis: well, we've had ppl complaining about it recently - not sure how much we get, but it does still happen
16:08:13 <daviddavis> +1
16:08:59 <daviddavis> I agree with the issue and making stuff private instead of deleting it. I guess I wonder if we should also disable the spam script until that issue gets implemented?
16:09:27 <fao89> we still have 2 items for open floor + triage, and we only have 20 minutes more
16:09:51 <daviddavis> let's think bout this topic and discuss more on friday?
16:09:54 <dkliban> ggainey: daviddavis: i'll open an issue to make things private instead of delete
16:09:59 <daviddavis> +1
16:10:03 <ggainey> +1
16:10:23 <fao89> +1
16:10:32 <bmbouter> oh wait lol
16:10:40 <bmbouter> I want to disable all private comments on redmine lol
16:10:49 <bmbouter> so this goes against that other goal
16:11:20 <bmbouter> I can explain why etc here if you want or just assume that's a valuable goal
16:12:08 <ggainey> I'm never fond of private bugs - but if deleting a legitimate user's issue is happening, but we still want to run the antispam-script, we are stuck
16:12:44 <bmbouter> when we accept private data we have several new problems: 1) we can never make it unprivate, 2) the "private read access" is a huge unmaintained mess 3) github issues won't support private issues when we switch 4) folks in the past expressed concerns that pulp is accepting private data and it legally shouldn't
16:13:20 <bmbouter> oh also it violates our agreement with plan.io as a fully public open source project
16:13:40 <ggainey> ok - then we don't do that, and take the (hopefully rare) hit
16:14:08 <ggainey> which I'm ok with, to be clear - it's suboptimal, but so is letting pulp.plan.io be used by spammers
16:15:00 <dalley> what about security bugs?
16:15:25 <bmbouter> we never file security bugs in pulp.plan.io
16:15:38 <dalley> off topic anyways. +1 to the proposal
16:15:42 <bmbouter> they are disclosed over mailing list and then kept in bugzilla only
16:15:49 <bmbouter> while embargoed
16:16:31 <daviddavis> timecheck, we have less than 15 min left with 2 topics and a handful of issues.
16:17:25 <bmbouter> what was the conclusion on this?
16:17:44 <ggainey> I honestly don't know :)
16:19:09 <fao89> conclusion: bring it on next open floor?
16:19:46 <dalley> ggainey, we did see at least one legit user get deleted, but yes thats no longer an issue
16:20:13 <dalley> but even getting locked would be a major pain / frustration
16:20:28 <ggainey> fao89: for the sake of moving thru today's list, sure
16:20:30 <fao89> I moved the topics for next open floor, I think we need to start triage
16:20:38 <daviddavis> oh I have a blocking issue
16:20:43 <daviddavis> er topic
16:20:49 <dkliban> what is it?
16:21:01 <daviddavis> Should model validation be skipped altogether?
16:21:01 <ggainey> dalley: spam-filtering is hard - we're going to frustrate somebody somewhere :)
16:21:05 <fao89> topic: Should model validation be skipped altogether?
16:21:06 <daviddavis> e.g. https://git.io/JttPW
16:21:17 <ggainey> daviddavis: what's the problem there?
16:21:46 <daviddavis> we said all validation belongs in the serializer but I'm wondering if model validation is ok in addition to serializer validation
16:21:50 <daviddavis> belt + suspenders :)
16:22:01 <bmbouter> lol
16:22:26 <ggainey> daviddavis: I mean, in that very model we're throwing valueerrror in get_downloaders()
16:22:28 <daviddavis> that is a ggainey-ism
16:22:34 <ggainey> no it isn't
16:22:47 <daviddavis> oh I got belt + suspenders from you
16:22:59 <ggainey> oooh, the expression, I get it
16:23:27 <daviddavis> so you're saying that model validation is ok in addition to other places in the code?
16:23:30 <bmbouter> moving it out of the model would be ok w/ me
16:23:32 <ggainey> this model has all sorts of validation.
16:23:48 <bmbouter> it's tough because we don't always use the serializers...
16:23:55 <daviddavis> I'm working on some validation in the serializer and I'd like to add it to the model too to be sure
16:23:57 <daviddavis> yea
16:24:10 <bmbouter> like for example migration code
16:24:14 <ggainey> I mean, I get it, it's aspirational - but it's also *everywhere* in our models
16:24:37 <bmbouter> serializers also can derive their validation from models in some cases right?
16:24:50 <bmbouter> or am I just thinking wishfully
16:24:53 <ggainey> daviddavis: if we want to say "one should strive to...", I'm absolutely onboard - not so much for "rewrite existing code Just Because", tho
16:25:09 <daviddavis> this is new code
16:25:17 <daviddavis> I just posted an examle of some existing code
16:25:29 <daviddavis> sorry if I caused some confusion there
16:25:34 <bmbouter> for new code if you can use serializer only I'm ok w/ that
16:25:36 <ggainey> ah, kk, thanks
16:25:38 <ggainey> yeah same
16:25:52 <daviddavis> I mean I'd like to do serializer + model to be sure
16:26:01 <daviddavis> since it's possible the model is used via the plugin api
16:26:06 <bmbouter> as long as it's DRY I'm ok w/ that
16:26:10 <daviddavis> ok
16:26:12 <bmbouter> if it's not DRY then I think we're taking on risk
16:26:21 <daviddavis> yea, ok
16:26:23 <ggainey> yeah, concur
16:26:40 <daviddavis> thanks fao89
16:26:52 <ttereshc> a minor problem is that if we change anything in validation, we'd need to do it in 2 places
16:27:17 <daviddavis> right, I think that was bmbouter's point about being DRY
16:27:30 <ttereshc> ah ok
16:27:34 <ggainey> yus
16:28:33 <fao89> can I start triage?
16:28:46 <ggainey> heh - for 2 min? :)
16:29:23 <fao89> 2 issues there seems to be important
16:29:23 <daviddavis> this is the only semi-urgent issue I see: https://pulp.plan.io/issues/8116
16:29:45 <fao89> !issue 8116
16:29:45 <fao89> #topic https://pulp.plan.io/issues/8116
16:29:46 <pulpbot> RM 8116 - paji@redhat.com - NEW - Export is generating the incorrect 'next_version' causing import to fail.
16:29:47 <pulpbot> https://pulp.plan.io/issues/8116
16:29:51 <ggainey> daviddavis: yeah - worked w/paji on this yesterday
16:29:59 <ggainey> accept and add, we need to figure something out here
16:30:18 <ttereshc> what's the target release for katello?
16:30:47 <ttereshc> do we need it late January or February?
16:30:56 <fao89> #idea Proposed for #8116: accept and add to sprint
16:30:56 <fao89> !propose other accept and add to sprint
16:30:56 <pulpbot> fao89: Proposed for #8116: accept and add to sprint
16:31:27 <fao89> #agreed accept and add to sprint
16:31:27 <fao89> !accept
16:31:27 <pulpbot> fao89: Current proposal accepted: accept and add to sprint
16:31:28 <fao89> #topic https://pulp.plan.io/issues/8115
16:31:28 <pulpbot> fao89: 4 issues left to triage: 8115, 8112, 7950, 7922
16:31:29 <pulpbot> RM 8115 - bmbouter - NEW - Satellite issue sync is broken
16:31:30 <pulpbot> https://pulp.plan.io/issues/8115
16:32:14 <ttereshc> I'm not a fan adding items to the sprint without knowing relative priority, since we are already stretched thin. that's why I asked.
16:33:22 <ggainey> ttereshc: this breaks a common katello use-case for import/export. It needs to be fixed for imp/exp to be useful to tyhe katello user. I'd think 4.0? but we will ask in integration-mtg tomorrow
16:33:31 <ggainey> maybe 4.1?
16:34:45 <fao89> I'll remove it from sprint, and just skip the triage
16:34:48 <fao89> #endmeeting
16:34:48 <fao89> !end