18:02:50 #startmeeting keystone 18:02:51 Meeting started Tue Mar 4 18:02:50 2014 UTC and is due to finish in 60 minutes. The chair is dolphm. Information about MeetBot at http://wiki.debian.org/MeetBot. 18:02:52 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 18:02:55 The meeting name has been set to 'keystone' 18:02:57 boris-42: success! 18:03:04 ayoung, bknudson, dstanek, jamielennox, morganfainberg, stevemar, gyee, henrynash, topol, marekd, lbragstad, joesavak, shardy, fabiog, fmarco76: ping 18:03:06 dolphm ou nice=) 18:03:11 w00t 18:03:13 dolphm's got the magic command :) 18:03:31 #topic Reminder: Icehouse feature freeze today! (features must be merged) 18:03:53 at this point we've only got one open bp: https://review.openstack.org/#/c/55908/ 18:03:53 dolphm, i thought it was the 6th :O 18:04:16 and one high priority bug fix: https://review.openstack.org/#/c/75727/ 18:04:30 stevemar: i3 is cut today 18:04:34 stevemar: it's released on the 6th 18:04:39 dolphm, ah okay 18:04:53 stevemar: pretty sure our meeting agenda has said march 4th for a long while :) 18:04:58 i gave topol the wrong info yesterday :) 18:04:59 dolphm so after today can there still be bug fixes or no? 18:05:07 topol: absolutely 18:05:19 topol: bug fixes only for the next few weeks 18:05:22 dolphm, ok cool 18:05:27 bug fixing is encouraged 18:05:44 i've already started laying out our blockers to shipping icehouse: 18:05:45 #link https://launchpad.net/keystone/+milestone/icehouse-rc1 18:05:47 looks like march 27 is start of release candidates 18:06:11 once we cut an RC1, master will be re-opened for Juno features 18:06:42 55908 now has the record for the most revisions on a single review. 80 as of now 18:06:48 81* 18:06:55 and we'll have a milestone-proposed branch to backport further bug fixes to (RC2 bugs, if any) 18:06:55 HA! 18:07:59 stevemar, the thing that scares me is it still hasn't gotten the bknudson treatment. 18:08:15 ayoung, i always fear the bknudson treatment 18:08:49 it would take me so long to review the change I wouldn't get done until next week 18:09:03 bknudson, hehe 18:09:07 the only feedback I could give on it now is that it's too complicated for me to understand it 18:09:13 Well, it got it back on revision 23 18:09:16 merge 55908 and call it experimental feature :) 18:09:45 if we have to do it EOB today that is 18:10:04 dstanek, did I get your changes? 18:10:32 gyee: considering it's a security feature, that's not really acceptable 18:10:32 ayoung: i think so - was going over it before this meeting 18:10:56 we have enough silly vulnerabilities on our track record as it stands 18:10:59 dolphm, but there's a kill switch 18:11:00 dstanek, yeah...just did the 80-81 diff 18:11:04 I mean its an extension 18:11:08 its disabled by default 18:11:18 gyee, but security extension == scary 18:11:32 is there a dire need for it? 18:11:36 morganfainberg, security is a process, software is a tool 18:11:40 it means it can be QA'ed before it is tested 18:11:45 ayoung: speaking of, you should definitely add SecurityImpact to the commit message 18:11:47 I think it needs to be in, experimental 18:11:58 dolphm, is there one if it in not enabled? 18:12:07 ayoung: ? 18:12:20 gyee, if it opens us up to security bugs, it doesn't matter if could cause a lot of headaches. 18:12:22 I mean, I'll do it, but there should be no impact until we tell people : go ahead an use it 18:12:29 gyee, not saying we shouldn't merge it. 18:12:48 gyee, just making sure we're aware what we're getting into 18:13:07 lets merge it with an eye to getting tempest support up to speed, the client using it, and a general shake out 18:13:15 morganfainberg, sure, call it experimental would cover our asses 18:13:31 I don't think experimental will mean we don't have to provide security fixes for it. 18:13:44 I think we need to start doing that for new, big features. Its part of the reason for doing it as an extension 18:13:46 ayoung: let's focus on *reviewing* it for now, not *merging* it 18:13:49 i.e., we will have to fix any security problems found whether it's experimental or not 18:13:50 ++ 18:13:55 bknudson: ++ 18:13:56 bknudson, ++ 18:13:57 bknudson, that's not what I mean, we'll have to fix whatever that is needed 18:14:18 dolphm, in general, though, we need an "experimental" track. 18:14:44 especially when changes need to be made both in server and client, and then tempest tests on top 18:14:50 maybe if we put in a big warning to not use this if you care about security?? 18:14:53 ayoung: i've actually prototyped an @experimental decorator to complement the @deprecated one 18:14:53 ephemeral will be in the same boat 18:14:57 ++ 18:14:59 since there's lots of things you could do if you don't care about security 18:15:07 ayoung, actually, i hope ephemeral will be J1 18:15:16 morganfainberg, experimental in J1.... 18:15:24 ayoung, so more drive time to get things in. last minute on merge day is hard to get in experimental or not 18:15:35 and GA in Juna GA 18:15:57 morganfainberg, you are right, I should have started this earlier. Like back in Essex 18:15:59 ayoung, dolphm, i like the idea we work on reviewing. 18:16:11 if everything is looking good, we can hit +A 18:16:20 if not, we have something that can land J1 18:16:30 morganfainberg: ++ 18:16:37 first review (after some sqlalchemy etc administativa if we're doing that) 18:17:05 and yes, i would target this as the absolute first feature of J1 if it pushes 18:17:29 morganfainberg: we said that about i2 -> i3, but the patchset more than doubled in size since then 18:17:41 dolphm, it's a big change. no doubt. 18:18:10 dolphm, but i don't see it doubling or even growing much at this point beyond "cleanup" 18:20:20 morganfainberg, so, I have tested the sql change to do it as an integer instead of a UUID and it looks right 18:20:30 ayoung, ++ cool 18:20:34 dolphm, wanted me to hold of on any more churn on the patch, though 18:20:50 fair enough. 18:20:50 I could potentially do it as a migration if desired 18:20:59 we should do that, but no need to lump it in 18:21:02 I'm going to post it WIP so you guys can see 18:21:44 i'd like to spend as much time as possible today reviewing code, so going to move on... 18:21:48 ayoung, ++ 18:22:09 #topic Fixing multi-backend LDAP with composite user/group IDs 18:22:20 my biggest concern is that i don't know if i have the whole picture in my head about how this impacts security 18:22:29 dolphm, beside that one, are there any other review that is high priority for today? 18:22:37 https://review.openstack.org/#/c/77954/ 18:22:44 ^^ is the WI{P for SQL 18:22:45 gyee: https://review.openstack.org/#/c/55908/77 https://review.openstack.org/#/c/75727/ https://review.openstack.org/#/c/77215/ 18:22:47 not a priority 18:22:51 k 18:23:15 henrynash: shall we defer this conversation until we're open for juno? 18:23:31 dolphm: yes, I think so 18:23:37 henrynash: sounds good 18:23:39 I don't think we should ram this in 18:23:40 #topic Reconstruction of V3 tokens from API on validate - breaks some operator use-cases 18:23:42 morganfainberg: o/ 18:23:49 i can follow up w/ henrynash later w/ what i talked w/ ayoung about last night 18:23:50 ok 18:23:56 thx 18:24:08 sorry henrynash 18:24:12 morganfainberg, what's the problem? bug? 18:24:15 bknudson ++++ 18:24:23 ayoung: getting it right is more improtant 18:24:27 so V3 tokens are reconstructed each time we do a validate - this means you must have a full keystone running to get a token validated. V2 uses data in the persistence 18:24:46 no, its it token data 18:24:49 morganfainberg: v2 did it so v3 can't? 18:24:59 the issue is something Ryan_lane brought up, some people (g and h) only replicate the token store between sites 18:25:11 morganfainberg: uuid tokens? 18:25:11 (unfortunately Ryan_Lane doesn't seem to be on) 18:25:15 morganfainberg: i'm not following 18:25:17 we only reconstruct on mixing use cases, i.e. validating v2 token using v3 API 18:25:46 wikimedia has a multi-region deployment with a replicated token store across each region ( morganfainberg correct me if i'm wrong ) 18:25:50 morganfainberg, let him file a bug 18:26:02 otherwise, Juno summit fodder 18:26:28 gyee, https://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L309 https://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L209 etc 18:26:48 ayoung, i brought this up before i asked him to submit a bug. 18:26:49 gyee, https://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L207 18:26:58 ayoung: wikimedia's architecture is something we supported in essex, folsom, and grizzly. we broke it in havana 18:27:11 (My connectivity may go out in a while…if so I'll reconnect later) 18:27:18 gyee, we basically need to clean that up and ensure we don't have broken tokens. 18:27:26 so if you asked for ?nocatalog before then when you validate you'll get a different catalog? 18:27:30 ephemeral will solve that, too. 18:27:30 bknudson, yep 18:27:34 bknudson: yes 18:27:34 ayoung, correct 18:27:49 ayoung, but we may want to clean this up as a bug in I, backport to H 18:28:03 that is fine....and I think we can do that 18:28:21 ayoung, i wanted a quick consensus from the core team before i had the bug filed 18:28:28 especially now that we support a reddis backend natively, he might be more inclined to participate, too 18:28:40 ayoung, and if we think that fixing / backport is good, we'll get the bug filed and tagged 18:28:49 it's not a lot of cleanup 18:28:50 so i think it's reasonable 18:28:57 morganfainberg: my question is whether it should be an RC blocker? 18:29:01 yeha, reconstitute is wrong. It means that if we slip and don';t revoke the token when one of the roles or something changem the content of the token is different 18:29:05 morganfainberg: given that it's already a bug in havana, i'd think not 18:29:11 dolphm, i think not 18:29:19 dolphm, i think if we miss RC we will backport to I 18:29:25 morganfainberg, https://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L633 18:29:34 and my comment above that line 18:30:03 if token_data is in the ref, we return it as is 18:30:19 gyee, well, sortof. 18:30:24 gyee, there are a lot of gaps that could call into keystone 18:30:36 gyee, we should remove the reconstruct code completely 18:30:49 gyee, make sure we only store valid tokens 18:30:51 morganfainberg, no, fox mixing use cases 18:31:13 unless we abstract the token format 18:31:19 gyee, that is my plan 18:31:37 gyee, yeah...convert v2 to v3 and vice-versa should be a tool in the provider, but shoud not go to the backend unless absolutelt necessary to fill in attributes 18:31:52 ayoung, problem is that signature 18:32:07 for PKI tokens, signature is generate on issuance 18:32:21 gyee, we can still return the same data we had before 18:32:25 gyee, for revocation I wanted to convert a v2 to v3 and then have a unified logic 18:32:44 sure, if we can do it without breaking the signature, I am all for it 18:32:48 gyee, but i expect i can fix the tokens to contain what they need 18:33:22 how do you not break the signature??? 18:33:26 gyee, ayoung, dolphm, i think the consensus is we can fix this, bug filed, and see what we can do 18:33:27 morganfainberg, the tricky part is the signature 18:33:37 morganfainberg: ++ 18:33:43 #topic open discussion 18:33:46 topol, don't temper that data 18:33:54 add the right random data to the end of the token data and you can get the same signature 18:34:02 bknudson, hehe 18:34:19 I have one more topic! 18:34:20 :) 18:34:22 SQL migrate collapse 18:34:26 there's been some discussion in OSSG about doing a thread analysis... 18:34:27 bknudson, I think calculating that data is NP Hard 18:34:36 oops 18:34:39 threat analysis 18:34:54 I am working on collapsing down migrations to start with havana for icehouse (like how nova does it) 18:34:59 the thread of threats? The threat of threads? 18:35:05 any concerns? i think it's generally a positive move 18:35:07 morganfainberg, +++++++++ 18:35:25 i expect to have this in before RC, it's not too complex a change 18:35:26 morganfainberg, just do it from the bottom up, and keep the interim tests if possible 18:35:36 see https://docs.google.com/file/d/0B1aEVfmQtqnoMmpPZ3hmUHpBa1k/edit for the thread modeling process 18:36:01 here's the a keystone-specific one: https://docs.google.com/file/d/0B1aEVfmQtqnobzB6M21uMEFXNUE/edit 18:36:03 and static source code analysis 18:36:05 pen testing 18:36:09 ayoung, i'm working to ensure the schema is the same from 036_havana as 036_token_somethingsomething.py 18:36:24 ayoung, if the schema lands the same, i'm happy, if not... ick 18:36:32 morganfainberg: collapsing the migrations would be good... I suspect it would speed up the tests significantly 18:36:43 although there's probably other ways to speed up the tests too. 18:36:48 ayoung, testing will be mucked with (sql update) once i'm sure the schema is good 18:36:58 bknudson, i think it's important to make it easier to maintain migrations 18:37:00 morganfainberg, we want the interim tests in the migrations that check columns and such...the migrations can probably be dropped 18:37:09 ayoung, ++ yep 18:37:33 ayoung, the "has X changed from Y" checks will need to go esp where they check before/after 18:37:41 ++ 18:37:45 sounds like you are on track 18:37:52 ayoung, it'll resduce sql upgrade complexity 18:37:57 reduce even 18:38:19 bknudson, a minor speedup will be a side benefit 18:38:28 ok thats all from me :) 18:38:46 and I just wanted to bring up the threat analysis if anyone was interested. 18:38:52 bknudson, ++ 18:39:46 I'll just pad a few extra chars to the end of the threat analysis doc until I decide I like it 18:39:49 bknudson, we've done some threat analysis and pen testing internally 18:40:07 gyee, any fuzz testing? 18:40:10 lemme talk to our security folks to see if they want to contribute 18:40:20 no fuzz testing :) 18:40:31 gyee, i'd love to see fuzz testing against our APIs 18:40:33 choas monkey testing, yes 18:40:41 it might be super interesting results 18:41:03 I think we know already that keystone does no input validation. 18:41:08 we also have requirement for FIFS 140-2 certificate, which is insane 18:41:08 except in the few places we've put it in 18:41:10 ++ 18:41:13 certification 18:41:24 I basically had to tell them to back off 18:41:28 gyee: we've had the same FIPS 140-2 request. 18:41:31 bknudson, sure, but it would be cool to have a definitive "here is what we need to fix" target. 18:41:48 bknudson, don't go FIPS 140-2 yet, your life will be miserable 18:41:56 I've got a one line change for 55908. I'm going to post it now 18:42:04 gyee: it has made me miserable already 18:42:10 FIPS! 18:42:21 gyee: have you submitted any vulnerabilities? 18:42:30 i also want to warn everyone about https://blueprints.launchpad.net/keystone/+spec/more-code-style-automation 18:42:49 dolphm, one I think, from last release 18:43:00 actually, ayoung submitted it for me 18:43:07 i was frustrated over the weekend because it seems like all reviews have a lot of the same problems - no i'm automating some of the common ones 18:43:19 dstanek, btw thanks for fixing the comments on ayoung's review, i was too tired to get those in cleanly last night post review 18:43:29 You guys rock...much appreciated 18:43:40 this one was really a team effort, to include YorikSar... 18:43:50 morganfainberg: automation my friend, automation :-) 18:43:55 dolphm, I am thinking submitting another one for scoping trust to ec2 key 18:43:56 dstanek, ++ 18:44:02 that's a vulnerability in the making 18:44:22 dstanek: is your goal to get those into hacking? 18:44:54 dolphm: yes, i'm going to get them running for us first though 18:45:09 can we have keystone-specific hacking? 18:45:16 * lbragstad listens... 18:45:19 bknudson: absolutely! 18:45:22 good question. 18:45:31 can people get tox -edocs to run on their local machines? Am I the only one that has it broken? 18:45:43 dstanek, i like that, i'm gong to add more 18:45:48 we already have keystone-specific: use single quotes overt double quotes when possible 18:46:04 its just small 18:46:04 ayoung, it runs just fine 18:46:10 topol, that's a keystone thing? 18:46:20 i pushed up a hacking check for inline comments, but not sure they agree. 18:46:21 bknudson: i only have 3 of the check i was working on running against our code - i'll submit a review for those and the checks shortly after the meeting 18:46:22 ayoung: works for me... might want to rm -r doc/build doc/source/api 18:46:33 it wa sin keystone hacking last time I looked. and not much else 18:46:49 topol: where is that check? 18:46:52 git clean -xdf doc/ that was probably it 18:46:53 topol: I mean custom automated checks. 18:47:13 no its not automated... :-) 18:47:19 dstanek: actually, the one I pushed for review is the first one on your list 18:47:24 topol: oh, you mean documented. i wrote a check for that already too! 18:47:37 topol: but it's not automated 18:47:40 dstanek: you've got one for use ' ? 18:47:41 yes, our documented list is quite small 18:47:56 bknudson: yes, or " if the string contains a ' 18:48:32 dstanek: great! 18:48:43 these are all either separate functions or classes (based on the type of check) and as a group we can decide what to keep and where i went overboard 18:48:45 I assume it hits a lot of existing code. 18:48:58 bknudson++ 18:49:05 we need a pep8 tool that FIXES the problems, not just complains 18:49:15 bknudson: yeah, yesterday i walked through the code and fixed it for a few of the checks 18:49:20 those are the ones i'll submit today 18:49:34 i'll get to more probably tonight 18:49:52 dstanek anything that keeps the code from getting sloppy via automation is very cool 18:49:56 I'm not sure that we want to add new pep8 checks now... 18:50:18 that could be a lot of churn 18:50:32 bknudson, wait till juno? 18:50:37 topol, PyCharm is pretty good for that 18:50:40 I'd prefer to wait until juno 18:50:50 me too 18:51:28 depends on the changes required to add it. 18:52:09 the first few there were not a ton of changes...but it'll also make patches failed that are already passing in the review process 18:52:38 ayoung, https://review.openstack.org/#/c/77215/4/keystoneclient/middleware/auth_token.py, why are we still using v2.0 to fetch the cert? Didn't jamielennox added the API for v3? 18:52:52 dstanek, i added a few, i really like the idea, and appreciate the effort 18:53:03 gyee, yeah, and we need to update the client 18:53:11 gyee: there is server side support, there isn't client side 18:53:19 gyee, that was just a bug fix change. 18:53:22 this all started when i was thinking about creating a project called nit-picker that would use git-review to pull patches locally and run the checks 18:53:25 bknudson: i'd be happy to see stylistic consistency changes land anytime, even during the push to RC. i'd argue that it will make backporting from master to icehouse later on a bit easier 18:53:32 jamielennox, he's talking about the atomic write change 18:53:34 so should we it in the same patch or a separate one? 18:53:49 separate 18:53:52 gyee, ayoung: sorry haven't been watching 18:53:55 dstanek, i think a lot of that can be moved into hacking eventually. 18:54:01 dstanek, some may not be automatable, but i wanted to add stuff i commonly have to mention in reviews 18:54:05 morganfainberg: i hope so 18:54:28 ayoung, maybe a separate patch, but enabling an options to store the certs in memcache would be nice too 18:54:35 gyee, this is for writing the file, makes no change to fetching it. We can do one for using V3 on top of it, but we need to support both v2 and v3 for a while 18:54:36 stevemar: add whenever you can think of - if we can't automate then at least we have a list to point to for newcomers 18:54:48 dstanek, rgr that 18:54:55 ayoung, k 18:54:56 gyee, memcache does us no good, as we need them in the FS in order to call openssl 18:55:23 we have a review checklist already: https://wiki.openstack.org/wiki/ReviewChecklist 18:55:43 bknudson, AH...reminds me 18:55:53 can we generate the sample conf using setup.py? 18:56:06 if it is generated code, it should not really be checked in 18:56:07 ayoung, yeah, need to figure out how to do this in-process, popen is expensive 18:56:52 https://review.openstack.org/#/c/77215 looks good, I am going to push the button, unless there are objections 18:56:54 on a patch earlier today the pep8 check for the sample failed... I think it was because my version of oslo.messaging didn't match the current one 18:56:55 gyee, it may be, but not in the way you think. popen can be fairly light weight 18:56:56 ayoung, didnt we have a similar conversation to what gyee suggested at the diner at the hackathon??? 18:57:12 they released a new version of the library and that caused the help strings to change! 18:57:15 topol: (gyee wasn't at the hackathon) 18:57:32 topol, must be my evil twin brother 18:57:32 dolphm, I know, but ayoung and I disucssed this 18:57:36 topol: oh the conversation with ayoung as at the hackathon, got it 18:57:42 topol: misread 18:57:46 :-) 18:58:22 ayoung: the goal of checking keystone.conf.sample into the repo is to provide documentation 18:58:51 * dolphm 2 minutes, ish 18:58:58 gyee: it looks ok, i think the comment may in inaccurate though 18:59:35 dstanek, you mean the encoding comment? 18:59:48 dolphm, ayoung , https://review.openstack.org/#/c/77789/ https://review.openstack.org/#/c/77790/ 19:00:03 gyee: yes, https://review.openstack.org/#/c/77215/4/keystoneclient/middleware/auth_token.py 19:00:26 #endmeeting