20:03 <dpm> #startmeeting 20:03 <meetingology> Meeting started Tue Aug 12 20:03:03 2014 UTC. The chair is dpm. Information about MeetBot at http://wiki.ubuntu.com/meetingology. 20:03 <meetingology> 20:03 <meetingology> Available commands: action commands idea info link nick 20:03 <dpm> so essentially, the main topic today is the security checks branch: 20:03 <dpm> https://code.launchpad.net/~ubuntu-filemanager-dev/ubuntu-filemanager-app/require-screenlock-password/+merge/230058 20:04 <dpm> I'd like to see if we can come up with a solution for RTM that is doable and still secure 20:04 <ajalkane> What's RTM dead-line? 20:05 <dpm> end of August, although we'd like this to land as soon as possible 20:05 <dpm> we originally targeted the 7th August 20:05 <dpm> and thanks ajalkane for getting your branch submitted before that 20:06 <dpm> so ajalkane has assessed the options from his POV at https://code.launchpad.net/~ubuntu-filemanager-dev/ubuntu-filemanager-app/require-screenlock-password/+merge/230058/comments/559751 20:06 <dpm> and sarnold has already looked at this from the security POV 20:07 <dpm> sarnold, what do you think of those options? As said before, my concern is that I don't think we might be able to deliver a full-blown PAM plugin by RTM 20:07 <dpm> or do you have other suggestions? 20:08 <sarnold> dpm: unfortunately, I'm very new to qml, and I don't know what's possible or clean in it 20:08 <sarnold> dpm: I agree with ajalkane that multithreading the interactions is probably going to be more complication 20:09 <sarnold> dpm: .. and doing our own qt event loop in another thread is way beyond my experience, but that also feels like it's going to be fraught with complications 20:09 <ajalkane> One clarification to my post is, that the 1) option in my first post is something that probably could be done in number of days instead of weeks. The other ones are more problematic. 20:10 <sarnold> so, even though there is a risk of the PAM module blocking execution of the UI thread, I think that's the option that makes most sense 20:11 <ajalkane> Ah I forgot that blocking of UI thread. The problem is that if Pam module wants to display UI it can't block the UI thread. It'd just hang 20:11 <dpm> hm, bummer 20:11 <sarnold> I don't think the PAM module should be calling into Qml, that also sounds like trouble; but the existing conversation function feels like it is 90% of the way there -- as I see it, it probably just needs to make visible the right dialog boxes at the righ time 20:11 <ajalkane> Pam library is a little troublesome in that way, that it's not asynchronous conversation. It's synchronous. 20:12 <ajalkane> So it practically needs to have a separate thread for back and forth passing of data 20:12 <dpm> ajalkane, you had a look at how the ubuntu settings app does authentication - is this something we could learn or reuse from that? 20:12 <dpm> I think it was calling unix binaries, though, IIRC 20:13 <sarnold> ajalkane: but is it really a trouble? it'll be blocked on the user to type in e.g. a password, and then turn around to authenticate the password 20:13 <ajalkane> I had a little look at various pieces of code, some of them used Gnome code which I have no good understanding, some of them called directly unix commands 20:13 <sarnold> ajalkane: granted, if it goes away to call an LDAP server or RADIUS server it could be gone for a while -- and even average case might be a second or two -- but until authentication succeeds or fails, it's not really safe to continue, is it? 20:14 <ajalkane> sarnold: it's a problem if we use Qt to display the asking of password. If Qt's UI thread is blocking asking for pam module to authenticate, and the pam module in turn asks Qt's UI thread to display a dialog, it probably don't work - but I'm not totally sure 20:15 * popey returns 20:15 <sarnold> ajalkane: I don't think the second half of that would happen -- all the interactivity happens in your conversation function, which you control, right? 20:16 <ajalkane> sarnold: depends how this is setup. Is the conversation function ran in another thread, or in Qt's UI thread? 20:16 <sarnold> ajalkane: I'm thinking in the Qt UI thread 20:17 <sarnold> I'm sorry I'm not more experienced with Qml and Qt; it's been on the tail end of my todo list for far too long 20:17 <ajalkane> If it's ran in Qt's UI thread then Qt's UI thread has to have made a blocking call to the pam library. Pam library making a blocking call to the Qt's UI probably will make the whole thing stuck. 20:18 <ajalkane> I'm thinking the processing of events is stuck when making the call to pam library. Pam library trying to pass events to Qt at that point won't work. But this is also just best-guess from me. 20:19 <sarnold> ajalkane: when would Pam make a call into Qt's UI? would that be for more complicated modules that require something beyond just text-based interaction? 20:19 <ajalkane> sarnold: Even text-based interaction would require calling Qt's UI, just to let user enter the password. 20:20 <ajalkane> That's why the current implementation asks for the password before doing the pam authentication - so that we have the password to answer with when pam conversation callback needs it. 20:23 <sarnold> yeah, once I figured out why you were asking for the password first, it all made more sense :) 20:24 <sarnold> it sure feels like you ought to be able to define a few more dialogs similar to the one hidden by the Button "Unlock full access"'s onclicked: piece... 20:24 <sarnold> have their visible: components be set or unset by the conversation callback 20:25 <sarnold> and have the dialogs kick off the next back-and-forth with the conversation callback 20:26 <ajalkane> That'd need launching a separate thread for PAM authentication and all the nasty signalling between threads with that 20:26 <ajalkane> because if we're running in single thread, the passing of information can go only one way. From QML to PAM library. 20:27 <ajalkane> Because the pam conversation function is a blocking synchronous call, not asynchronous 20:28 <ajalkane> Not sure if that makes any sense, but ask away and I'll try to elaborate what I see as the problem 20:29 <sarnold> ajalkane: your conversation function currently builds the response right then.. 20:29 <sarnold> ajalkane: I don't think that is necessary 20:30 <ajalkane> But if we only need to support PAM's "show some information" and not get any information (apart from password) back to QML, then that'd work with single thread 20:30 <ajalkane> We just can't get any back-and-forth conversation between PAM library and the QML part except for pre-entered information, without using separate threads 20:31 <ajalkane> * back to PAM library 20:31 <sarnold> ajalkane: as I'm imagining this working, the conversation callback would perform the same switch statement, set one of four visible: flags, and then return PAM_SUCCESS immediately.. 20:31 <sarnold> ajalkane: one of those four dialog boxes would then pop up, show the text from the pam_response structs, construct echoing fields or non-echoing fields.. 20:32 <ajalkane> sarnold: that can be done. So enter password before calling pam, store PAM_TEXT_INFO and PAM_ERROR_MSG notifications and show them? 20:32 <sarnold> ajalkane: .. and then when the 'submit' button is hit, they'd then .... oh that's where I lose track of things.. 20:33 <sarnold> ajalkane: but what if the PAM module asks for the HOTP input from a google authenticator or yubikey or rsa secid? 20:33 <sarnold> let me go re-read the pam conversation docs :) 20:34 <dpm> sarnold, do we really need to care for non-text-based authentication for RTM? 20:35 <ajalkane> For all these options I think a separate OS provided conversation function would be the best choice - it'd be available to all applications, and only OS vendor can support all options. 20:35 <dpm> I'm sure we can add them afterwards, but I think we should limit the scope to what's reasonable and still secure 20:35 <sarnold> dpm: I'm fine handling only text-based stuff for RTM indeed :) 20:36 <dpm> ok, cool 20:36 <sarnold> when someone comes along with something compelling that requires something more than the usual text-based dialogs, we can handle it then :) 20:36 * dpm nods 20:36 <ajalkane> But even in that case making it work with QML would (IMO) require a separate thread and probably own Qt Event loop or some other mechanism for handling it 20:36 <sarnold> ajalkane: okay, I think where I ran off the rails was in the assumption that the application could respond to the conversation callback "eventually" 20:37 <sarnold> dpm: I definitely agree with ajalkane here, it'd be wonderful to have the sdk provide this mechanism from the start 20:38 <sarnold> dpm: not every application can (or should) do PAM-based authentication, but we've got at least two right now that do (fileman and terminal), and having a unified mechanism for them to use would make most sense 20:39 <dpm> sarnold, indeed, but I'm not sure if we should foresee any other applications that need authentication. We're building this mechanism simply to ensure these 2 unconfined apps are more secure 20:40 <dpm> the only other one I could think of is ubuntu-system-settings 20:41 <dpm> but I'm not too sure this would be a feature that would be added to the SDK roadmap any time soon 20:41 <sarnold> dang. makes sense though, they've got priorities same as everyone else.. 20:41 <dpm> and ideally we should try to reduce the number of unconfined apps, not adding more :) 20:42 <sarnold> :) 20:42 <sarnold> so.... what's involved in kicking off a new thread to handle the pam conversation function here? 20:43 <ajalkane> Tinkering and testing 20:43 <sarnold> that also sounds expensive :) 20:44 <ajalkane> Yeah. Regarding RTM deadline, not at all sure I can put enough hours myself to make it 20:45 <sarnold> makes sense. 20:45 <sarnold> I don't think I can learn enough qt/qml to do it myself, though I can review anything :) 20:46 <dpm> sarnold, ajalkane, do you think one of you could summarize what you think we should make? Just to make sure we're all on the same page 20:47 <sarnold> dpm: well, I think ajalkane has talked me into understanding that it can't be fixed simply. 20:49 <ajalkane> My last post on the branch summarized the options that I can see if current implementation must be changed. My practical recommendation if RTM is to be made for sure is the current implementation with few targetted tweaks if necessary. Otherwise it's potentially expensive with high probability of not making RTM. 20:51 <sarnold> dpm: the current implementation would work assuming no one fiddles with their file manager PAM configuration. It kinda defeats the purpose of having authentication be pluggable :) but given the constraints, I can see that leaving it alone has its appeal 20:51 <ajalkane> The current implementation works with passwords - if there's upcoming devices with fingerprints and other such more involved authentications methods in pipeline, then I say scratch the whole thing and work on it 20:53 <sarnold> I wouldn't want to -support- the current merge proposal for very long -- to draw an analogy, I wouldn't want it in an LTS release, but in one of the shorter releases it'd be an alright stepping stone 20:53 <ajalkane> yes, my understanding also was that this was just a stepping-stone for RTM 20:53 <dpm> ajalkane, sarnold, thanks. Ok, to answer these points: sarnold, how could a user fiddle with their file manager config? ajalkane, there are no plans to support more advanced authentication devices (at least afaik). If we were to go with the current implementation, do you guys think we could agree to a list of changes required? 20:54 <dpm> ajalkane, indeed, that's the plan 20:54 <dpm> sarnold, also note that this is a click package, so it's not got the same level of support as packages in the archive - we can more easily update the file manager app from the store 20:55 <sarnold> dpm: if a user edits /etc/pam.d/filemanager and replaces pam_unix or pam_extrausers with pam_permit (to disable the password entirely), the'll still be prompted for passwords 20:55 <ajalkane> dpm: I wholeheartedly recommend Ubuntu Touch to provide it's own PAM library callback that displays the UI for authentication. Then the PamAuthentication Plugin can just launch a thread and pass it the conversation function 20:57 <dpm> sarnold, but for that they'd need to use the terminal app, which would be locked, right? (the idea is to use the same QML plugin as file manager) 20:57 <ajalkane> sarnold: I do think a user capable of that, could just as easily modify filemanager's QML file to not ask for any password 20:57 <dpm> indeed, qml files are text-based 20:58 <ajalkane> While it's annoying that user who knows how to edit the pam files, should have to edit many different files to get what he wants, is annoying. But it's not something that leaves unassuming user at risk as such. 20:58 <ajalkane> Which is what I guess we're trying to protect here 20:59 <sarnold> heh, sure, disabling the password was just one possibility :) someone else might want to add pam_duo for two-factor authentication 21:01 <dpm> so perhaps the current implementation is not too bad security-wise as the first iteration? 21:01 <sarnold> yeah 21:02 <ajalkane> that's nice 21:02 <dpm> cool 21:03 <sarnold> this is going in the store, right? we can ship fixes to this more simply than the full image updates, right? 21:03 <ajalkane> is there any consensus how it should move forward or is it something to sit on for a while? OS provided conversation function or improvements to the plugin? 21:03 <dpm> sarnold, indeed, it's a click package that we update in the store 21:04 <dpm> ajalkane, I'd say, if sarnold agrees, the second: improvements to the current plugin in the MP 21:04 <dpm> sarnold, so do you think you could do a further review in the light of the discussion here, and perhaps list any caveats that you see should be addressed post-RTM? 21:04 <sarnold> ajalkane: in some sense this feels like a chef's choice -- if you want to tackle a full mechanism here, that'd be great, but I can ask the sdk team if they can put some cycles on it eventually if you'd rather just be done with it :) 21:06 <ajalkane> sarnold: I feel like this is so close to the core OS, that it'd be better for the OS developers to iterate all possibilities and implement a unifying experience. But for sure it can be done in the plugin also, but it's kind of half-way-there solution. 21:06 <sarnold> ajalkane: okay, thanks 21:08 <dpm> ajalkane, I've come to agree from your feedback. However, it's also a matter of priorities, and for now this is not something that any other apps than the 2 unconfined ones are going to use 21:09 <dpm> so it might not be something that we see the OS providing soon 21:09 <ajalkane> dpm: yeah, I understand. Priorities are annoying in perfect world :P 21:09 <dpm> lol 21:12 <ajalkane> so we'll after feedback from SDK devs what's the course of action after RTM? I can do a little tinkering and testing, but since I have a pretty poor understanding of the requirements and also of pam libraries I'd need quite specific specifications what's needed to do any real work. 21:12 <ajalkane> * веэлл сее 21:12 <dpm> sarnold, what do you think of the suggestion to do another review to the current MP and list caveats/points that should be addressed after RTM? Does it make sense to you? 21:13 <sarnold> dpm: I believe ajalkane addressed all my other complaints :) 21:13 <dpm> sarnold, ok, cool, so after the discussion, do you think the branch is good to go as it is? 21:13 <sarnold> dpm: yes 21:14 <ajalkane> I just noticed I forgot to add the code to not allow locked accounts, so I'll just fix that to the branch 21:14 <sarnold> ajalkane: sweet 21:14 <dpm> ajalkane, cool, thanks. And yes, I'll kick off a discussion to agree on post-RTM requirements 21:14 <ajalkane> I'll try and fix it during this week 21:15 <ajalkane> (I'm having a yummy upgrade from 12.04 to 14.04.1 waiting that might delay this a bit) 21:16 <dpm> oh wow, good luck (should work well, though) 21:16 <sarnold> oof :) 21:16 <ajalkane> I was surprised how well the last LTS to LTS upgrade work. Previously I was always reinstalling as the upgrade fucked up. * Fingers crossed * 21:16 <dpm> ok, I think we can call it a meeting. To summarize: 21:16 <dpm> :-) 21:17 <ajalkane> Good summary, thanks all! 21:17 <sarnold> :) 21:17 <dpm> lol 21:17 <sarnold> thanks ajalkane, dpm :) 21:17 <dpm> I was going to be a bit more verbose, but that's also a good summary :) 21:18 <dpm> - ajalkane to add fix to the current branch to not allow access to locked accounts 21:18 <dpm> - sarnold to review again after that fix 21:18 <dpm> - dpm to kick off a discussion for post-RTM requirements 21:19 <dpm> Good work everyone, and thanks! 21:19 <ajalkane> Thanks, cya next week. And thanks sarnold for taking the time to attend and work on the issues 21:19 <dpm> #endmeeting