14:30:58 #startmeeting Pulp Triage 2020-05-08 14:30:58 #info fao89 has joined triage 14:30:58 !start 14:30:58 Meeting started Fri May 8 14:30:58 2020 UTC. The chair is fao89. Information about MeetBot at http://wiki.debian.org/MeetBot. 14:30:58 Useful Commands: #action #agreed #help #info #idea #link #topic. 14:30:58 The meeting name has been set to 'pulp_triage_2020-05-08' 14:30:58 fao89: fao89 has joined triage 14:31:04 #info bmbouter has joined triage 14:31:04 !here 14:31:05 bmbouter: bmbouter has joined triage 14:31:08 #info daviddavis has joined triage 14:31:08 !here 14:31:08 daviddavis: daviddavis has joined triage 14:31:11 !next 14:31:12 fao89: 5 issues left to triage: 6687, 6670, 6669, 6667, 6658 14:31:12 #topic https://pulp.plan.io/issues/6687 14:31:13 RM 6687 - daviddavis - NEW - Return Docs link to main navigation 14:31:14 https://pulp.plan.io/issues/6687 14:31:30 #info dkliban has joined triage 14:31:30 !here 14:31:30 dkliban: dkliban has joined triage 14:31:34 I opened this for some feedback 14:32:00 what you're saying makes sense 14:32:00 can someone add me on infrastructure project? it will help me triage issues 14:32:13 fao89: sure 14:32:15 #info x9c4 has joined triage 14:32:15 !here 14:32:15 x9c4: x9c4 has joined triage 14:32:30 as an aside we track website issues here https://github.com/pulp/pulpproject.org/issues 14:32:39 oh right 14:32:41 #info ggainey has joined triage 14:32:41 !here 14:32:41 ggainey: ggainey has joined triage 14:32:42 which likely is not where we should track them 14:32:58 I'll move it 14:33:04 daviddavis: would you want to email pulp-dev w/ this quesiton and/or link? 14:33:12 sure, will do 14:33:14 mcorr: fyi ^ 14:33:44 fao89: added 14:33:57 so let's skip 14:33:59 thank you! 14:34:05 !skip 14:34:06 #topic https://pulp.plan.io/issues/6670 14:34:06 fao89: 4 issues left to triage: 6670, 6669, 6667, 6658 14:34:07 RM 6670 - fao89 - NEW - pulp-operator is not exhibiting logs when getting 500 14:34:08 https://pulp.plan.io/issues/6670 14:34:25 it was a problem with galaxy CI 14:34:36 this is valid but we're not fixing pulp-operator issues atm, so I propose accept 14:34:49 #idea Proposed for #6670: Leave the issue as-is, accepting its current state. 14:34:49 !propose accept 14:34:49 fao89: Proposed for #6670: Leave the issue as-is, accepting its current state. 14:34:52 +1 14:35:12 #agreed Leave the issue as-is, accepting its current state. 14:35:12 !accept 14:35:12 fao89: Current proposal accepted: Leave the issue as-is, accepting its current state. 14:35:13 #topic https://pulp.plan.io/issues/6669 14:35:14 fao89: 3 issues left to triage: 6669, 6667, 6658 14:35:15 RM 6669 - mdepaulo@redhat.com - NEW - pulp_installer does not support plugins having both the version and the upgrade vars specified 14:35:16 https://pulp.plan.io/issues/6669 14:35:40 mikedep333: should we accept or accept and add to sprint? 14:36:00 ~here 14:36:08 Add to sprint please. 14:36:11 #idea Proposed for #6669: accept and add to sprint 14:36:11 !propose other accept and add to sprint 14:36:11 fao89: Proposed for #6669: accept and add to sprint 14:36:15 +1 14:36:17 +1 14:36:18 #info mikedep333 has joined triage 14:36:18 !here 14:36:18 mikedep333: mikedep333 has joined triage 14:36:19 #agreed accept and add to sprint 14:36:19 !accept 14:36:19 fao89: Current proposal accepted: accept and add to sprint 14:36:20 fao89: 2 issues left to triage: 6667, 6658 14:36:20 #topic https://pulp.plan.io/issues/6667 14:36:21 RM 6667 - mdepaulo@redhat.com - NEW - Create larger pulp_installer docs 14:36:22 https://pulp.plan.io/issues/6667 14:36:38 convert to task? 14:36:44 +1 14:36:49 yes 14:36:53 convert to task 14:36:55 #idea Proposed for #6667: convert to a task 14:36:55 !propose other convert to a task 14:36:55 fao89: Proposed for #6667: convert to a task 14:37:00 #agreed convert to a task 14:37:00 !accept 14:37:00 fao89: Current proposal accepted: convert to a task 14:37:01 fao89: 1 issues left to triage: 6658 14:37:01 #topic https://pulp.plan.io/issues/6658 14:37:02 RM 6658 - xenlo - NEW - Pain points when trying Pulp3 for the first time 14:37:03 https://pulp.plan.io/issues/6658 14:37:32 let's accept ... we are going to improve the installer docs based on this feedback 14:37:38 but it's not meant to go directly on the sprint 14:37:40 Agreed 14:37:42 #idea Proposed for #6658: Leave the issue as-is, accepting its current state. 14:37:42 !propose accept 14:37:42 fao89: Proposed for #6658: Leave the issue as-is, accepting its current state. 14:37:50 #agreed Leave the issue as-is, accepting its current state. 14:37:50 !accept 14:37:50 fao89: Current proposal accepted: Leave the issue as-is, accepting its current state. 14:37:51 fao89: No issues to triage. 14:38:10 open floor! 14:38:48 we need to finish the discussion about what we want to do about the secret char fields. 14:38:55 or at least continue it 14:38:55 we do 14:39:37 somewhat related with it, I don't know what to do with my write_only fields PR, should I close it? 14:39:50 fao89: this is all part of that discussion 14:40:00 fao89: please don't close it yet 14:40:08 my biggest concern right now is that the client bindings are unable to use the username and password fields on the Remote 14:40:28 I think, we concluded, that whoever can write to a resource can read all of it. 14:40:30 which makes pulp_container plugin unable to sync from registries that require authentication ... which is almost all of them 14:40:58 x9c4: that's right 14:41:04 so let's go back to that 14:43:10 if we start returning the values of these 'secret' fields, what kind of warning should we provide our users during the time without RBAC? 14:43:33 with bindings docs issue, and this write_only fields issue, it is leading me to conclude bindings should have github repos 14:44:01 dkliban: I think we should highlight in the release notes that at this time pulp is only safe as a single-user system 14:44:31 "Do not let anyone stand behind you while operating pulp!" 14:44:36 which is already the case but it's so important we can say it again single user only! 14:44:37 lol 14:45:39 the main thing I've realized is that any secretCharField w euse can be easily compromised by someone updating the url 14:45:39 hehehe 14:45:50 i agree 14:46:01 and the solution to limiting who can update that is rbac 14:46:13 Does the api allow to specify field i do not want to see in the request? 14:46:28 x9c4: yes, you can filter out fields 14:46:40 but the apispec does not 14:46:49 openapi v2 specifically AIUI 14:47:49 that's right 14:48:36 bmbouter: but that shouldn't really be a problem. a user that can update the resource should be able to read all of it anyway. 14:49:33 dkliban: I agree but to answer x9c4's question 14:49:57 actually your answering the question asked, I was not 14:50:40 So there is a way to prevent evesdropping of already saved credentials if you do not use the tools provided. 14:51:04 so the big question still remains, are we comfortable with returning the values for username and password fields (and similar ones) and telling the users that pulp is only to be used by a single user for now? 14:51:20 But in the other direction (to set them) we transfer then unecrypted anyway. 14:51:52 x9c4: hopefully they will be set over https 14:52:28 yes pulp can be configured with https (I've been doing this lately for certguard) 14:53:01 I have an installer branch that configures self-signed certs for nginx and httpd which work 14:53:18 Sure. But i think that means, we are not really adding security by hiding them on the way out. (that should be ssl too.) 14:54:16 x9c4: I agree https is needed I think the installer team probably needs to discuss that goal soon 14:54:30 regardless of what we hide on the response if the request is insecure it's not safe 14:55:50 true 14:56:07 but that's a separate issue because users could do that today 14:56:59 agreed 14:59:19 So SecretCharFields look like a security improvement, but really arent. If we can agree on that, let's remove them. And sure keep telling users, to thoroughly restrict access to the API. 15:00:36 that is the conclsuion I've reached 15:00:54 yep 15:01:15 i am convinced that SecretCharField is feaux security 15:01:50 dalley++ 15:01:50 x9c4: dalley's karma is now 245 15:02:52 daviddavis, ggainey, dalley: wdyt about this ^? 15:04:53 bmbouter: that works for me 15:04:58 +1 from me 15:05:01 (sorry, had to take a sec to catch up) 15:05:46 np, I don't want to move on w/o you all 15:06:35 so in terms of implementing this change, did secretCharField ever get merged or used anywhere? 15:07:48 bmbouter: yes, it's used for client_cert and client_key 15:09:08 ok so one issue to specifically remove the use there? 15:09:15 that's right 15:09:24 and the other is updating the PR from fao89 15:09:31 hey question 15:09:33 yes 15:09:36 would that break semver? 15:09:54 not if we are saying that this is a bug 15:10:04 yea I guess we could say we're fixing a bug 15:10:19 +1 from me 15:10:45 https://pulp.plan.io/issues/6691 15:11:05 dkliban: what is the bug related to not seeing those username and password fields? 15:11:25 I propose we put that on the spring and have it fixed with plaintext responses also along with 6691 15:11:29 sprint 15:11:45 bmbouter: https://pulp.plan.io/issues/6346 .. on the sprint already 15:11:47 bmbouter: your task is a bit ambiguous. are we removing the field or just its usage? 15:11:58 both! 15:12:06 +1 from me 15:13:23 and +1 to putting it on the sprint 15:14:24 I added it to the sprint 15:15:06 so back to fao89 's PR, I think two new PRs should be made one for 6691 and one for 6346, and the write_only field only deal wil the remaining write_only usage outside of those things 15:16:05 bmbouter: ok ... so close the current PR? 15:16:23 I thought there were a few other places write_only was used outside of these two? 15:16:47 for cases where the data submitted by the request wasn't actually saved so it couldn't be returned, e.g. the file at upload time 15:16:51 yes, in the Content serializer 15:17:07 yup 15:17:17 so more like thin the current open PR down 15:18:37 I had another issue to look at write_only fields ot see if they could be converted to non write only https://pulp.plan.io/issues/6421 15:18:48 since many of them are on serializers used only for POST requests 15:19:47 yep 15:20:01 i am going to put a comment on the open PR 15:20:24 to say that this change should simply be limited to making username and password fields not write_only 15:20:42 and that's it ... everything else we will handle with sparate PRs 15:20:48 +1 15:21:44 +1 15:22:00 and the 3.4 relase note needs a big banner on it 15:22:49 https://github.com/pulp/pulpcore/pull/600#pullrequestreview-408316663 15:22:50 something like "Don't use write_only fields. We don't support them" ? 15:22:50 Actually we could put it here above the "latest" changelog at all times and do that ahead of cutting the changelog 15:23:52 https://raw.githubusercontent.com/pulp/pulpcore/master/CHANGES.rst above the line ".. towncrier release notes start" 15:23:56 let me file that now 15:24:15 any suggestions on where else this shoul go are welcome 15:24:43 on bumper sticker! 15:25:08 i am just kidding ... it's friday! 15:25:18 !friday 15:25:18 ♪ It's Friday, Friday, gotta get down on Friday ♪ 15:25:29 sigh - and here I am in the middle of adding write-only fields to export :( 15:25:31 blargh 15:25:38 i'll figure out how to do it differently 15:26:11 https://pulp.plan.io/issues/6692 15:26:46 +1 15:27:06 ggainey: if it's a legit write_only use (as in not for secrecy and also not able to be returned on the subsequent GET) then maybe continue? fao89's PR will need to handle that more broadly by splitting the serializers 15:27:23 what about grooming 6692 for the sprint? 15:27:27 bmbouter: yeah, I am prob going to leave it as-is for now 15:27:29 and what other places should thisbe documented? 15:31:15 i am not sure 15:32:07 I can find a few more places to suggest 15:32:15 and then maybe we can add to sprint later? 15:32:22 I'll post back here about it 15:32:22 sounds good 15:32:25 we should end open floor 15:32:36 #endmeeting 15:32:36 !end