SP v4 alpha2 sessions ?

[Scott Cantor]

> Thanks, is hub "stickiness" needed during login (between agent calls to
> hub session-initiator and token-consumer) (similar to IdP where
> login/flow needs to go to same server)

All calls are 100% stateless apart from some caching of agent authentication when possible.

But if RelayState/OIDC state tracking is handled with storage instead of cookies, and that storage bean is shibboleth.StorageService, then it's going to be in memory and that would require stickiness but is more accurately meant as "can't be clustered". It's for localhost use akin to shibd today. I don't believe I set that as the default, I think cookies are used.

The problem is that cookies break all sorts of broken clients. Another case popped up recently with something issuing the AuthnRequest out of one process and only afterwards launching a real browser to handle the response, like Office links do/did. The cookie doesn't get set properly so the response won't be accepted.

So in practice, people who cluster are going to need something for state or will need stickiness or have to stop accepting broken clients as a fact of life. I don't have a ton of confidence they will grow a spine regarding those.

All of that is pretty much the same as now, though the old SP's correlation logic only supported cookies, and now it's part of a larger design with RelayState that does support storage persistence, so it's at least unified now and more flexible.

> Does hub get called on local logout(localOnly = true) or only with
> SAML/SLO logout ?

No.

> There could be a logic error in
> attribute/impl/DefaultAttributeConfiguration.cpp:L363-365

Yep, thanks.

> session/impl/FilesystemSessionCache.cpp:L213:
> if (f > 0) {
> 0 is valid filedescriptor, so this should be if (f >= 0) ?
> 

Yep, thanks.

> I think there's TOCTOU race in
> session/impl/FilesystemSessionCache.cpp:L211-220 and L422-448
> session file is created with open O_CREAT | O_EXCL and then fd is
> closed and then reopended with ofstream os(path);

There shouldn't be unless open() doesn't in fact create the file, but it's possible it doesn't, I'll have to add a unit test for that in isolation to check. The intent is that once the file exists, that isolates that sequence of code because anything else will be gated by the O_CREAT|O_EXCL flags and fail trying to create a duplicate. Nothing else is meant to know about that session key yet, so it can't be in use in another thread in a legitimate way.

> Using (gnu extension) something like:

I can't use gnu extensions. I'll see if there's any other way to adopt the handle, but it shouldn't be necessary unless open doesn't do what it's meant to. If I have to I can write something to the file to ensure it's actually created in while still open.

> and removing close(f) after open() could fix the open/use race.

It's not a race as long as the file actually exists after open() or unless the flags don't do what they are documented to do and prevent a second call to open the same filename. The name itself is the mutex, it's trying to reserve the name.

All of this assumes atomic open calls. If they eventually prove not be atomic, I will abandon the pretense that the file system is a viable session store.

> For utime (L397,L508,L609) lutimes shouldn't follow symlink.

Not sure what you mean.

> Looks like cookie value could be used as is for filesystem ops:
> shibsp/session/impl/AbstractSessionCache.cpp:L425-427 and L691-695
> pair<string,unsigned int> keyver = parseCookieValue(cookieval); could
> check that keyver.first is only hexdigits(isxdigit) and correct/sane
> length.

The directory itself is not controllable from the cookie, but you're right that it needs some sanity checking. I thought I had scanned the string for certain cases but I'll review it. I don't, however, think it can be made truly secure in the face of enough creativity.

To be clear...I wouldn't use the filesystem in a million years. It isn't going to be the default once this ships, I'll flip it to storage or to a cookie-based mechanism as a default. The risks are too difficult to mitigate.

The problem is that I don't believe any server side state options are reliable, and I know cookies have size limits. So I don't actually think there is a reliable clustering solution, I never really have.

-- Scott