assertion not always signed
Cantor, Scott
cantor.2 at osu.edu
Tue Jan 17 16:53:18 EST 2017
> Update to 3.3 you mean?
Yes, and that is not optional anyway.
> I think the explanation in IDP-1108 (actually IDP-920) is consistent
> with what we've seen. So it breaks not based on "time that the IdP
> session was alive," but "whether we have previously had a short-lived
> session with this SP during the same long-lived IdP session."
Same NameID to an SP for a given user within a session more than once. I have no doubt that's the issue, I just forgot about it until now. Even the description didn't jog my memory until I saw the log.
> The quick and dirty 3.3 ./install.sh seems to have worked after a)
> removing the redundant DuoWeb-*.jar and b) restoring the pre-3.3
> behavior of leaving credentials in RAM for a while. Is there a quick
> fix to replace usernamePasswordContext.getUsername() with a call to
> get the principal elsewhere?
Well, the flag is there to give you compatible behavior, but nobody should be relying on that context in another flow, it's working state for the Password flow itself, not part of its output contract. That is the main problem here.
> Longer-term fix is to switch to the new
> 3.3 MFA framework, which doesn't look hard, so it's likely that'll be
> the short-term fix after all.
That is certainly not a precondition to updating. Until 4.0 there's nothing that shouldn't keep working as long as it wasn't using undocumented or unsupported interfaces. But that context being used by *another* flow? No, that's not right. The flag was put there to make sure people using the context *inside* the Password flow via some kind of customization could continue to do that.
Anything that needs to survive the flow has to be handled in other ways, and the username survives in several places. The supplied Duo implementation relies on a function to obtain the username and gets it from one of two supported places. It is essential that the value used is post-c14n. Pulling it raw means you're getting whatever the user entered, and there are many deployments where that's not what you want.
> Here's where the legacy Duo (not Unicode) chokes "out of the box."
Right, that's inappropriate. It's a "feature" that it broke, essentially, just a canary flagging an incorrect implementation behavior. But it can be made to work with the option changed for the time being, so I'm not terribly angsty about it hurting anybody.
These aren't criticisms. I didn't even have documentation on the extension contract before close to 3.3 shipping, and even now I doubt it's sufficient.
-- Scott
More information about the users
mailing list