On 27 February 2015 at 11:56, Robbie Gemmell <[email protected]> wrote: > On 26 February 2015 at 17:52, Andrew Stitcher <[email protected]> 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.
Replied on the wiki once more. https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=51812103&focusedCommentId=51812858#comment-51812858 ..but again for anyone not reading there here is the reply (forgive the wiki text): {quote} 5. The exclusion list is not meant to be used to exclude ANONYMOUS in any way, as ANONYMOUS won't even be offered if authentication is required. {quote} I realise, I was just using that as an example of one of reasons I might use an exclude list, and highlighted that it wouldnt be required in that case because of the alternative method. That said, ANONYMOUS presumably will be offered if the 'force anonymous' method is also in use? {quote} I think having both exclusion and inclusion in the API is the worst option, because then the coder has to understand how the two lists interact (and in the past exactly this sort of issue has been a security risk). As it stands, with the cyrus sasl code you do have both options, just at different levels, you can include mechs by using the cyrus sasl configuration files, and exclude mechs using the API. I think the idea of forcing a specific single mech has some merit - a generalisation of the proposed "force ANONYMOUS" option - it still leaves open the question of interaction with the exclusion list though. {quote} I dont think we need both either, just the positive/inlusion list, but I also dont see it needing to be that confusing if we did, with the simplest case just being to make specifying them mutually exclusive. I think if you generalise the forcing then it simply becomes a 'positive/inclusion' list that can take 1 or more entries, which removes any need for wondering about interaction with an exclusion lists or specific methods for forcing ANONYMOUS etc (which itself already has interesting interaction issues with 'requires auth' and the exclusion list, since someone might put ANONYMOUS in there even if that isnt whats intended). {quote} 6. I don't (didn't?) think the change of default is that significant as all the server code in the proton tree previously allowed (or even required) anonymous , in that code even though SASL was required the ANONYMOUS mech was specified. All that the new default has done is to remove a level of unnecessary processing. Of course the situation in the Java tree might be different. {quote} We may be discussing things from different viewpoints. I dont care about the server code in the proton tree, since we can change that to do whatever we like based on what the engine does. That code is not the only use of the engine. {quote} In any case, I find it hard to believe that someone writing a production secured server wouldn't think about security enough to know whether they wanted authenticated and/or encrypted connections and specify them as such. {quote} That you believe it to be so certain to be specified is more reason in my mind to default to the previous behaviour. {quote} I will emphasize that the old "skip SASL" option was not in itself related to authentication or not as the ANONYMOUS SASL mech was still allowed. The new code unifies the default across the layers and allows you to skip an unnecessary layer - This change should definitely be release noted. {quote} As above, it might have been allowed in *our* code, but that is not the only code to consider. {quote} Your point about just removing pn_sasl_allow_skip() is noted (and I agree), given my previous reasoning for pn_sasl_client() and pn_sasl_server() it should go (and it will force anyone using it to understand the change in default too) {quote} Unfortunately the only people likely to be using it will be the people who wouldnt really need to understand the change in default. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
