Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication security, or lack thereof. #384

Closed
tekacs opened this issue Dec 12, 2010 · 5 comments
Closed

Authentication security, or lack thereof. #384

tekacs opened this issue Dec 12, 2010 · 5 comments

Comments

@tekacs
Copy link

tekacs commented Dec 12, 2010

Right. Having sent the message below a while back, I thought I'd post it here and hope for a response... It may transpire that this is just a blown up issue and Padrino's admin isn't supposed to be even remotely secure (or users are supposed to replace the security with their own), but I just thought I'd ask, since people out there do appear to be using the 'security' of Padrino out of the box...

For that matter, it's based upon this line:

app.set :session_id, "padrino#{File.basename(Padrino.root)}_#{app.app_name}".to_sym

in "padrino-admin/lib/padrino-admin/access_control.rb"

Thanks,
TechKid

Hi,

I just thought I would e-mail to suggest that the authentication method used by Padrino be improved as in its current state, it doesn't really provide any security, at all. Certainly, anyone who knows a Padrino instance's root directory seems to be able to bypass authentication completely (something I noticed experimenting on a Padrino-based project of mine). Is this intentionally simple (similarly to your use of crypted passwords rather than hashes for accounts?) - if so, please do at least point this out to users on the home page or similar so that people are not falsely led to believe that Padrino's authentication system is in any way secure.

For reference, the following cookie (as rack.session) should be enough to bypass authentication on virtually any instance of Padrino running in the root directory of Heroku out there:

BAh7EzocX3BhZHJpbm9fd2Vic2l0ZXNfYWRtaW5pBjocX3BhZHJpbm9faHR0

(which unmarshals to {:FLASH=>{}, :_padrino_mnt_admin=>1} where 'mnt' is Padrino's root directory)

I chose not to post this to issues 'just in case', though I hope that you have mentioned the pseudo-security of Padrino before and this isn't the first time that this has been brought to your attention.

I await your response.

Thanks,
TechKid

@tekacs
Copy link
Author

tekacs commented Dec 12, 2010

I should point out that the above assumes a few things - firstly, that the admin app is indeed mounted under the name 'admin', though this is the default and most users are unlikely to change this.
Similarly, it assumes that user 1 is an admin user, something which is the default and is to be expected for most installations, but if not, it isn't difficult to try different IDs since it just involves changing this cookie ever so slightly...

@nesquena
Copy link
Member

Thanks for bringing this up techkid. It's a valid concern and I appreciate the post.

I think it's clear padrino-admin is not a particularly secure solution to the authentication / persmissions space right now. It is my intention that this changes in the future and that it becomes more secure (one-way hashed passwords, etc). In the time being, I tend towards recommending people to use padrino-admin only if they are aware of these issues and the security implications.

Otherwise, it is certainly possible to use Padrino for other reasons and then use padrino_warden or another authentication solution on top. If anyone reads this and wants to help us make padrino-admin a more secure solution, that would be greatly appreciated by the team. These issues will be addressed though and I appreciate this ticket as a reminder of one of the issues.

@tekacs
Copy link
Author

tekacs commented Dec 12, 2010

Thanks for your response. It's great to hear that this has been thought about. The only request I'd have would be for a small warning in the main README for the unaware...

While I reckon that there are probably others who would make a better job of this than me, I'd be happy to have a go at securing up padrino-admin some more. Though your own ideas for the security of the admin section might differ from my own, I'll have a go at getting a more secure solution together on my own fork when I have time in the next few days and, I'll keep it around and updated just in case...

@nesquena
Copy link
Member

Thanks again for taking the time to post. And believe me I do hear you about making the security risks in the admin section of the framework. This work would benefit everybody moving forward in a big way. I would very much appreciate any work you put into helping us with this. I will definitely be taking a look at your fork and hopefully we can begin to pull in any and all changes that help make Padrino a more secure framework.

@pke
Copy link

pke commented Jan 10, 2012

Has this already been addressed in 0.10.5?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants