sso_multi_domain_encrypt_string doesn't actually encrypt, it just base 64 encodes the data after adding some info before/after. That is not near how encryption works.

The result is that any anonymous user can visit sso_multi_domain/fetch_login_details/ and pass in hashes to find the username, email, password and other private data of all logged in users.

This is a critical bug and should be fixed before a stable release is created. There may not be a true "fix" for this and perhaps the approach has to be abandoned. I also encourage you to add a link to this issue on the project page until this issue is fixed.

Comments

varunmishra2006’s picture

varunmishra2006 commented

The hash is value is random and no one can predict. The hash table is also cleared after every 1 hour.
Can you please let me know a test case scenario to fetch login details?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I don't think the module is as secure as you do.

1. The hash value is not random. It is easy to predict. It is the md5(time()), time is integers in seconds. Which means that trying all the hashes for the last hour requires 60*60 requests to a server. 3,600 is not a lot of requests.

2. Even if the hash were based on a cryptographically appropriate source of randomness, 32 characters of a-z0-9 is not a ton of entropy to hide this kind of information.

heine’s picture

heine commented

varumishra2006, can you please follow up on this issue and #2361503: Security issue: Create users with chosen roles.? I believe they are rather serious and could warrant some kind of warning on the project page.

varunmishra2006’s picture

varunmishra2006 commented

@Heine
I can sort out this security rist by doing following things:-
1) In the admin section we will provide a text field for the unique secret key.
2) After that when I will generate the hash, it will be combination of secret key and current time.

This is way no can predict the hash value.

Let me know if that sounds ok?

@greggles

The only reason to create this module is to provide an easier way to achieve single sign on using for a non-technical person.

I think we can work out to make it more secure as well as easier.

mc0e’s picture

mc0e commented

@varumishra2006 you don't know anything near enough about cryptography to be designing your own algorithms. It's not enough just to come up with an approach that you don't know how to break.

Fortunately there's excellent encryption libraries available, written by peoplewho know more about this than most of us could hope to learn in a lifetime of study. Use the existing stuff.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Why is there still not a warning and links on the project page to this issue and to #2361503?

Currently the project page just says "This module is very secured and uses encryption." but neither of those statements is true. The module is definitely *not* secure. Base64 is encoding, not encryption.

varunmishra2006’s picture

varunmishra2006 commented

@greggles

Security warning and link to issue added on project page.

drupalninja99’s picture

drupalninja99 commented

@greggles do you have a recommendation for what to do?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

I would probably use https://www.drupal.org/project/real_aes to do the encryption.

But I'm not sure whether the approach of this single-sign-on-multiple-domain module makes sense at all, even if the encryption functions did true encryption. I suggest looking for other solutions like CAS instead.

varunmishra2006’s picture

varunmishra2006 commented

@greggles

I am not able to provide proper time to fix its security issues. Therefore please suggest whether I should allow co-maintainership for this project or discontinue it for downloading.

please suggest.

dddave’s picture

dddave commented

@varunmishra2006

Please add a co-maintainer: #2541550: Offering to maintain or take over sso_multi_domain

drupalninja99’s picture

drupalninja99 commented

@greggles I am not sure I follow. I think this approach is good if we have a secure enough method of authenticating. If we added the encrypt module as a dependency and mentioned that you should use SSL or a VPN would that not be good enough? How many Drupal sites out there don't have SSL even on their user login page?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

The hash for accessing the data in the db is not sufficiently random. I also think that using username/password as the way to authenticate on a lot of sites doesn't make sense.

Bakery has ways for the master and slave to establish trust using a shared secret. Once that's done, the master just asserts "this person is username foo, email foo@example.com, and uid 10 on the master site" - there's no reason to pass around password as is done in this module. It just increases the risk.

But even bakery has had several security issues over the years and it had a lot of security-focused developers working on it from the very beginning. For example, the encryption method used since the very beginning of bakery had a bug in it - https://heine.familiedeelstra.com/bakery-sso-from-bug-to-exploit took a long time before anyone noticed.

That's why I suggest looking at a Drupal module that supports a more broadly used SSO solution like CAS. Drupal-invented solutions have a history of basic bugs related to encryption or CSPRNG.

mc0e’s picture

mc0e commented

Is it time to withdraw this project?

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

The community policy is generally not to withdraw projects from drupal.org. There probably is a secure way to achieve the goal of this module, so the project space can stay alive. If a module is unsupported for security reasons, the releases are removed, but the code still stays available in git (so it can be fixed) and the project page stays available.