search

OpenIDC / mod_auth_openidc

OpenID Certified™ OpenID Connect Relying Party implementation for Apache HTTP Server 2.x
Apache License 2.0
993 stars 326 forks source link

Shared Session/Session Conflict issue #431

Closed amitnarang28 closed 5 years ago

amitnarang28 commented 5 years ago
Environment
Expected behaviour

During High load openin module should generate different session ID for different request and maintain correct headers in SHM Cache.

Actual behaviour

On very high load where login is like 400+ user login/second session id generated for two different users are same. If we run load test for many (5-10) hours I can see 1-2 occurrence of this happening.

Minimized example

During debug logs for two different request same session id was generated.

-User2 [Fri Mar 01 15:20:01 2019] [debug] [pid 83207] src/cache/common.c(628): [client IP:40775] oidc_cache_set: enter: 64746bf8-3c5f-11e9-a559-5f2a1f420858 (section=s, len=1697, encrypt=0, ttl(s)=1199, type=shm)

Configuration and Apache server log files

Let me know what config's are necessary.

I tried downloading code and analyzed how the mod_auth_oidc_session cookie is being created. I saw the session id is nothing but the value of apr_uuid_get function from apache util. As per the uuid implementation it can have conflicts in a very close time stamp, which is what my guess is. Is there any other way this session cookie be created so that it makes sure there for any reason it has no conflict.

Thanks

zandbelt commented 5 years ago

interesting, I would not expect such relatively low numbers to collide; I'm not sure what to make of this:

apr_uuid_get uses a system provided underlying random generator, e.g. on LInux it uses /dev/urandom; it may be that your random generator does not have enough entropy; are you perhaps running in a VM or container?

amitnarang28 commented 5 years ago

Thanks Zanbelt for your thought on it. To answer your question, yes we are on VM and the point is in the cloud world, I think most of us using openid might be using VM's.

Now my point is if this algorithm can even have 1 conflict, to have such an algorithm is not safe to use as session value. Since there can be many sensitive products trying to use this module and even 1 such conflict can give nightmares. Any such session sharing switches user identity to someone else and take whole system as that.

Is there any option we can update this module to have some configurable parameter so that session include something more than just random generated string, like sub and re encrypt with some algorithm to become a session value.

Let me know your thoughts or if you have seen in past any way to make sure UUID generation is really unique at least within some hour's.

Thanks

zandbelt commented 5 years ago

you're asking the impossible: your system does not have a more random generator than /dev/urandom and ultimately mod_auth_openidc runs on your system...

you probably need to change your system setup to something that has more entropy

amitnarang28 commented 5 years ago

Hans, appreciate your quick response. Yes, I am looking into what can be done from system level.

But is it a wise choice to use just a random key generator which can have conflicts to use as session identifier for a web security module!!!

I believe, it must have some kind of additional mechanism to make sure it doesn't have conflicts.

zandbelt commented 5 years ago

