cyrusimap / cyrus-sasl

Other
132 stars 150 forks source link

realms in digest-md5 should be simplified #49

Closed brong closed 23 years ago

brong commented 23 years ago

From: Larry Greenfield Bugzilla-Id: 504 Version: 2.0 Owner: Rob Siemborski

brong commented 23 years ago

From: Larry Greenfield

currently, we support sending different realms with DIGEST-MD5. commercial implementations of DIGEST-MD5 always send out the same (arbitrary) realm, and then accept usernames of the form user@realm.

making this change will simplify how realms work vs. CRAM-MD5 or PLAIN and doesn't seem to lose much (if any) functionality.

?

brong commented 23 years ago

From: Rob Siemborski

It's unclear to me how much this "simplifies"

The default currently is for the server to fallback to serverFQDN if user_realm is not provided, other than that, I can't see many changes that would take place. As it is, we can accept usernames in the form of foo@bar.com

brong commented 23 years ago

From: Larry Greenfield

the complication is that specifying "user@realm" as your username works in CRAM and PLAIN but not DIGEST. in DIGEST, you specify "user" and then set the realm to "realm". this means that the client UI has to be designed differently depending on whether you're authenticating with CRAM or DIGEST.

since using the realm parameter doesn't add any security, it's easier to just make it a constant and have the user use "user@realm" as their username

brong commented 23 years ago

From: Rob Siemborski

Okay. I guess all that really required was pulling out the getrealm callback requirement. I can now do authentications using foo@bar.com to authenticate to the realm bar.com (just like in plain) so I'll leave it at that.

However, I don't think disableing the use of the callback entirely is necessary.

brong commented 23 years ago

From: Larry Greenfield

whoa, this bug is meant to only apply to the server-side of the DIGEST-MD5 code. the client-side should be unaffected or perhaps the callback should be made optional.

the important changes are in the server side. has this work been done?

brong commented 23 years ago

From: Rob Siemborski

The current behavior on the server side is that the serverFQDN or user-supplied realm is passed as the "realm" token to the remote. The password lookup is done against the canonified username, which will either be unqualfied (serverFQDN as the realm) or be in the user@realm syntax. The sasldb auxprop plugin, atleast, handles all this correctly for every plugin that uses it.

What I am unsure of is what happens if the client passes back a different realm in the realm parameter than the one that is supplied to it, though I am pretty sure this works too. I'll have to verify on monday.

What most likely happened is that this situation went away in some of the rewrites (most likely the one that made digest use plaintext passwords). I'll recheck that this is doing the right thing on monday.

brong commented 23 years ago

From: Rob Siemborski

I believe the behavior is currently correct, with the possible exception that we (as in SASLv1) reject a client-supplied realm if we did not offer it, which seems to be allowed by the RFC but doesn't make much sense.

brong commented 23 years ago

From: Larry Greenfield

i don't think this is fixed. the get_realm() function is still doing wacky things seeing if user_realm is set to "" or "foo" or "bar".

i think if get_realm() always returns params->serverFQDN things would be better.