14:30:52 #startmeeting Pulp Triage 2020-05-05 14:30:52 !start 14:30:52 #info fao89 has joined triage 14:30:52 Meeting started Tue May 5 14:30:52 2020 UTC. The chair is fao89. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:30:52 Useful Commands: #action #agreed #help #info #idea #link #topic. 14:30:52 The meeting name has been set to 'pulp_triage_2020-05-05' 14:30:52 fao89: fao89 has joined triage 14:30:58 #info dkliban has joined triage 14:30:58 !here 14:30:59 dkliban: dkliban has joined triage 14:31:06 !here 14:31:07 #info ggainey has joined triage 14:31:08 ggainey: ggainey has joined triage 14:31:34 !next 14:31:35 fao89: 10 issues left to triage: 6645, 6644, 6643, 6642, 6638, 6631, 6628, 6623, 6597, 6463 14:31:35 !here 14:31:36 #topic https://pulp.plan.io/issues/6645 14:31:36 #info bmbouter has joined triage 14:31:36 RM 6645 - mdepaulo@redhat.com - ASSIGNED - pre-flight check not work properly when it needs a prereq role 14:31:37 #info ipanova has joined triage 14:31:37 !here 14:31:38 https://pulp.plan.io/issues/6645 14:31:39 bmbouter: bmbouter has joined triage 14:31:40 ipanova: ipanova has joined triage 14:31:42 #info ttereshc has joined triage 14:31:42 !here 14:31:42 ttereshc: ttereshc has joined triage 14:31:48 !propose accept and add to sprint 14:31:48 dkliban: propose accept Propose accepting the current issue in its current state. 14:31:51 #idea Proposed for #6645: accept and add to sprint 14:31:51 !propose other accept and add to sprint 14:31:51 fao89: Proposed for #6645: accept and add to sprint 14:31:56 !°here 14:31:56 x9c4: Error: "°here" is not a valid command. 14:32:00 #info x9c4 has joined triage 14:32:00 !here 14:32:00 x9c4: x9c4 has joined triage 14:32:14 +1 14:32:39 mikedep333, are you already working on it, right? 14:32:50 should these be triaged by the installer team? 14:32:57 #info daviddavis has joined triage 14:32:57 !here 14:32:57 daviddavis: daviddavis has joined triage 14:33:04 #info mikedep333 has joined triage 14:33:04 !here 14:33:04 mikedep333: mikedep333 has joined triage 14:33:05 yes 14:33:13 I can't vote for adding or not adding it to the sprint 14:33:28 it's a critical bug that's being worked on 14:33:39 I'm wondering if we need a project for the installer 14:33:52 #agreed accept and add to sprint 14:33:52 !accept 14:33:52 fao89: Current proposal accepted: accept and add to sprint 14:33:53 fao89: 9 issues left to triage: 6644, 6643, 6642, 6638, 6631, 6628, 6623, 6597, 6463 14:33:53 #topic https://pulp.plan.io/issues/6644 14:33:54 RM 6644 - mdepaulo@redhat.com - ASSIGNED - pre-flight check does not check plugins that are installed but not listed in pulp_install_plugins 14:33:54 let's discsuss that after triage 14:33:55 https://pulp.plan.io/issues/6644 14:33:57 Please add it. I am working on 6623's 4 sub-issues, #6642 -#6645 14:34:02 !propose accept and add to sprint 14:34:02 dkliban: propose accept Propose accepting the current issue in its current state. 14:34:23 #idea Proposed for #6644: accept and add to sprint 14:34:23 !propose other accept and add to sprint 14:34:23 fao89: Proposed for #6644: accept and add to sprint 14:34:25 #agreed accept and add to sprint 14:34:25 !accept 14:34:25 fao89: Current proposal accepted: accept and add to sprint 14:34:25 fao89: We discussed that at Pulpcon I think. Let's discuss later; I'll dig up any notes. 14:34:26 #topic https://pulp.plan.io/issues/6643 14:34:26 fao89: 8 issues left to triage: 6643, 6642, 6638, 6631, 6628, 6623, 6597, 6463 14:34:28 RM 6643 - mdepaulo@redhat.com - ASSIGNED - pre-flight check does not understand the plugin "upgrade" boolean 14:34:29 https://pulp.plan.io/issues/6643 14:34:32 !propose accept and add to sprint 14:34:32 dkliban: propose accept Propose accepting the current issue in its current state. 14:34:56 !accept 14:34:56 fao89: No action proposed, nothing to accept. 14:35:03 #idea Proposed for #6643: accept and add to sprint 14:35:03 !propose other accept and add to sprint 14:35:03 dkliban: Proposed for #6643: accept and add to sprint 14:35:07 i was using the wrong format 14:35:09 #agreed accept and add to sprint 14:35:09 !accept 14:35:09 fao89: Current proposal accepted: accept and add to sprint 14:35:10 #topic https://pulp.plan.io/issues/6642 14:35:10 fao89: 7 issues left to triage: 6642, 6638, 6631, 6628, 6623, 6597, 6463 14:35:11 RM 6642 - mdepaulo@redhat.com - ASSIGNED - pre-flight check does not check older pulp 3 installs properly 14:35:12 https://pulp.plan.io/issues/6642 14:35:15 #idea Proposed for #6642: accept and add to sprint 14:35:15 !propose other accept and add to sprint 14:35:15 dkliban: Proposed for #6642: accept and add to sprint 14:35:21 #agreed accept and add to sprint 14:35:21 !accept 14:35:21 fao89: Current proposal accepted: accept and add to sprint 14:35:22 #topic https://pulp.plan.io/issues/6638 14:35:22 fao89: 6 issues left to triage: 6638, 6631, 6628, 6623, 6597, 6463 14:35:23 RM 6638 - bmbouter - NEW - [Epic] Host fixtures docker container at https://fixtures.pulpproject.org 14:35:24 https://pulp.plan.io/issues/6638 14:35:31 convert to task 14:35:33 or story 14:35:37 is an epic an issue? or stary? 14:35:40 story even 14:35:41 task 14:35:51 +1 to task 14:35:55 yeah sounds good 14:35:56 #idea Proposed for #6638: convert to task 14:35:56 !propose other convert to task 14:35:56 dkliban: Proposed for #6638: convert to task 14:35:57 +1 14:36:20 #agreed convert to task 14:36:20 !accept 14:36:20 fao89: Current proposal accepted: convert to task 14:36:21 fao89: 5 issues left to triage: 6631, 6628, 6623, 6597, 6463 14:36:21 #topic https://pulp.plan.io/issues/6631 14:36:22 RM 6631 - phlogistonjohn - NEW - pulp_installer: failure to install pulp-rpm on minimal fedora vm 14:36:23 https://pulp.plan.io/issues/6631 14:36:58 bmbouter, I do not have permission to convert to a task on infrastructure 14:37:16 this is not a bug 14:37:25 I'll convert that to task 14:37:30 user is not using rpm_prerequisite_role 14:37:38 maybe skip for now and see what user reports? 14:37:39 I don't know what to propose 14:37:44 or notabug is fine 14:37:46 skip sounds good 14:37:47 dkliban: exactly. 14:38:09 #idea Proposed for #6631: close as not a bug 14:38:09 !propose other close as not a bug 14:38:09 dkliban: Proposed for #6631: close as not a bug 14:38:26 #agreed close as not a bug 14:38:26 !accept 14:38:26 fao89: Current proposal accepted: close as not a bug 14:38:27 #topic https://pulp.plan.io/issues/6628 14:38:27 fao89: 4 issues left to triage: 6628, 6623, 6597, 6463 14:38:28 RM 6628 - bmclaugh - NEW - nginx config broken in Pulp in a Container 14:38:29 https://pulp.plan.io/issues/6628 14:38:52 #idea Proposed for #6628: Leave the issue as-is, accepting its current state. 14:38:52 !propose accept 14:38:52 fao89: Proposed for #6628: Leave the issue as-is, accepting its current state. 14:39:05 and add to sprint 14:39:17 #idea Proposed for #6628: accept and add to sprint 14:39:17 !propose other accept and add to sprint 14:39:17 fao89: Proposed for #6628: accept and add to sprint 14:39:20 the single container doesn't include nginx config snippets from plugins 14:39:30 and it should 14:39:32 ah 14:39:32 #agreed accept and add to sprint 14:39:32 !accept 14:39:32 fao89: Current proposal accepted: accept and add to sprint 14:39:33 #topic https://pulp.plan.io/issues/6623 14:39:33 fao89: 3 issues left to triage: 6623, 6597, 6463 14:39:34 RM 6623 - bmbouter - NEW - pulpcore and plugin pre-flight check seems to not be enforcing and then installer fails at collectstatic 14:39:35 https://pulp.plan.io/issues/6623 14:39:57 Accept please, this is the parent issue for 6642 - 6645. 14:40:04 #idea Proposed for #6623: accept and add to sprint 14:40:04 !propose other accept and add to sprint 14:40:04 fao89: Proposed for #6623: accept and add to sprint 14:40:06 Assume a parent issue is the correct way to do this. 14:40:07 +1 14:40:09 #agreed accept and add to sprint 14:40:09 !accept 14:40:09 fao89: Current proposal accepted: accept and add to sprint 14:40:10 fao89: 2 issues left to triage: 6597, 6463 14:40:10 #topic https://pulp.plan.io/issues/6597 14:40:11 RM 6597 - dkliban@redhat.com - NEW - docs fail to build for Pulp 2 14:40:12 https://pulp.plan.io/issues/6597 14:40:13 (can issues be parents?) 14:40:27 mikedep333: yes 14:40:33 Thanks 14:40:43 we need to accept this and add to sprint also so that we can release 2.21.2 14:40:52 #idea Proposed for #6597: accept and add to sprint 14:40:52 !propose other accept and add to sprint 14:40:52 fao89: Proposed for #6597: accept and add to sprint 14:40:52 yeah +1 14:40:54 yeah 14:40:54 ok 14:41:10 #agreed accept and add to sprint 14:41:10 !accept 14:41:10 fao89: Current proposal accepted: accept and add to sprint 14:41:11 fao89: 1 issues left to triage: 6463 14:41:12 #topic https://pulp.plan.io/issues/6463 14:41:12 RM 6463 - binlinf0 - NEW - pulp 3.2.1 duplicate key error when sync 14:41:13 https://pulp.plan.io/issues/6463 14:41:36 #idea Proposed for #6463: accept and add to sprint 14:41:36 !propose other accept and add to sprint 14:41:36 dkliban: Proposed for #6463: accept and add to sprint 14:41:40 we now have a reproducer 14:41:42 +1 14:41:44 +1 14:41:45 +1 14:41:53 +1 14:41:56 #agreed accept and add to sprint 14:41:56 !accept 14:41:57 fao89: Current proposal accepted: accept and add to sprint 14:41:58 fao89: No issues to triage. 14:42:07 open floor! 14:42:21 Q for this installer work 14:42:29 I want to talk about secretcharfield 14:42:38 can we skip the installer bugs during triage and let the installer team handle them? 14:42:50 It sounds like if a user specifies a plugin name like pulp_file rather than pulp-file, pip will convert it to the dash. 14:42:51 daviddavis: we need to create a separate project in redmine 14:42:56 +1 14:43:10 +1 14:43:40 I'll follow up on pulp-dev list 14:43:43 But i am not sure about pip-tools. I was thinking of replacing underscores in plugin names with dashes for whenever pip-tools get called. 14:43:53 I did a query for installer, so it will be easier to migrate to a new project https://pulp.plan.io/projects/pulp/issues?query_id=154 14:44:00 fao89: thanks! 14:44:23 mikedep333: i am not sure ... can we discuss after the secret char field? 14:44:23 I agree with this, it will make my work a lot easier. 14:44:29 dkliban: sure 14:44:30 thats great 14:44:47 (the separate project will make things easier) 14:44:57 * mikedep333 yields 14:45:36 bmbouter: secret char field/ 14:46:15 so I've heard from katello testers that they are having bindings issues because username/password are not shown in the openapi schema for remotes 14:46:45 yep 14:47:25 the solution we have been persuing is to use secretCharFields 14:47:33 this is causing a major problem where you can't use pulp_container to sync from a registry that requires authentication 14:47:43 bmbouter: that's right 14:48:53 this field would allow users to submit a value for username and password, but only show a checksum of the value when they retrieve a remote 14:49:28 yes that is what secret char field would do 14:49:28 What if it didn't show anything but "***"? 14:49:58 there is a use case where the user wants to be able to verify that the saved data is correct 14:50:02 x9c4: then the user would have no way of verifying that the configuration is what they expect it to be. debuggging auth problems would be hard. 14:50:27 The foreman api does it that way. 14:50:31 and also having anything but the actual data in that fields' response creates another problem I call the "read-then-write-whoops" problem 14:50:40 foreman api probably has problem verifying the config then 14:51:18 that problem is where you read the data in the bindings, e.g. '***' and then accidentally write that data back 14:51:27 hence the whoops at the end of my description 14:52:17 yep 14:52:47 What if the field provided the checksum in a different field? 14:53:11 like 'password_checksum' 14:53:28 well it complicates things in terms of the read/write serializers (complicates in the sense of additional complexity) 14:53:34 it's not an off table idea tho 14:54:04 sure, the user needs to know how to handle it, but he does anyway to prevent you whoops 14:54:20 that's true it would prevent the whoops. so I've been asking myself why are we hiding these fields at all? 14:54:46 and the reason is because on a multi-user system one user could read the password from another 14:54:57 bmbouter: we're hiding them, iirc, because without authentication/rbac, once I set a remote, *anyone* can retrieve it, yeah? 14:55:02 yeah that 14:55:03 yeah exactly 14:55:05 :) 14:55:19 so here's the thing, pulp as is isn't safe for multi-user in a whole bunch of ways 14:55:29 currently ... until rbac 14:55:58 but rbac is one of our next major things, and planning is underway, so I think assuming we'll have it in 2 months is a reasonable expectation to me 14:56:29 so as a thought experiement, let's assume rbac is already in place ... I believe in that world we don't have to hide the fields at all 14:58:29 bmbouter: so with rbac in place, would some users not be shown these fields? 14:58:41 i mean, if I take a big security-step back, security-conscious users aren't going to want us to be storing things like this in pulp's db at all, but getting them from some keystore-solution 14:58:58 I guess users would not be able to see remotes of other users. 14:59:01 but anyway - with rbac, I'd imagine that only someone with access-to a remote would be able to see the pwds 14:59:04 yeah 14:59:31 admin can see everything, anyone w/db-access clearly can, otherwise you'd have to have allowed-read-access 14:59:36 yes users having read access to remotes is one of the primary use cases 15:00:15 You would need to trust your pulp-admin in addition to root and the db admin. 15:00:41 bmbouter: i suspect it would be useful for some users to see the URL of a remote but not the username and password 15:00:47 what do you thi? 15:01:17 the choice on who to trust currently includes the users, admin, db-admin, and root 15:01:31 my proposal is only focused on removing users from that list 15:01:54 yeah, admin is either trusted or...you're in trouble 15:01:59 agreed 15:02:35 dkliban: the rbac system would be flexible enough to have more fine grainey permissions, but the issue is openapi schema is not. so I put that in the hard-but-possible category. I'd want to have user nominated use case before thinking much more about it (is my take) 15:02:58 as in, we'd have to start spliitting serializers around that field ... 15:03:09 yeah 15:03:42 bmbouter: i was thinking that the queryset would filter out that field based on permissions. but perhaps i am dreaming 15:04:02 anyway, back to the bigger picture 15:04:03 so, I understand the problems it can cause, but I am having problems shaking being Really Uncomfortable with treating auth-info as just-another-field 15:04:33 do we have personas defined anywhere? (I thought we had but I can't find it) 15:04:43 Do we need to dumb down the serializers? Whoever can write may probably read all fields, right? 15:04:50 consider the damage someone could do with other non-auth fields, oh I secretly rewrote the url field to sync from untrusted falsified content 15:05:01 ggainey: my claim with ^ example is that rbac is our only hope 15:05:10 i agree 15:05:38 ttereshc: personas will be plugin defined, these are the "roles" that users and groups can be assigned 15:05:39 bmbouter: RE changing-url - the issue here is that the auth-info may be used for *other access* to that remote site 15:06:02 I'm not as worried about "I can break your pulp install" as "your pulp install gave me access to someone else's machine(s)" 15:06:09 ttereshc: we do not have a concrete list of personas I'm hoping thursday's call will help us get that 15:06:31 ggainey: I'm more worried about compromising pulp actually, if you compromise pulp you p0wn the datacenter 15:07:14 bmbouter: if I compromise your login, I can comprise a totally-different datacenter - that's the thinking there 15:07:42 and I can do that with just read-access to the remote, even after RBAC exists 15:07:49 x9c4: the beauty of this approach is that the read and write would not need to be different for remotes for example and would reduce the write_only issues to only serializers that accept data that isn't stored for retrieval later 15:08:08 ggainey: I'm not quiet following, rbac only hands data to those authorized to have it 15:09:37 bmbouter: if my RBAC setup gives someone read-access to remotes, without write-access - so they can't break "your" pulp-installation - but they now have access to auth-info that gives them access elsewhere 15:10:22 ggainey: but that's not pulp's problem ... it sounds like the permissions are being handed out to the wrong people 15:10:30 (I mean, at some point users are going to want to be able to insure that this kind of auth-info exists only in some protected keystore and doesn't live in pulp's db at all, but tha's prob way down the road) 15:10:52 I agree and I agree, I do hope that becomes an option down the road 15:11:27 yeah, I have a similar concern as ggainey , that's why I asked about personas, I want to understand use cases for different ones. I believe some users will need to have a read-access without seeing auth info 15:12:00 yea, I could see someone with access to a remote set it up in pulp with username+pass for someone else in pulp to sync from 15:12:14 but I don't want to share user+pass with this pulp user 15:12:43 the way this use case is being described is making sense to me 15:12:51 despite my rejection from dkliban's description earlier 15:13:06 heh 15:13:16 the general-security-rule is to protect auth-info more than Other Stuff, mostly because even thye people you trust can't be trusted :) - the rabbit-hole does eventually have a bottom, but this isn't it 15:13:18 hehehe 15:13:20 rbac could have a "use a remote" permission, but it wouldn't allow you to see some fields and not others 15:13:28 and rbac is well suited for this 15:13:38 yeah, field-level RBAC is...rather a PITA 15:13:46 but, could be done I guess 15:14:08 we need to keep in mind the practical issues of having openapi schema's try to have definitions of objects with conditional fields ... it's very hard and confusing 15:14:16 bmbouter: RE "despite rejection" - well, this is what we strive for, right? being able to change each other's minds? :) 15:14:23 yeah 15:14:31 not me 15:14:32 jk 15:14:41 bmbouter: i don't think we would want the schema to have conditional fields. i think we would simply want some fields returned as null. 15:14:44 :) 15:14:55 dkliban: well that's the workaround that has problems with it 15:15:03 so, lemme back up - How much work is it to make these secret fields? how often will that really bite users? how har dis it to puty warnings in the related docs? 15:15:25 making the fields is easy, but the usability impact is not 15:15:28 good lord, and why can't I type today? 15:15:29 ggainey: we have the secret fields already for the client_key and client_cert 15:15:47 ggainey: I thought that was your southern accent 15:15:47 and those would also be handled in this same way, rbac 15:16:04 that was my understanding also bmbmouter 15:16:13 :P 15:16:19 bmbouter: so what problems do you see with the workaround? 15:16:29 the issue wiht secretcharfields is a) they are not usable they require more work from the user b) they are insecure also because we don't use salts c) rbac is a better solution anyway 15:17:10 so I am unclear on c), I don't think RBAC will help unless/until we get to field-level RBAC. 15:17:32 but not using salts makes this not-security, and I'd rather have plaintext than pretend-security 15:17:46 dkliban: the read-then-write-whoops problem still exists if we rewrite the data to null on READ 15:18:15 ggainey: having two personas I think would resolve it a) rbac role to read a remote (all fields or none) b) rbac role to use but not read 15:18:39 ah kk 15:18:52 and that use aligns well with openapischema also 15:19:24 Is read-than-write a thing for users without full access to the object? 15:19:24 RE the oops issue - security is def inconvenient - the number of times security-related-bugs get closed with "yes. it's painful if you do that. stop doing it. NOTABUG" is...large. :) 15:20:12 x9c4: i would expect users that can't see those fields, to not be able to modify that resource 15:20:17 bmbouter: ^ 15:20:18 defining object partial access is hard and has usability issues overall 15:20:24 yup 15:20:31 def concur 15:20:48 and we don't have a solid use case defining it as the only solution 15:20:52 Yes, and not beeing able to write circumvents any read-than-write-oops 15:21:17 yes but then we end up with split serializers, that's what I mean when I say hard 15:21:30 time check ... we have 9 minutes 15:21:37 ah right 15:21:37 rbac or not we could split the remote serializers and have some users use some and others others but this is going to get complicated fast 15:21:46 I mean if you cannot see the password, but change the URL, you can leak the password. 15:22:01 x9c4: i agree 15:22:09 wow I never thought of that 15:22:21 So being able to write should imply full read access. 15:22:21 hand the password to meeeeeee 15:22:25 yep 15:22:39 ok, so I'm just going to put this out there - I think having these as secretfields is worth the useability issues, both from a 'real' and a 'perceived' security stance - *BUT*, it depends on whether secretfields are actually keeping their content secret 15:22:42 And then there is no need to split the serializer. 15:23:08 so my proposal is to have one serializer and not split it, do away with secret char fields, and let rbac do the rest 15:23:13 salting them should be fairly easy. 15:23:27 it's easy for us, not easy for users their verification step becomes harder 15:23:43 and how do they provide the salt? 15:23:54 how do they know what salt to use when verifying 15:24:08 it seems complicated 15:24:09 we select it server side so theyw ould have to read salt and then compute hash(salt + orig_password) == hashed_password 15:24:31 so it's ok to give out the salt "? 15:24:35 it's more work and it's going to be error prone because of the additonal steps which if you mess any of it up as a user lead you to an incorrect 15:24:43 you provide salt:hash to the user. 15:24:44 it is safe to give out the salt 15:24:50 gotcha 15:24:56 yeah 15:25:14 an attacker would know what to recompute the password with, but they would have to recompute it for that psecific salt 15:25:29 so they can't use a pre-generated well known table, they have to make a custom table 15:25:48 And a different one for each password. 15:25:54 gotcha 15:26:09 yupyup 15:26:28 it sounds like we are not ready to make a decision 15:26:49 and our time is almost up for today 15:27:11 what's our next step? 15:27:21 we need to fix this write_only fields bug soon 15:27:23 more discussion is all I can see 15:27:23 very soon 15:27:49 yeah, that's my concern - we're broken *now*, and rbac isn't going to help for a while 15:28:15 pulp is already not safe for multi-user as is I don't think exposing them is going to make it worse 15:28:26 agreed 15:29:34 I thought pulp is not multi-user. There is only admin. 15:29:48 And users come bac with rbac 15:29:49 well, I'm going to go on record as a -0 for 'leave them plaintext' - I don't like it, but not at the stop-the-presses level 15:30:21 I copy that ggainey 15:31:13 mostly because we have, as noted, way bigger fish to fry on the security front :) 15:31:14 the thing is we would need a counterproposal and all of the ways we've contemplated resolving it come with even more issues 15:31:38 I'd make them secret, and eat the useability issues as doc 15:31:42 ok I gotta run, this has been a great discussion ty for all the thought and energy 15:31:48 yessir! 15:31:59 thank you! 15:32:36 we can end open floor 15:33:03 #endmeeting 15:33:03 !end