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

Reply via email to