You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Kishan Kavala <Ki...@citrix.com> on 2013/05/16 12:39:48 UTC

Review Request: Added PlainTextAuthenticator

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11194/
-----------------------------------------------------------

Review request for cloudstack and Chip Childers.


Summary (updated)
-----------------

Added PlainTextAuthenticator


Description (updated)
-------

Added PlainTextAuthenticator for backward compatibility. Removed MD5 auth from PlainTextAuthenticator. It just does plain text compare.


This addresses bug CLOUDSTACK-2516.


Diffs (updated)
-----

  client/tomcatconf/applicationContext.xml.in 849c0bc 
  client/tomcatconf/componentContext.xml.in ecd4a11 
  plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3 

Diff: https://reviews.apache.org/r/11194/diff/


Testing (updated)
-------

Tested login with password sent as both MD5 hash and plaintext


Thanks,

Kishan Kavala


Re: Review Request: Added PlainTextAuthenticator

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11194/#review20644
-----------------------------------------------------------


This should not be merged until consensus on the approach is reached via the ML discussion.

- Chip Childers


On May 16, 2013, 10:39 a.m., Kishan Kavala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11194/
> -----------------------------------------------------------
> 
> (Updated May 16, 2013, 10:39 a.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> Added PlainTextAuthenticator for backward compatibility. Removed MD5 auth from PlainTextAuthenticator. It just does plain text compare.
> 
> 
> This addresses bug CLOUDSTACK-2516.
> 
> 
> Diffs
> -----
> 
>   client/tomcatconf/applicationContext.xml.in 849c0bc 
>   client/tomcatconf/componentContext.xml.in ecd4a11 
>   plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3 
> 
> Diff: https://reviews.apache.org/r/11194/diff/
> 
> 
> Testing
> -------
> 
> Tested login with password sent as both MD5 hash and plaintext
> 
> 
> Thanks,
> 
> Kishan Kavala
> 
>


Re: Review Request: Added PlainTextAuthenticator

Posted by Chip Childers <ch...@sungard.com>.
On Thu, May 16, 2013 at 04:03:14PM +0200, Ove Ewerlid wrote:
> I vote -1 for enabling plain text authentication allowing auth
> directly against hashes. I'm not clear if this functionality exists
> in ACS4.0, I would assume not.
> 
> The API breakage reported was in createUser, where the ability to
> pass in a hash has value. Think migration scenarios where only the
> hash is known.
> 
> createUser, createAccount and createDomain have, in v41, been
> enhanced with parameters to allow specifying the UUID directly to
> accommodate for external provisioning (or migration from older
> systems). The ability to pass in existing hashes has value in these
> scenarios. There is also value in being able to pass in a plain text
> password and have it encrypted depending on how the external account
> provisioning is done. Seems a new parameter is needed in createUser
> to allow both while retaining backwards compat. Perhaps a parameter
> specifying the type of hash or if the password is plain text that
> needs to be encrypted. If this parameter is not present, the
> assumption should be that the password is an MD5 hash, the old
> behavior.

Great ideas for how to remain backward compatible Ove.

Kishan - I think this is a strong case.  Compatibility is required to
keep the 4.* version number, and I think we've broken that compatibility
as of right now.  What do you think about Ove's ideas?

> 
> /Ove
> 
> On 05/16/2013 03:23 PM, Kishan Kavala wrote:
> >
> >
> >>-----Original Message-----
> >>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> >>Sent: Thursday, 16 May 2013 6:25 PM
> >>To: dev@cloudstack.apache.org
> >>Subject: Re: Review Request: Added PlainTextAuthenticator
> >>
> >>On 05/16/2013 02:16 PM, Kishan Kavala wrote:
> >>>Ove,
> >>>    Plain text authenticator will allow logging using the hash value. Or else,
> >>clients sending MD5 hash will fail to login. This is primarily for backward
> >>compatibility.
> >>>To avoid logging in using has value itself, plain text authenticator can be
> >>removed from auth adapter list, provided the client sends plain text instead
> >>of hash.
> >>
> >>I'm not seeing the plain-text authenticator in ACS4.0 list of authenticators
> >>(components.xml). MD5 and LDAP are listed. Help me out, where in ACS4.0 is
> >>the code to allow login using the password hash itself?
> >>
> >>/Ove
> >
> >
> >I checked 4.0 code.  plain-text authenticator is not in components.xml but it is part of the code.
> >
> >plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> >
> >It does MD5 has compare instead of plain text (don't know why), so it may not serve u'r purpose even after adding it to components.xml
> >
> >>
> >>
> >>>~kishan
> >>>
> >>>>-----Original Message-----
> >>>>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> >>>>Sent: Thursday, 16 May 2013 5:33 PM
> >>>>To: dev@cloudstack.apache.org; Kishan Kavala
> >>>>Subject: Re: Review Request: Added PlainTextAuthenticator
> >>>>
> >>>>Hi Kishan!
> >>>>
> >>>>Did you verify that adding the plain text authenticator will not
> >>>>allow login using the hash value itself?
> >>>>
> >>>>
> >>>>from AccountManagerImpl.java;
> >>>>    ... getUserAccount ...
> >>>>    ...
> >>>>     boolean authenticated = false;
> >>>>            for(UserAuthenticator authenticator : _userAuthenticators) {
> >>>>                if (authenticator.authenticate(username, password,
> >>>>domainId, requestParameters)) {
> >>>>                    authenticated = true;
> >>>>                    break;
> >>>>                }
> >>>>            }
> >>>>    ...
> >>>>
> >>>>/Ove
> >>>>
> >>>>On 05/16/2013 12:39 PM, Kishan Kavala wrote:
> >>>>>
> >>>>>-----------------------------------------------------------
> >>>>>This is an automatically generated e-mail. To reply, visit:
> >>>>>https://reviews.apache.org/r/11194/
> >>>>>-----------------------------------------------------------
> >>>>>
> >>>>>Review request for cloudstack and Chip Childers.
> >>>>>
> >>>>>
> >>>>>Summary (updated)
> >>>>>-----------------
> >>>>>
> >>>>>Added PlainTextAuthenticator
> >>>>>
> >>>>>
> >>>>>Description (updated)
> >>>>>-------
> >>>>>
> >>>>>Added PlainTextAuthenticator for backward compatibility. Removed
> >>MD5
> >>>>auth from PlainTextAuthenticator. It just does plain text compare.
> >>>>>
> >>>>>
> >>>>>This addresses bug CLOUDSTACK-2516.
> >>>>>
> >>>>>
> >>>>>Diffs (updated)
> >>>>>-----
> >>>>>
> >>>>>     client/tomcatconf/applicationContext.xml.in 849c0bc
> >>>>>     client/tomcatconf/componentContext.xml.in ecd4a11
> >>>>>     plugins/user-authenticators/plain-
> >>>>text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> >>>>52e7cb3
> >>>>>
> >>>>>Diff: https://reviews.apache.org/r/11194/diff/
> >>>>>
> >>>>>
> >>>>>Testing (updated)
> >>>>>-------
> >>>>>
> >>>>>Tested login with password sent as both MD5 hash and plaintext
> >>>>>
> >>>>>
> >>>>>Thanks,
> >>>>>
> >>>>>Kishan Kavala
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>--
> >>>>Ove Everlid
> >>>>System Administrator / Architect / SDN & Linux hacker
> >>>>Mobile: +46706662363
> >>>>Office: +4618656913 (note EMEA Time Zone)
> >>
> >>
> >>--
> >>Ove Everlid
> >>System Administrator / Architect / SDN & Linux hacker
> >>Mobile: +46706662363
> >>Office: +4618656913 (note EMEA Time Zone)
> 
> 
> -- 
> Ove Everlid
> System Administrator / Architect / SDN & Linux hacker
> Mobile: +46706662363
> Office: +4618656913 (note EMEA Time Zone)
> 

Re: Review Request: Added PlainTextAuthenticator

Posted by Ove Ewerlid <Ov...@oracle.com>.
On 05/16/2013 10:55 PM, Chip Childers wrote:
>> For those moving from 2.2.x, 3.0.x, 4.0 to 4.1:
>> 1. We remove the incorrect auth mechanism and put in the right fix of
>> encoding at the server and not doing any UI magic.
>> 2. We correct the API docs and other docs to indicate the user to send
>> in plaintext so clients can adjust to the change.
>> 3. We describe this migration situation as Ove encountered and how it
>> can be corrected without any change using the plaintext authenticator.
>>
>> I hope that this is fixed right and at the same time it doesn't break
>> backwards compatibility which is the solution that Kishan is proposing
>> and I'd recommend too.
>
> Well said Prasanna.  I follow now.
>
> So I'll pull in the patch.  What's missing though, is an update to the
> release notes that describes the situation.
>
> If someone wants to add that, then we can proceed with closing the bug
> IMO.  If someone simply wants to write it into an email, I'll add it to
> the release notes XML file if you want.
>
> Let's keep the bug open until we get it documented though...
>
> -chip
>

+1

I was baffled by the fact that the server side authentication process up 
until now did not expect plain text passwords, that had me confused on 
what Kishan was communicating. From my point of view, fixing this design 
flaw is a must and motivates the user provisioning breakage and an 
improved hash with salt adds additional icing. All is good.

For migration and provisioning scenarios requiring adding hashes 
directly, there is always direct DB access.

/Ove


-- 
Ove Everlid
System Administrator / Architect / SDN & Linux hacker
Mobile: +46706662363
Office: +4618656913 (note EMEA Time Zone)

Re: Review Request: Added PlainTextAuthenticator

Posted by Chip Childers <ch...@sungard.com>.
On Thu, May 16, 2013 at 09:50:21PM +0530, Prasanna Santhanam wrote:
> On Thu, May 16, 2013 at 04:03:14PM +0200, Ove Ewerlid wrote:
> > I vote -1 for enabling plain text authentication allowing auth
> > directly against hashes. I'm not clear if this functionality exists
> > in ACS4.0, I would assume not.
> > 
> > The API breakage reported was in createUser, where the ability to
> > pass in a hash has value. Think migration scenarios where only the
> > hash is known.
> > 
> > createUser, createAccount and createDomain have, in v41, been
> > enhanced with parameters to allow specifying the UUID directly to
> > accommodate for external provisioning (or migration from older
> > systems). The ability to pass in existing hashes has value in these
> > scenarios. There is also value in being able to pass in a plain text
> > password and have it encrypted depending on how the external account
> > provisioning is done.
> 
> Chip, Ove,
> 
> There's two parts to this process - the auth and the encode.
> 
> In auth - existing tenants of your system send through their passwd
> over the wire that is compared with the password in your cloudstack
> database as follows:
> 
> Order of authenticators
> SHA256 > MD5 > PlainText
> 
> For a moment assume that Alice (existing user) sends only plaintext
> passwords as she entered in the system when her account was created:
> Her password in the db is say alicesecretsauce and she passes
> alicesecretesauce over-the-wire.
> 
> CloudStack will do the following while authenticating Alice:
> 1. Is SHA256(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 
> 2. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 
> 3. Is alicesecretsauce == CloudStack_DB(alicesecretesauce) 
> 
> In your case since the DB contains the MD5 of alicesecretsauce against
> Alice's account the second comparison returns and authenticates Alice
> successfully after SHA256 fails.
> 
> Now let's say you upgrade to 4.1 with the same order of authenticators
> and bug fixed as sent in the patch by Kishan:
> 
> Let's look at Alice's case again:
> She sends alicesecretsauce over-the-wire - and the same process
> works out for her and she is able to login.
> 
> Now let's say Bob is a new account that is created in your system
> post-upgrade to 4.1:
> 
> When Bob creates his account, his password is encoded using the SHA256
> scheme since that's the first one in the configured list. So all new
> accounts now have a SHA256 value in the DB against them.
> 
> When Bob attempts to login the first comparison ie
> SHA256(bobsecretsauce) == CloudStack_DB(bobsecretsauce) and he too is
> allowed to login.
> 
> Coming to your scenario where you want to hash passwords which are
> coming over-the-wire: The scenario before upgrade should be clear so I
> won't explain it here.
> 
> Post-upgrade:
> Alice sends MD5(alicesecretsauce) as per your provisoner-
> 
> 1. Is SHA256(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 
> 2. Is MD5(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 
> 3. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 
> 
> So she is authenticated using the plaintext authenticator now in 3.
> Without that her auth fails. This is what Kishan is asking that you
> enable.
> 
> Bob on the other hand sends in MD5(bobsecretsauce) and his account was
> saved in the DB when your provisioner created his account with
> passwd:  SHA256(MD5(secretsauce)) thereby for him the 1st
> authenticator works and helps him login to cloudstack.
> 
> If I were you - I'd migrate everything with the plaintext
> authenticator enabled and then switch over to an auth mechanism that
> suits my security needs and my external provisioner.
> 
> For those moving from 2.2.x, 3.0.x, 4.0 to 4.1:
> 1. We remove the incorrect auth mechanism and put in the right fix of
> encoding at the server and not doing any UI magic.
> 2. We correct the API docs and other docs to indicate the user to send
> in plaintext so clients can adjust to the change.
> 3. We describe this migration situation as Ove encountered and how it
> can be corrected without any change using the plaintext authenticator.
> 
> I hope that this is fixed right and at the same time it doesn't break
> backwards compatibility which is the solution that Kishan is proposing
> and I'd recommend too.

Well said Prasanna.  I follow now.

So I'll pull in the patch.  What's missing though, is an update to the
release notes that describes the situation.

If someone wants to add that, then we can proceed with closing the bug
IMO.  If someone simply wants to write it into an email, I'll add it to
the release notes XML file if you want.

Let's keep the bug open until we get it documented though...

-chip

RE: Review Request: Added PlainTextAuthenticator

Posted by Koushik Das <ko...@citrix.com>.
Nice explanation Prasanna.
+1 to Kishan's fix.

> -----Original Message-----
> From: Prasanna Santhanam [mailto:tsp@apache.org]
> Sent: Thursday, May 16, 2013 9:50 PM
> To: dev@cloudstack.apache.org
> Cc: Kishan Kavala
> Subject: Re: Review Request: Added PlainTextAuthenticator
> 
> On Thu, May 16, 2013 at 04:03:14PM +0200, Ove Ewerlid wrote:
> > I vote -1 for enabling plain text authentication allowing auth
> > directly against hashes. I'm not clear if this functionality exists in
> > ACS4.0, I would assume not.
> >
> > The API breakage reported was in createUser, where the ability to pass
> > in a hash has value. Think migration scenarios where only the hash is
> > known.
> >
> > createUser, createAccount and createDomain have, in v41, been enhanced
> > with parameters to allow specifying the UUID directly to accommodate
> > for external provisioning (or migration from older systems). The
> > ability to pass in existing hashes has value in these scenarios. There
> > is also value in being able to pass in a plain text password and have
> > it encrypted depending on how the external account provisioning is
> > done.
> 
> Chip, Ove,
> 
> There's two parts to this process - the auth and the encode.
> 
> In auth - existing tenants of your system send through their passwd over the
> wire that is compared with the password in your cloudstack database as
> follows:
> 
> Order of authenticators
> SHA256 > MD5 > PlainText
> 
> For a moment assume that Alice (existing user) sends only plaintext
> passwords as she entered in the system when her account was created:
> Her password in the db is say alicesecretsauce and she passes
> alicesecretesauce over-the-wire.
> 
> CloudStack will do the following while authenticating Alice:
> 1. Is SHA256(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 2. Is
> MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 3. Is
> alicesecretsauce == CloudStack_DB(alicesecretesauce)
> 
> In your case since the DB contains the MD5 of alicesecretsauce against Alice's
> account the second comparison returns and authenticates Alice successfully
> after SHA256 fails.
> 
> Now let's say you upgrade to 4.1 with the same order of authenticators and
> bug fixed as sent in the patch by Kishan:
> 
> Let's look at Alice's case again:
> She sends alicesecretsauce over-the-wire - and the same process works out
> for her and she is able to login.
> 
> Now let's say Bob is a new account that is created in your system post-
> upgrade to 4.1:
> 
> When Bob creates his account, his password is encoded using the SHA256
> scheme since that's the first one in the configured list. So all new accounts
> now have a SHA256 value in the DB against them.
> 
> When Bob attempts to login the first comparison ie
> SHA256(bobsecretsauce) == CloudStack_DB(bobsecretsauce) and he too is
> allowed to login.
> 
> Coming to your scenario where you want to hash passwords which are
> coming over-the-wire: The scenario before upgrade should be clear so I
> won't explain it here.
> 
> Post-upgrade:
> Alice sends MD5(alicesecretsauce) as per your provisoner-
> 
> 1. Is SHA256(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 2.
> Is MD5(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 3. Is
> MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce)
> 
> So she is authenticated using the plaintext authenticator now in 3.
> Without that her auth fails. This is what Kishan is asking that you enable.
> 
> Bob on the other hand sends in MD5(bobsecretsauce) and his account was
> saved in the DB when your provisioner created his account with
> passwd:  SHA256(MD5(secretsauce)) thereby for him the 1st authenticator
> works and helps him login to cloudstack.
> 
> If I were you - I'd migrate everything with the plaintext authenticator
> enabled and then switch over to an auth mechanism that suits my security
> needs and my external provisioner.
> 
> For those moving from 2.2.x, 3.0.x, 4.0 to 4.1:
> 1. We remove the incorrect auth mechanism and put in the right fix of
> encoding at the server and not doing any UI magic.
> 2. We correct the API docs and other docs to indicate the user to send in
> plaintext so clients can adjust to the change.
> 3. We describe this migration situation as Ove encountered and how it can be
> corrected without any change using the plaintext authenticator.
> 
> I hope that this is fixed right and at the same time it doesn't break backwards
> compatibility which is the solution that Kishan is proposing and I'd
> recommend too.
> 
> > Seems a new parameter is needed in createUser to allow both while
> > retaining backwards compat. Perhaps a parameter specifying the type of
> > hash or if the password is plain text that needs to be encrypted. If
> > this parameter is not present, the assumption should be that the
> > password is an MD5 hash, the old behavior.
> >
> > /Ove
> >
> > On 05/16/2013 03:23 PM, Kishan Kavala wrote:
> > >
> > >
> > >>-----Original Message-----
> > >>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> > >>Sent: Thursday, 16 May 2013 6:25 PM
> > >>To: dev@cloudstack.apache.org
> > >>Subject: Re: Review Request: Added PlainTextAuthenticator
> > >>
> > >>On 05/16/2013 02:16 PM, Kishan Kavala wrote:
> > >>>Ove,
> > >>>    Plain text authenticator will allow logging using the hash
> > >>>value. Or else,
> > >>clients sending MD5 hash will fail to login. This is primarily for
> > >>backward compatibility.
> > >>>To avoid logging in using has value itself, plain text
> > >>>authenticator can be
> > >>removed from auth adapter list, provided the client sends plain text
> > >>instead of hash.
> > >>
> > >>I'm not seeing the plain-text authenticator in ACS4.0 list of
> > >>authenticators (components.xml). MD5 and LDAP are listed. Help me
> > >>out, where in ACS4.0 is the code to allow login using the password hash
> itself?
> > >>
> > >>/Ove
> > >
> > >
> > >I checked 4.0 code.  plain-text authenticator is not in components.xml but
> it is part of the code.
> > >
> > >plugins/user-authenticators/plain-text/src/com/cloud/server/auth/Plai
> > >nTextUserAuthenticator.java
> > >
> > >It does MD5 has compare instead of plain text (don't know why), so it
> > >may not serve u'r purpose even after adding it to components.xml
> > >
> > >>
> > >>
> > >>>~kishan
> > >>>
> > >>>>-----Original Message-----
> > >>>>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> > >>>>Sent: Thursday, 16 May 2013 5:33 PM
> > >>>>To: dev@cloudstack.apache.org; Kishan Kavala
> > >>>>Subject: Re: Review Request: Added PlainTextAuthenticator
> > >>>>
> > >>>>Hi Kishan!
> > >>>>
> > >>>>Did you verify that adding the plain text authenticator will not
> > >>>>allow login using the hash value itself?
> > >>>>
> > >>>>
> > >>>>from AccountManagerImpl.java;
> > >>>>    ... getUserAccount ...
> > >>>>    ...
> > >>>>     boolean authenticated = false;
> > >>>>            for(UserAuthenticator authenticator : _userAuthenticators) {
> > >>>>                if (authenticator.authenticate(username, password,
> > >>>>domainId, requestParameters)) {
> > >>>>                    authenticated = true;
> > >>>>                    break;
> > >>>>                }
> > >>>>            }
> > >>>>    ...
> > >>>>
> > >>>>/Ove
> > >>>>
> > >>>>On 05/16/2013 12:39 PM, Kishan Kavala wrote:
> > >>>>>
> > >>>>>-----------------------------------------------------------
> > >>>>>This is an automatically generated e-mail. To reply, visit:
> > >>>>>https://reviews.apache.org/r/11194/
> > >>>>>-----------------------------------------------------------
> > >>>>>
> > >>>>>Review request for cloudstack and Chip Childers.
> > >>>>>
> > >>>>>
> > >>>>>Summary (updated)
> > >>>>>-----------------
> > >>>>>
> > >>>>>Added PlainTextAuthenticator
> > >>>>>
> > >>>>>
> > >>>>>Description (updated)
> > >>>>>-------
> > >>>>>
> > >>>>>Added PlainTextAuthenticator for backward compatibility. Removed
> > >>MD5
> > >>>>auth from PlainTextAuthenticator. It just does plain text compare.
> > >>>>>
> > >>>>>
> > >>>>>This addresses bug CLOUDSTACK-2516.
> > >>>>>
> > >>>>>
> > >>>>>Diffs (updated)
> > >>>>>-----
> > >>>>>
> > >>>>>     client/tomcatconf/applicationContext.xml.in 849c0bc
> > >>>>>     client/tomcatconf/componentContext.xml.in ecd4a11
> > >>>>>     plugins/user-authenticators/plain-
> > >>>>text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> > >>>>52e7cb3
> > >>>>>
> > >>>>>Diff: https://reviews.apache.org/r/11194/diff/
> > >>>>>
> > >>>>>
> > >>>>>Testing (updated)
> > >>>>>-------
> > >>>>>
> > >>>>>Tested login with password sent as both MD5 hash and plaintext
> > >>>>>
> > >>>>>
> > >>>>>Thanks,
> > >>>>>
> > >>>>>Kishan Kavala
> > >>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>>--
> > >>>>Ove Everlid
> > >>>>System Administrator / Architect / SDN & Linux hacker
> > >>>>Mobile: +46706662363
> > >>>>Office: +4618656913 (note EMEA Time Zone)
> > >>
> > >>
> > >>--
> > >>Ove Everlid
> > >>System Administrator / Architect / SDN & Linux hacker
> > >>Mobile: +46706662363
> > >>Office: +4618656913 (note EMEA Time Zone)
> >
> >
> > --
> > Ove Everlid
> > System Administrator / Architect / SDN & Linux hacker
> > Mobile: +46706662363
> > Office: +4618656913 (note EMEA Time Zone)
> 
> --
> Prasanna.,
> 
> ------------------------
> Powered by BigRock.com


Re: Review Request: Added PlainTextAuthenticator

Posted by Chiradeep Vittal <Ch...@citrix.com>.
+1

On 5/16/13 9:20 AM, "Prasanna Santhanam" <ts...@apache.org> wrote:

>On Thu, May 16, 2013 at 04:03:14PM +0200, Ove Ewerlid wrote:
>> I vote -1 for enabling plain text authentication allowing auth
>> directly against hashes. I'm not clear if this functionality exists
>> in ACS4.0, I would assume not.
>> 
>> The API breakage reported was in createUser, where the ability to
>> pass in a hash has value. Think migration scenarios where only the
>> hash is known.
>> 
>> createUser, createAccount and createDomain have, in v41, been
>> enhanced with parameters to allow specifying the UUID directly to
>> accommodate for external provisioning (or migration from older
>> systems). The ability to pass in existing hashes has value in these
>> scenarios. There is also value in being able to pass in a plain text
>> password and have it encrypted depending on how the external account
>> provisioning is done.
>
>Chip, Ove,
>
>There's two parts to this process - the auth and the encode.
>
>In auth - existing tenants of your system send through their passwd
>over the wire that is compared with the password in your cloudstack
>database as follows:
>
>Order of authenticators
>SHA256 > MD5 > PlainText
>
>For a moment assume that Alice (existing user) sends only plaintext
>passwords as she entered in the system when her account was created:
>Her password in the db is say alicesecretsauce and she passes
>alicesecretesauce over-the-wire.
>
>CloudStack will do the following while authenticating Alice:
>1. Is SHA256(alicesecretsauce) == CloudStack_DB(alicesecretesauce)
>2. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce)
>3. Is alicesecretsauce == CloudStack_DB(alicesecretesauce)
>
>In your case since the DB contains the MD5 of alicesecretsauce against
>Alice's account the second comparison returns and authenticates Alice
>successfully after SHA256 fails.
>
>Now let's say you upgrade to 4.1 with the same order of authenticators
>and bug fixed as sent in the patch by Kishan:
>
>Let's look at Alice's case again:
>She sends alicesecretsauce over-the-wire - and the same process
>works out for her and she is able to login.
>
>Now let's say Bob is a new account that is created in your system
>post-upgrade to 4.1:
>
>When Bob creates his account, his password is encoded using the SHA256
>scheme since that's the first one in the configured list. So all new
>accounts now have a SHA256 value in the DB against them.
>
>When Bob attempts to login the first comparison ie
>SHA256(bobsecretsauce) == CloudStack_DB(bobsecretsauce) and he too is
>allowed to login.
>
>Coming to your scenario where you want to hash passwords which are
>coming over-the-wire: The scenario before upgrade should be clear so I
>won't explain it here.
>
>Post-upgrade:
>Alice sends MD5(alicesecretsauce) as per your provisoner-
>
>1. Is SHA256(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce)
>2. Is MD5(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce)
>3. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce)
>
>So she is authenticated using the plaintext authenticator now in 3.
>Without that her auth fails. This is what Kishan is asking that you
>enable.
>
>Bob on the other hand sends in MD5(bobsecretsauce) and his account was
>saved in the DB when your provisioner created his account with
>passwd:  SHA256(MD5(secretsauce)) thereby for him the 1st
>authenticator works and helps him login to cloudstack.
>
>If I were you - I'd migrate everything with the plaintext
>authenticator enabled and then switch over to an auth mechanism that
>suits my security needs and my external provisioner.
>
>For those moving from 2.2.x, 3.0.x, 4.0 to 4.1:
>1. We remove the incorrect auth mechanism and put in the right fix of
>encoding at the server and not doing any UI magic.
>2. We correct the API docs and other docs to indicate the user to send
>in plaintext so clients can adjust to the change.
>3. We describe this migration situation as Ove encountered and how it
>can be corrected without any change using the plaintext authenticator.
>
>I hope that this is fixed right and at the same time it doesn't break
>backwards compatibility which is the solution that Kishan is proposing
>and I'd recommend too.
>
>> Seems a new parameter is needed in createUser
>> to allow both while retaining backwards compat. Perhaps a parameter
>> specifying the type of hash or if the password is plain text that
>> needs to be encrypted. If this parameter is not present, the
>> assumption should be that the password is an MD5 hash, the old
>> behavior.
>> 
>> /Ove
>> 
>> On 05/16/2013 03:23 PM, Kishan Kavala wrote:
>> >
>> >
>> >>-----Original Message-----
>> >>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
>> >>Sent: Thursday, 16 May 2013 6:25 PM
>> >>To: dev@cloudstack.apache.org
>> >>Subject: Re: Review Request: Added PlainTextAuthenticator
>> >>
>> >>On 05/16/2013 02:16 PM, Kishan Kavala wrote:
>> >>>Ove,
>> >>>    Plain text authenticator will allow logging using the hash
>>value. Or else,
>> >>clients sending MD5 hash will fail to login. This is primarily for
>>backward
>> >>compatibility.
>> >>>To avoid logging in using has value itself, plain text authenticator
>>can be
>> >>removed from auth adapter list, provided the client sends plain text
>>instead
>> >>of hash.
>> >>
>> >>I'm not seeing the plain-text authenticator in ACS4.0 list of
>>authenticators
>> >>(components.xml). MD5 and LDAP are listed. Help me out, where in
>>ACS4.0 is
>> >>the code to allow login using the password hash itself?
>> >>
>> >>/Ove
>> >
>> >
>> >I checked 4.0 code.  plain-text authenticator is not in components.xml
>>but it is part of the code.
>> >
>> 
>>>plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTe
>>>xtUserAuthenticator.java
>> >
>> >It does MD5 has compare instead of plain text (don't know why), so it
>>may not serve u'r purpose even after adding it to components.xml
>> >
>> >>
>> >>
>> >>>~kishan
>> >>>
>> >>>>-----Original Message-----
>> >>>>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
>> >>>>Sent: Thursday, 16 May 2013 5:33 PM
>> >>>>To: dev@cloudstack.apache.org; Kishan Kavala
>> >>>>Subject: Re: Review Request: Added PlainTextAuthenticator
>> >>>>
>> >>>>Hi Kishan!
>> >>>>
>> >>>>Did you verify that adding the plain text authenticator will not
>> >>>>allow login using the hash value itself?
>> >>>>
>> >>>>
>> >>>>from AccountManagerImpl.java;
>> >>>>    ... getUserAccount ...
>> >>>>    ...
>> >>>>     boolean authenticated = false;
>> >>>>            for(UserAuthenticator authenticator :
>>_userAuthenticators) {
>> >>>>                if (authenticator.authenticate(username, password,
>> >>>>domainId, requestParameters)) {
>> >>>>                    authenticated = true;
>> >>>>                    break;
>> >>>>                }
>> >>>>            }
>> >>>>    ...
>> >>>>
>> >>>>/Ove
>> >>>>
>> >>>>On 05/16/2013 12:39 PM, Kishan Kavala wrote:
>> >>>>>
>> >>>>>-----------------------------------------------------------
>> >>>>>This is an automatically generated e-mail. To reply, visit:
>> >>>>>https://reviews.apache.org/r/11194/
>> >>>>>-----------------------------------------------------------
>> >>>>>
>> >>>>>Review request for cloudstack and Chip Childers.
>> >>>>>
>> >>>>>
>> >>>>>Summary (updated)
>> >>>>>-----------------
>> >>>>>
>> >>>>>Added PlainTextAuthenticator
>> >>>>>
>> >>>>>
>> >>>>>Description (updated)
>> >>>>>-------
>> >>>>>
>> >>>>>Added PlainTextAuthenticator for backward compatibility. Removed
>> >>MD5
>> >>>>auth from PlainTextAuthenticator. It just does plain text compare.
>> >>>>>
>> >>>>>
>> >>>>>This addresses bug CLOUDSTACK-2516.
>> >>>>>
>> >>>>>
>> >>>>>Diffs (updated)
>> >>>>>-----
>> >>>>>
>> >>>>>     client/tomcatconf/applicationContext.xml.in 849c0bc
>> >>>>>     client/tomcatconf/componentContext.xml.in ecd4a11
>> >>>>>     plugins/user-authenticators/plain-
>> >>>>text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
>> >>>>52e7cb3
>> >>>>>
>> >>>>>Diff: https://reviews.apache.org/r/11194/diff/
>> >>>>>
>> >>>>>
>> >>>>>Testing (updated)
>> >>>>>-------
>> >>>>>
>> >>>>>Tested login with password sent as both MD5 hash and plaintext
>> >>>>>
>> >>>>>
>> >>>>>Thanks,
>> >>>>>
>> >>>>>Kishan Kavala
>> >>>>>
>> >>>>>
>> >>>>
>> >>>>
>> >>>>--
>> >>>>Ove Everlid
>> >>>>System Administrator / Architect / SDN & Linux hacker
>> >>>>Mobile: +46706662363
>> >>>>Office: +4618656913 (note EMEA Time Zone)
>> >>
>> >>
>> >>--
>> >>Ove Everlid
>> >>System Administrator / Architect / SDN & Linux hacker
>> >>Mobile: +46706662363
>> >>Office: +4618656913 (note EMEA Time Zone)
>> 
>> 
>> -- 
>> Ove Everlid
>> System Administrator / Architect / SDN & Linux hacker
>> Mobile: +46706662363
>> Office: +4618656913 (note EMEA Time Zone)
>
>-- 
>Prasanna.,
>
>------------------------
>Powered by BigRock.com
>


Re: Review Request: Added PlainTextAuthenticator

Posted by Prasanna Santhanam <ts...@apache.org>.
On Thu, May 16, 2013 at 04:03:14PM +0200, Ove Ewerlid wrote:
> I vote -1 for enabling plain text authentication allowing auth
> directly against hashes. I'm not clear if this functionality exists
> in ACS4.0, I would assume not.
> 
> The API breakage reported was in createUser, where the ability to
> pass in a hash has value. Think migration scenarios where only the
> hash is known.
> 
> createUser, createAccount and createDomain have, in v41, been
> enhanced with parameters to allow specifying the UUID directly to
> accommodate for external provisioning (or migration from older
> systems). The ability to pass in existing hashes has value in these
> scenarios. There is also value in being able to pass in a plain text
> password and have it encrypted depending on how the external account
> provisioning is done.

Chip, Ove,

There's two parts to this process - the auth and the encode.

In auth - existing tenants of your system send through their passwd
over the wire that is compared with the password in your cloudstack
database as follows:

Order of authenticators
SHA256 > MD5 > PlainText

For a moment assume that Alice (existing user) sends only plaintext
passwords as she entered in the system when her account was created:
Her password in the db is say alicesecretsauce and she passes
alicesecretesauce over-the-wire.

CloudStack will do the following while authenticating Alice:
1. Is SHA256(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 
2. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 
3. Is alicesecretsauce == CloudStack_DB(alicesecretesauce) 

In your case since the DB contains the MD5 of alicesecretsauce against
Alice's account the second comparison returns and authenticates Alice
successfully after SHA256 fails.

Now let's say you upgrade to 4.1 with the same order of authenticators
and bug fixed as sent in the patch by Kishan:

Let's look at Alice's case again:
She sends alicesecretsauce over-the-wire - and the same process
works out for her and she is able to login.

Now let's say Bob is a new account that is created in your system
post-upgrade to 4.1:

When Bob creates his account, his password is encoded using the SHA256
scheme since that's the first one in the configured list. So all new
accounts now have a SHA256 value in the DB against them.

When Bob attempts to login the first comparison ie
SHA256(bobsecretsauce) == CloudStack_DB(bobsecretsauce) and he too is
allowed to login.

Coming to your scenario where you want to hash passwords which are
coming over-the-wire: The scenario before upgrade should be clear so I
won't explain it here.

Post-upgrade:
Alice sends MD5(alicesecretsauce) as per your provisoner-

1. Is SHA256(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 
2. Is MD5(MD5(alicesecretsauce)) == CloudStack_DB(alicesecretesauce) 
3. Is MD5(alicesecretsauce) == CloudStack_DB(alicesecretesauce) 

So she is authenticated using the plaintext authenticator now in 3.
Without that her auth fails. This is what Kishan is asking that you
enable.

Bob on the other hand sends in MD5(bobsecretsauce) and his account was
saved in the DB when your provisioner created his account with
passwd:  SHA256(MD5(secretsauce)) thereby for him the 1st
authenticator works and helps him login to cloudstack.

If I were you - I'd migrate everything with the plaintext
authenticator enabled and then switch over to an auth mechanism that
suits my security needs and my external provisioner.

For those moving from 2.2.x, 3.0.x, 4.0 to 4.1:
1. We remove the incorrect auth mechanism and put in the right fix of
encoding at the server and not doing any UI magic.
2. We correct the API docs and other docs to indicate the user to send
in plaintext so clients can adjust to the change.
3. We describe this migration situation as Ove encountered and how it
can be corrected without any change using the plaintext authenticator.

I hope that this is fixed right and at the same time it doesn't break
backwards compatibility which is the solution that Kishan is proposing
and I'd recommend too.

> Seems a new parameter is needed in createUser
> to allow both while retaining backwards compat. Perhaps a parameter
> specifying the type of hash or if the password is plain text that
> needs to be encrypted. If this parameter is not present, the
> assumption should be that the password is an MD5 hash, the old
> behavior.
> 
> /Ove
> 
> On 05/16/2013 03:23 PM, Kishan Kavala wrote:
> >
> >
> >>-----Original Message-----
> >>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> >>Sent: Thursday, 16 May 2013 6:25 PM
> >>To: dev@cloudstack.apache.org
> >>Subject: Re: Review Request: Added PlainTextAuthenticator
> >>
> >>On 05/16/2013 02:16 PM, Kishan Kavala wrote:
> >>>Ove,
> >>>    Plain text authenticator will allow logging using the hash value. Or else,
> >>clients sending MD5 hash will fail to login. This is primarily for backward
> >>compatibility.
> >>>To avoid logging in using has value itself, plain text authenticator can be
> >>removed from auth adapter list, provided the client sends plain text instead
> >>of hash.
> >>
> >>I'm not seeing the plain-text authenticator in ACS4.0 list of authenticators
> >>(components.xml). MD5 and LDAP are listed. Help me out, where in ACS4.0 is
> >>the code to allow login using the password hash itself?
> >>
> >>/Ove
> >
> >
> >I checked 4.0 code.  plain-text authenticator is not in components.xml but it is part of the code.
> >
> >plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> >
> >It does MD5 has compare instead of plain text (don't know why), so it may not serve u'r purpose even after adding it to components.xml
> >
> >>
> >>
> >>>~kishan
> >>>
> >>>>-----Original Message-----
> >>>>From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> >>>>Sent: Thursday, 16 May 2013 5:33 PM
> >>>>To: dev@cloudstack.apache.org; Kishan Kavala
> >>>>Subject: Re: Review Request: Added PlainTextAuthenticator
> >>>>
> >>>>Hi Kishan!
> >>>>
> >>>>Did you verify that adding the plain text authenticator will not
> >>>>allow login using the hash value itself?
> >>>>
> >>>>
> >>>>from AccountManagerImpl.java;
> >>>>    ... getUserAccount ...
> >>>>    ...
> >>>>     boolean authenticated = false;
> >>>>            for(UserAuthenticator authenticator : _userAuthenticators) {
> >>>>                if (authenticator.authenticate(username, password,
> >>>>domainId, requestParameters)) {
> >>>>                    authenticated = true;
> >>>>                    break;
> >>>>                }
> >>>>            }
> >>>>    ...
> >>>>
> >>>>/Ove
> >>>>
> >>>>On 05/16/2013 12:39 PM, Kishan Kavala wrote:
> >>>>>
> >>>>>-----------------------------------------------------------
> >>>>>This is an automatically generated e-mail. To reply, visit:
> >>>>>https://reviews.apache.org/r/11194/
> >>>>>-----------------------------------------------------------
> >>>>>
> >>>>>Review request for cloudstack and Chip Childers.
> >>>>>
> >>>>>
> >>>>>Summary (updated)
> >>>>>-----------------
> >>>>>
> >>>>>Added PlainTextAuthenticator
> >>>>>
> >>>>>
> >>>>>Description (updated)
> >>>>>-------
> >>>>>
> >>>>>Added PlainTextAuthenticator for backward compatibility. Removed
> >>MD5
> >>>>auth from PlainTextAuthenticator. It just does plain text compare.
> >>>>>
> >>>>>
> >>>>>This addresses bug CLOUDSTACK-2516.
> >>>>>
> >>>>>
> >>>>>Diffs (updated)
> >>>>>-----
> >>>>>
> >>>>>     client/tomcatconf/applicationContext.xml.in 849c0bc
> >>>>>     client/tomcatconf/componentContext.xml.in ecd4a11
> >>>>>     plugins/user-authenticators/plain-
> >>>>text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> >>>>52e7cb3
> >>>>>
> >>>>>Diff: https://reviews.apache.org/r/11194/diff/
> >>>>>
> >>>>>
> >>>>>Testing (updated)
> >>>>>-------
> >>>>>
> >>>>>Tested login with password sent as both MD5 hash and plaintext
> >>>>>
> >>>>>
> >>>>>Thanks,
> >>>>>
> >>>>>Kishan Kavala
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>--
> >>>>Ove Everlid
> >>>>System Administrator / Architect / SDN & Linux hacker
> >>>>Mobile: +46706662363
> >>>>Office: +4618656913 (note EMEA Time Zone)
> >>
> >>
> >>--
> >>Ove Everlid
> >>System Administrator / Architect / SDN & Linux hacker
> >>Mobile: +46706662363
> >>Office: +4618656913 (note EMEA Time Zone)
> 
> 
> -- 
> Ove Everlid
> System Administrator / Architect / SDN & Linux hacker
> Mobile: +46706662363
> Office: +4618656913 (note EMEA Time Zone)

-- 
Prasanna.,

------------------------
Powered by BigRock.com


Re: Review Request: Added PlainTextAuthenticator

Posted by Ove Ewerlid <Ov...@oracle.com>.
I vote -1 for enabling plain text authentication allowing auth directly 
against hashes. I'm not clear if this functionality exists in ACS4.0, I 
would assume not.

The API breakage reported was in createUser, where the ability to pass 
in a hash has value. Think migration scenarios where only the hash is known.

createUser, createAccount and createDomain have, in v41, been enhanced 
with parameters to allow specifying the UUID directly to accommodate for 
external provisioning (or migration from older systems). The ability to 
pass in existing hashes has value in these scenarios. There is also 
value in being able to pass in a plain text password and have it 
encrypted depending on how the external account provisioning is done. 
Seems a new parameter is needed in createUser to allow both while 
retaining backwards compat. Perhaps a parameter specifying the type of 
hash or if the password is plain text that needs to be encrypted. If 
this parameter is not present, the assumption should be that the 
password is an MD5 hash, the old behavior.

/Ove

On 05/16/2013 03:23 PM, Kishan Kavala wrote:
>
>
>> -----Original Message-----
>> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
>> Sent: Thursday, 16 May 2013 6:25 PM
>> To: dev@cloudstack.apache.org
>> Subject: Re: Review Request: Added PlainTextAuthenticator
>>
>> On 05/16/2013 02:16 PM, Kishan Kavala wrote:
>>> Ove,
>>>     Plain text authenticator will allow logging using the hash value. Or else,
>> clients sending MD5 hash will fail to login. This is primarily for backward
>> compatibility.
>>> To avoid logging in using has value itself, plain text authenticator can be
>> removed from auth adapter list, provided the client sends plain text instead
>> of hash.
>>
>> I'm not seeing the plain-text authenticator in ACS4.0 list of authenticators
>> (components.xml). MD5 and LDAP are listed. Help me out, where in ACS4.0 is
>> the code to allow login using the password hash itself?
>>
>> /Ove
>
>
> I checked 4.0 code.  plain-text authenticator is not in components.xml but it is part of the code.
>
> plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
>
> It does MD5 has compare instead of plain text (don't know why), so it may not serve u'r purpose even after adding it to components.xml
>
>>
>>
>>> ~kishan
>>>
>>>> -----Original Message-----
>>>> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
>>>> Sent: Thursday, 16 May 2013 5:33 PM
>>>> To: dev@cloudstack.apache.org; Kishan Kavala
>>>> Subject: Re: Review Request: Added PlainTextAuthenticator
>>>>
>>>> Hi Kishan!
>>>>
>>>> Did you verify that adding the plain text authenticator will not
>>>> allow login using the hash value itself?
>>>>
>>>>
>>>> from AccountManagerImpl.java;
>>>>     ... getUserAccount ...
>>>>     ...
>>>>      boolean authenticated = false;
>>>>             for(UserAuthenticator authenticator : _userAuthenticators) {
>>>>                 if (authenticator.authenticate(username, password,
>>>> domainId, requestParameters)) {
>>>>                     authenticated = true;
>>>>                     break;
>>>>                 }
>>>>             }
>>>>     ...
>>>>
>>>> /Ove
>>>>
>>>> On 05/16/2013 12:39 PM, Kishan Kavala wrote:
>>>>>
>>>>> -----------------------------------------------------------
>>>>> This is an automatically generated e-mail. To reply, visit:
>>>>> https://reviews.apache.org/r/11194/
>>>>> -----------------------------------------------------------
>>>>>
>>>>> Review request for cloudstack and Chip Childers.
>>>>>
>>>>>
>>>>> Summary (updated)
>>>>> -----------------
>>>>>
>>>>> Added PlainTextAuthenticator
>>>>>
>>>>>
>>>>> Description (updated)
>>>>> -------
>>>>>
>>>>> Added PlainTextAuthenticator for backward compatibility. Removed
>> MD5
>>>> auth from PlainTextAuthenticator. It just does plain text compare.
>>>>>
>>>>>
>>>>> This addresses bug CLOUDSTACK-2516.
>>>>>
>>>>>
>>>>> Diffs (updated)
>>>>> -----
>>>>>
>>>>>      client/tomcatconf/applicationContext.xml.in 849c0bc
>>>>>      client/tomcatconf/componentContext.xml.in ecd4a11
>>>>>      plugins/user-authenticators/plain-
>>>> text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
>>>> 52e7cb3
>>>>>
>>>>> Diff: https://reviews.apache.org/r/11194/diff/
>>>>>
>>>>>
>>>>> Testing (updated)
>>>>> -------
>>>>>
>>>>> Tested login with password sent as both MD5 hash and plaintext
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Kishan Kavala
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Ove Everlid
>>>> System Administrator / Architect / SDN & Linux hacker
>>>> Mobile: +46706662363
>>>> Office: +4618656913 (note EMEA Time Zone)
>>
>>
>> --
>> Ove Everlid
>> System Administrator / Architect / SDN & Linux hacker
>> Mobile: +46706662363
>> Office: +4618656913 (note EMEA Time Zone)


-- 
Ove Everlid
System Administrator / Architect / SDN & Linux hacker
Mobile: +46706662363
Office: +4618656913 (note EMEA Time Zone)

RE: Review Request: Added PlainTextAuthenticator

Posted by Kishan Kavala <Ki...@citrix.com>.

> -----Original Message-----
> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> Sent: Thursday, 16 May 2013 6:25 PM
> To: dev@cloudstack.apache.org
> Subject: Re: Review Request: Added PlainTextAuthenticator
> 
> On 05/16/2013 02:16 PM, Kishan Kavala wrote:
> > Ove,
> >    Plain text authenticator will allow logging using the hash value. Or else,
> clients sending MD5 hash will fail to login. This is primarily for backward
> compatibility.
> > To avoid logging in using has value itself, plain text authenticator can be
> removed from auth adapter list, provided the client sends plain text instead
> of hash.
> 
> I'm not seeing the plain-text authenticator in ACS4.0 list of authenticators
> (components.xml). MD5 and LDAP are listed. Help me out, where in ACS4.0 is
> the code to allow login using the password hash itself?
> 
> /Ove


I checked 4.0 code.  plain-text authenticator is not in components.xml but it is part of the code. 

plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java

It does MD5 has compare instead of plain text (don't know why), so it may not serve u'r purpose even after adding it to components.xml

> 
> 
> > ~kishan
> >
> >> -----Original Message-----
> >> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> >> Sent: Thursday, 16 May 2013 5:33 PM
> >> To: dev@cloudstack.apache.org; Kishan Kavala
> >> Subject: Re: Review Request: Added PlainTextAuthenticator
> >>
> >> Hi Kishan!
> >>
> >> Did you verify that adding the plain text authenticator will not
> >> allow login using the hash value itself?
> >>
> >>
> >> from AccountManagerImpl.java;
> >>    ... getUserAccount ...
> >>    ...
> >>     boolean authenticated = false;
> >>            for(UserAuthenticator authenticator : _userAuthenticators) {
> >>                if (authenticator.authenticate(username, password,
> >> domainId, requestParameters)) {
> >>                    authenticated = true;
> >>                    break;
> >>                }
> >>            }
> >>    ...
> >>
> >> /Ove
> >>
> >> On 05/16/2013 12:39 PM, Kishan Kavala wrote:
> >>>
> >>> -----------------------------------------------------------
> >>> This is an automatically generated e-mail. To reply, visit:
> >>> https://reviews.apache.org/r/11194/
> >>> -----------------------------------------------------------
> >>>
> >>> Review request for cloudstack and Chip Childers.
> >>>
> >>>
> >>> Summary (updated)
> >>> -----------------
> >>>
> >>> Added PlainTextAuthenticator
> >>>
> >>>
> >>> Description (updated)
> >>> -------
> >>>
> >>> Added PlainTextAuthenticator for backward compatibility. Removed
> MD5
> >> auth from PlainTextAuthenticator. It just does plain text compare.
> >>>
> >>>
> >>> This addresses bug CLOUDSTACK-2516.
> >>>
> >>>
> >>> Diffs (updated)
> >>> -----
> >>>
> >>>     client/tomcatconf/applicationContext.xml.in 849c0bc
> >>>     client/tomcatconf/componentContext.xml.in ecd4a11
> >>>     plugins/user-authenticators/plain-
> >> text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java
> >> 52e7cb3
> >>>
> >>> Diff: https://reviews.apache.org/r/11194/diff/
> >>>
> >>>
> >>> Testing (updated)
> >>> -------
> >>>
> >>> Tested login with password sent as both MD5 hash and plaintext
> >>>
> >>>
> >>> Thanks,
> >>>
> >>> Kishan Kavala
> >>>
> >>>
> >>
> >>
> >> --
> >> Ove Everlid
> >> System Administrator / Architect / SDN & Linux hacker
> >> Mobile: +46706662363
> >> Office: +4618656913 (note EMEA Time Zone)
> 
> 
> --
> Ove Everlid
> System Administrator / Architect / SDN & Linux hacker
> Mobile: +46706662363
> Office: +4618656913 (note EMEA Time Zone)

Re: Review Request: Added PlainTextAuthenticator

Posted by Ove Ewerlid <Ov...@oracle.com>.
On 05/16/2013 02:16 PM, Kishan Kavala wrote:
> Ove,
>    Plain text authenticator will allow logging using the hash value. Or else, clients sending MD5 hash will fail to login. This is primarily for backward compatibility.
> To avoid logging in using has value itself, plain text authenticator can be removed from auth adapter list, provided the client sends plain text instead of hash.

I'm not seeing the plain-text authenticator in ACS4.0 list of 
authenticators (components.xml). MD5 and LDAP are listed. Help me out, 
where in ACS4.0 is the code to allow login using the password hash itself?

/Ove


> ~kishan
>
>> -----Original Message-----
>> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
>> Sent: Thursday, 16 May 2013 5:33 PM
>> To: dev@cloudstack.apache.org; Kishan Kavala
>> Subject: Re: Review Request: Added PlainTextAuthenticator
>>
>> Hi Kishan!
>>
>> Did you verify that adding the plain text authenticator will not allow login
>> using the hash value itself?
>>
>>
>> from AccountManagerImpl.java;
>>    ... getUserAccount ...
>>    ...
>>     boolean authenticated = false;
>>            for(UserAuthenticator authenticator : _userAuthenticators) {
>>                if (authenticator.authenticate(username, password,
>> domainId, requestParameters)) {
>>                    authenticated = true;
>>                    break;
>>                }
>>            }
>>    ...
>>
>> /Ove
>>
>> On 05/16/2013 12:39 PM, Kishan Kavala wrote:
>>>
>>> -----------------------------------------------------------
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/11194/
>>> -----------------------------------------------------------
>>>
>>> Review request for cloudstack and Chip Childers.
>>>
>>>
>>> Summary (updated)
>>> -----------------
>>>
>>> Added PlainTextAuthenticator
>>>
>>>
>>> Description (updated)
>>> -------
>>>
>>> Added PlainTextAuthenticator for backward compatibility. Removed MD5
>> auth from PlainTextAuthenticator. It just does plain text compare.
>>>
>>>
>>> This addresses bug CLOUDSTACK-2516.
>>>
>>>
>>> Diffs (updated)
>>> -----
>>>
>>>     client/tomcatconf/applicationContext.xml.in 849c0bc
>>>     client/tomcatconf/componentContext.xml.in ecd4a11
>>>     plugins/user-authenticators/plain-
>> text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3
>>>
>>> Diff: https://reviews.apache.org/r/11194/diff/
>>>
>>>
>>> Testing (updated)
>>> -------
>>>
>>> Tested login with password sent as both MD5 hash and plaintext
>>>
>>>
>>> Thanks,
>>>
>>> Kishan Kavala
>>>
>>>
>>
>>
>> --
>> Ove Everlid
>> System Administrator / Architect / SDN & Linux hacker
>> Mobile: +46706662363
>> Office: +4618656913 (note EMEA Time Zone)


-- 
Ove Everlid
System Administrator / Architect / SDN & Linux hacker
Mobile: +46706662363
Office: +4618656913 (note EMEA Time Zone)

RE: Review Request: Added PlainTextAuthenticator

Posted by Kishan Kavala <Ki...@citrix.com>.
Ove,
  Plain text authenticator will allow logging using the hash value. Or else, clients sending MD5 hash will fail to login. This is primarily for backward compatibility.
To avoid logging in using has value itself, plain text authenticator can be removed from auth adapter list, provided the client sends plain text instead of hash.

~kishan

> -----Original Message-----
> From: Ove Ewerlid [mailto:Ove.Ewerlid@oracle.com]
> Sent: Thursday, 16 May 2013 5:33 PM
> To: dev@cloudstack.apache.org; Kishan Kavala
> Subject: Re: Review Request: Added PlainTextAuthenticator
> 
> Hi Kishan!
> 
> Did you verify that adding the plain text authenticator will not allow login
> using the hash value itself?
> 
> 
> from AccountManagerImpl.java;
>   ... getUserAccount ...
>   ...
>    boolean authenticated = false;
>           for(UserAuthenticator authenticator : _userAuthenticators) {
>               if (authenticator.authenticate(username, password,
> domainId, requestParameters)) {
>                   authenticated = true;
>                   break;
>               }
>           }
>   ...
> 
> /Ove
> 
> On 05/16/2013 12:39 PM, Kishan Kavala wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/11194/
> > -----------------------------------------------------------
> >
> > Review request for cloudstack and Chip Childers.
> >
> >
> > Summary (updated)
> > -----------------
> >
> > Added PlainTextAuthenticator
> >
> >
> > Description (updated)
> > -------
> >
> > Added PlainTextAuthenticator for backward compatibility. Removed MD5
> auth from PlainTextAuthenticator. It just does plain text compare.
> >
> >
> > This addresses bug CLOUDSTACK-2516.
> >
> >
> > Diffs (updated)
> > -----
> >
> >    client/tomcatconf/applicationContext.xml.in 849c0bc
> >    client/tomcatconf/componentContext.xml.in ecd4a11
> >    plugins/user-authenticators/plain-
> text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3
> >
> > Diff: https://reviews.apache.org/r/11194/diff/
> >
> >
> > Testing (updated)
> > -------
> >
> > Tested login with password sent as both MD5 hash and plaintext
> >
> >
> > Thanks,
> >
> > Kishan Kavala
> >
> >
> 
> 
> --
> Ove Everlid
> System Administrator / Architect / SDN & Linux hacker
> Mobile: +46706662363
> Office: +4618656913 (note EMEA Time Zone)

Re: Review Request: Added PlainTextAuthenticator

Posted by Ove Ewerlid <Ov...@oracle.com>.
Hi Kishan!

Did you verify that adding the plain text authenticator will not allow 
login using the hash value itself?


from AccountManagerImpl.java;
  ... getUserAccount ...
  ...
   boolean authenticated = false;
          for(UserAuthenticator authenticator : _userAuthenticators) {
              if (authenticator.authenticate(username, password, 
domainId, requestParameters)) {
                  authenticated = true;
                  break;
              }
          }
  ...

/Ove

On 05/16/2013 12:39 PM, Kishan Kavala wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11194/
> -----------------------------------------------------------
>
> Review request for cloudstack and Chip Childers.
>
>
> Summary (updated)
> -----------------
>
> Added PlainTextAuthenticator
>
>
> Description (updated)
> -------
>
> Added PlainTextAuthenticator for backward compatibility. Removed MD5 auth from PlainTextAuthenticator. It just does plain text compare.
>
>
> This addresses bug CLOUDSTACK-2516.
>
>
> Diffs (updated)
> -----
>
>    client/tomcatconf/applicationContext.xml.in 849c0bc
>    client/tomcatconf/componentContext.xml.in ecd4a11
>    plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3
>
> Diff: https://reviews.apache.org/r/11194/diff/
>
>
> Testing (updated)
> -------
>
> Tested login with password sent as both MD5 hash and plaintext
>
>
> Thanks,
>
> Kishan Kavala
>
>


-- 
Ove Everlid
System Administrator / Architect / SDN & Linux hacker
Mobile: +46706662363
Office: +4618656913 (note EMEA Time Zone)

Re: Review Request: Added PlainTextAuthenticator

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/11194/#review20655
-----------------------------------------------------------

Ship it!


Ship It!

- Chip Childers


On May 16, 2013, 10:39 a.m., Kishan Kavala wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11194/
> -----------------------------------------------------------
> 
> (Updated May 16, 2013, 10:39 a.m.)
> 
> 
> Review request for cloudstack and Chip Childers.
> 
> 
> Description
> -------
> 
> Added PlainTextAuthenticator for backward compatibility. Removed MD5 auth from PlainTextAuthenticator. It just does plain text compare.
> 
> 
> This addresses bug CLOUDSTACK-2516.
> 
> 
> Diffs
> -----
> 
>   client/tomcatconf/applicationContext.xml.in 849c0bc 
>   client/tomcatconf/componentContext.xml.in ecd4a11 
>   plugins/user-authenticators/plain-text/src/com/cloud/server/auth/PlainTextUserAuthenticator.java 52e7cb3 
> 
> Diff: https://reviews.apache.org/r/11194/diff/
> 
> 
> Testing
> -------
> 
> Tested login with password sent as both MD5 hash and plaintext
> 
> 
> Thanks,
> 
> Kishan Kavala
> 
>