On 26 February 2015 at 17:52, Andrew Stitcher <astitc...@redhat.com> wrote: > On Thu, 2015-02-26 at 12:28 +0000, Robbie Gemmell wrote: >> ... >> I'm going to post my comments here and on the wiki, as I dont think >> many (except maybe you) will actually see them on the wiki ;) > > Thank you for the excellent feedback. I'm going to answer on the wiki - > as it'll save me from cutting and pasting. > > [I did try to add the lists as watchers, but that doesn't seem to be > possible in any obvious way] > > wiki link: > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812468#comment-51812468 > > Andrew > >
Replied on the Wiki, but also including below for anyone not clicking these links ;) https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812571#comment-51812571 3. Do you really mean to have only ANONYMOUS for 0.9 for Proton-J? That would render it fairly useless if the existing transport/sasl API is removed since that would prevent people using any existing implementation they already have (ANONYMOUS, PLAIN, CRAM-MD5 thus far for the JMS client, with EXTERNAL sitting on the TODO list). 4. I do still think there is a change (by having to handle the SASL previously, you would always know the authentication result before the connection ever happened, and you had to call done on the server to say it was, whereas now you get a transport event in either case), but I take your point that you could adjust to waiting for the successful connection open state/event to be reached and both approaches would then appear to work the same from that point. 5. I'd say the positive list of mechs to use is more straight forward in many scenarios, and the exclusion list could work better in some others. Offering both options would let people do whatever they want. - The exclusion list might be something you used initially to e.g prevent use of ANONYMOUS, but you have that scenario covered by pn_transport_require_auth already. - After that, it is presumably to be used for things like disabling mechs that are either not considered secure enough (e.g PLAIN, because you are requiring everying uses SCRAM-SHA-<foo> today because its Friday), or are installed but not configured/usable (e.g GSSAPI as you mentioned). Between those, you would need to know the name of every mechanism that could ever be present, and then exclude them all in case they should be installed (possibly later). Or, you might better get what you want by being able to specify only the mechs you want to be enabled and have already installed as a result. - On what to do to do if a positively-requested mechanism isnt available, as I said we could posibly give the choice of of throwing to indicate the requirement couldn't be met or potentially making that optional, because if you positively-specify more than one you probabyl dont care which is used too much. 6. I saw the deprecation, but I wasnt aware the default had already been changed for allow skip, was that ever mentioned anywhere, e.g a JIRA raised to cover a significant change in behaviour? I think its the wrong decision personally, and code using the engine should have to opt-in to permitting non-SASL connections (and ANONYMOUS). If the rest of the current SASL API is going away and the default remains changed, then it seems allow skip should probably just be removed too, since its very likely the only people using it will be configuring the new behaviour and there is a new API for anyone wanting to configure the historical default behaviour when they realise (this will presumably be release-noted?) it becomes necessary to secure it with the new release.