please suggest a mechanism to create more randomness than /dev/urandom can do on the same system... (and we'll both be millionaires)

amitnarang28 commented 5 years ago

I definitely want to be millionaire, just currently my focus is not there though!!!

I understand we can't get more randomness in system but with current solution we have chances of conflict as documented by UUID algorithms.

As suggested earlier in my opinion this is what we can do:

Get the same random_string and append or prepend it with userid/sub and encrypt with some SHA algorithm. Field to prepend/append can be some of config so if different open ID systems give username in different fields it can be controlled.

Pros of doing same: 1.This way for two users even if we have got same UUID they will differ from each other and will be unique session id. Even if same user is trying to get 2 session for same application within those nano seconds he will still see same application and no authenticity of application get's screwed with it.

  1. We still have something used as session which can't be easily mimicked as it is more than 128 bits.

Cons: We are attaching some identifiable information in session cookie if someone know how to decrypt it might see whose session is it!! But does it matter as if someone has session cookie of someone they must be able to replay some request on server with that cookie that can give this information anyhow. Session cookie might be some bytes long, but for the matter of security I am sure no one in world will mind that!!!.

Let me know your thoughts.

zandbelt commented 5 years ago

Really the problem is in your system or in your measurements.

You should fix the system entropy otherwise you'll be in trouble with other random values such as nonce's, state values, jti etc.; the 64 bit uuid is good enough in itself.

The con you mention is definitely a no-go.

amitnarang28 commented 5 years ago

Hans Cons which is wrote is choice of implementer to see what security is important for. Shared session even if it is one user is not worth taking risk!!!

Instead of taking it as positive feedback to improve module, you are just putting it on server.

How do you know only my one server is bad where as I have myself replicated on 3 different servers with different CPU/RAM and just same OS flavor and still this issue stands true.

Will hope that someday, you feel this as useful thread and you reopen this case back and get a solution.

Thanks

zandbelt commented 5 years ago

I do want to thank you for your contribution, this is useful indeed and I guess I should be more verbose about why I'm closing this:

Firstly I don't think you fully grasp what random number generation means, please have a look at something like Coursera's Cryptography I or similar.

Secondly, your suggestion would push the problem down a little bit as clashes between user id's and sub values across different providers may easily occur.

Thirdy, on a true random system this error would occur 1 out of 2^64 times. In your case it happens after 400 3600 10 times on average which is about 1281023894007 times faster than expected... So you must agree with me that either

  1. apr_uuid_get is terribly broken in itself
  2. your systems's /dev/urandom is not generating enough entropy (since that's being used underneath)
  3. your measurements are off

I hope you see why I don't see a problem with this module. Also: as you point out this module is in use by many many enterprise users and they've done much more extensive experiments that what you've done so far, all with good results.

I actually expect 3 over 2.. If you still expect 1. you can take this discussion to the libapr project but all projects using apr_uuid_get would horribly be broken if this were true.

amitnarang28 commented 5 years ago

Hans, thanks for your insight.

I am very stubborn student and when it comes to learning/solving issue I do my best to do it. Short story is, yes this is happening. I am part of enterprise where I am getting this issue so that means one of enterprise is seeing this.

Measurements would have been wrong if this has happened once and I am assuming it, so it is not. I have logs from apache where two different request from front end while creating session have got same key.

Now I am taking few action items for me:

  1. Why we are seeing this noise on only our servers (so far I have got on three different servers which are sized at 8x24 or 6x32) and why entropy is less for our flavor of linux/system?
  2. How other products use this random value, do they use it as is or it become salt for some algorithm with some more mix which is what I was asking.
  3. What UUID algorithm apr_uuid_get uses as there are many way's these things can be generated and few have more conflicts than others. ref: https://en.wikipedia.org/wiki/Universally_unique_identifier Section Collision is something which is interesting

I understand for you this issue is invalid, but I will definitely come back with more evidences and reasons. It might be our apache setup or something is wrong with our server and it might be learning for others to know if they ever see it.

Thanks again and I really appreciate your insights.

amitnarang28 commented 5 years ago

Hi Hans

I know it has been over a month with any update from me on this case. Here is what I have found so far based on limited time and availability.

I looked at code for apr module and found there is routine behind apr_uuid_get, where it tries to get entroy from system using apr_os_uuid_get or using some custom routine which is timestamp based. At this time I think it is somehow not even using system entropy rather an fallback apache function.

Our web team used to install every thing from source, so we thought of installing one Apache/oidc using source and one server we installed using RPM and binary which I think most of world might be doing.

We ran same test using these two servers and we found that Apache/OIDC install from source is having similar issue where as apache installed using binary didn't had this issue.

Next step, we did is comparing libraries for each executable using ldd command and found that one library which is not included while compiling oidc/apache module from source is lib_uuid and currently our web team is trying to find any documentation to link this while compiling and run more test.

Hans, do you have any special compilation parameters or configuration which you use so that rpm have linkage to /lib64/libuuid.so?

I will keep you posted with what exact issue we found with installation as this might help other's understand that there can be issue like that. Just a thought!! can there be warning while installing OIDC module if system finds that it has no libuuid module linked while using in Apache etc?

zandbelt commented 2 years ago

just to close the loop on this: it is caused by custom built libapr libraries that are not compiled against libuuid; I don't think mod_auth_openidc should cater for all custom built library versions, but to avoid this specific (simple) case l I'll change the code to no longer rely on apr_uuid_get but use apr_generate_random_bytes instead