Closed (fixed)
Project:
Echo
Version:
7.x-1.9
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
21 Feb 2012 at 15:50 UTC
Updated:
7 Mar 2018 at 14:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
yannisc commentedI put some watchdog messages in the _echo_access function to check what gets wrong. It appears that $_REQUEST['title'], $_REQUEST['content'] and $_REQUEST['theme'] are set, but $access = cache_get($key) returns false.
I don't know what that means.
Comment #2
sanderjp commentedSame here..
Comment #3
ymdavoust commentedsame issue for me!
Comment #4
pillarsdotnet commentedPerhaps your cache implementation is not working?
The
echo_themed_page()function contains the followingcache_set()call:Then the
echo_access()function merely performs a matchingcache_get():Hmm... This check will also fail if you have multiple Drupal servers behind a front-end round-robin proxy, and they do not share a common cache implementation.
Gonna add a sanity-check
echo_requirements()function.Comment #5
damien_vancouver commentedI had this problem too. XDebug wouldn't work in the access callback (because it is a drupal_http_request from code, not from my local browser). But some PHP error_logging and standalone PHP page testing gave me the following (confusing) results:
I'm currently at a loss to explain either of these behaviors.
As a workaround for now, I just added "return TRUE;" to the top of function _echo_access(). I will try some more testing next week when I have more time, and see if I can't narrow down what is causing the above. Note that if you put a return TRUE; in the access callback, you are opening yourself up to some nasty security issues. For example: Someone using /echo to echo back a fake login form using your site's current theme, that pointed to a malicious destination. (a phishing attack, but using your servers to make sure it looks exactly right). So beware and do not use that workaround as a permanent solution.
I've attached my patch to work around the ini_get issue, which might be useful because "max_execution_time" defaults to 0 or -1 in most CGI (FCGI as well?) php.ini's. So if PHP returned 0 or -1, that would be a nonsensical value and echo should probably use a sane default instead. This patch did not fix the problem for me on its own though!
I'll report back here when I find out anything further. I have plans to debug this properly next week, after a looming Monday demo deadline.
Comment #6
damien_vancouver commented@pillarsdotnet,
I tried your patch from #4, hoping my environment would at least fail the requirements test.
But, the test worked and it passes requirements, reporting that the cache system is working. This matches my testing in a standalone PHP page (see comments in #5). The failure only seems to occur inside the access callback which answers the drupal_http_request from echo_themed_page().
Attached is a re-roll of your #4 with a missing <?php opening tag added and no other changes. (The missing tag was causing the text of the .install file to show up at the top of pages).
If we can nail down exactly what is broken, we can then improve your test to hopefully catch what is really wrong. Or if it turns out to be something else (like a bug introduced in recent 7.x versions of Drupal, as I am starting to suspect), then the test is fine and is likely something that should be included as-is.
Has anyone on this thread got it working for them after it was failing?
Comment #7
pillarsdotnet commentedThirty seconds ought to be enough for anyone. (tm)
Are you using
DrupalDatabaseCacheor some otherDrupalCacheInterfaceimplementation?Are successive queries handled by the same PHP instance or by separate instances?
Does your hostname resolve to the PHP server or to a caching/proxy front-end?
Comment #8
damien_vancouver commentedEDIT: removed duplicate posts, Drupal.org having problems today.
Comment #9
damien_vancouver commentedI'm using DrupalDatabaseCache with a single instance and no proxies. One thing it might be is that I did have memcache running on this Drupal install before. I've since disabled the module and removed the settings.php cache entries. However, that's definitely near the top of my list of suspects. I'm not sure if I actually ran the uninstall, as I was planning on turning it back on later...
Just tried the uninstall of memcache / memcache_admin and.... YES!!!!! That was it!! Working now.
Then this is looking like it's really a memcache module issue instead - if that thing is disabled it shouldn't be interfering with normal cache operation. I will try and reproduce that failure on its own and then I'll start an issue there if there isn't one already.
Ideally we could have the cache check in requirements detect that failing configuration, if we can detect that. I can use a database dump from my production server to recreate that configuration, and I'll see if I can figure that out. Meanwhile I don't see any reason to hold up committing #7. I'll submit a patch here for a better test if I can work one out, and also a crosslink to the eventual memcache issue.
Thanks for your help and the enhancements!
Comment #10
damien_vancouver commentedEDIT: Please ignore double post.. Drupal.org having issues today.
Comment #11
damien_vancouver commentedSorry for that triplicate last post. Seems some of those error pages were actually saving!
Sadly, the problem is still here. I think what actually happened was my memcache uninstall did some kind of cache clearing that fixed it temporarily. I tried clearing all caches right before the page hit that sends the email, and tried again, and then I got a new strange result: An email back with the contents only, as if I wasn't using the echo module at all. So no access denied, but all the cached site theme stuff was suspiciously missing.
Anyway I'll test this more next week, starting with a fresh D7 install, and work my way back to where I am now with it failing. That will help us narrow it down further.
*rolls the "Mission Accomplished" banner back up for now, and puts it back in its box*
Comment #12
pillarsdotnet commentedHere's a slightly more robust approach.
Comment #13
damien_vancouver commentedNice one! Patch #11 applies cleanly, but shows it passing. I stepped through in XDebug and verified to myself the full HTML was returning with no Access Denied in there anywhere.
So I'm certain something else I have installed or something htmlmail is doing must be causing it. I wouldn't spend any more time on it until I chase down what that is, since it's clearly working in a simple use case. Thanks again for your expedient help so far!
Comment #14
pillarsdotnet commentedOh, well; I guess it won't hurt, even if it doesn't help.
Released 6.x-1.9 / 7.x-1.9 / 8.x-1.9 with the patch from #11 applied.
Setting status back to "Needs more info" until you (or someone else) gets around to testing further.
Comment #15
tomgf commentedI am using this version (7.x-1.9) and still having this problem (sometimes…).
I also had memcached installed (v. 7.x-1.0), which now I have disabled…
Any further advanced…?
Comment #16
pillarsdotnet commentedIf you have any suggestions as to how I can rewrite the
_echo_access()function to reject requests from outside hosts (including other virtual hosts on the same shared-hosting server), I'd love to hear them.I realize that depending on a working cache system is not an ideal solution, but I've been unable to come up with a better one.
Comment #17
tomgf commentedWhat about a configuration page where you can set a
valid_hostvariable…?I have written a patch with these changes:
I think this could work.
Comment #18
tomgf commentedLooking at the patch, I have noticed two things:
echo_valid_hostnameI have attached here a new version of the patch with these two changes.
Comment #19
pillarsdotnet commentedYou have totally misunderstood the problem.
Please read the security report at https://security.drupal.org/node/61334.
In short, your changes remove the existing protection against XSS attacks.
The
$_SERVER['HTTP_HOST']variable reports the internal name of the server executing the current script. OF COURSE that will be the same server that the REST of Drupal is in, unless you've inserted Echo module code into Joomla, for instance.The security concern is whether the request ORIGINATED on the same host that is SERVING it. If you can guarantee that no other website is sharing the same IP address, you can check whether
$_SERVER['REMOTE_ADDR']is equal to$_SERVER['SERVER_ADDR'].However, if I released code like that, I'd have a Drupal Security Advisory published against my module, warning that it is insecure in a shared-hosting environment.
Unfortunately, if
cache_clear_all()runs during the fraction of a second betweencache_set()andcache_get(), then you'll get an Access Denied error. No way around it that I can see. Setting a minimum cache lifetime won't even help.This is a known issue and is being worked on.
Unfortunately, a real fix would require a dedicated table just for holding Echo tokens. And I'd have to implement my own cache clearing logic.
(sigh) This was supposed to be such a beautifully simple module.
Comment #20
tomgf commentedI am sorry if I misunderstood the problem. I have attempted to help with a solution (that obviously won't work).
I thought that according to what you've explained as “to reject requests from outside hosts”, one could to set an option specifying which is the valid hostname, leaving everything else in the outside world. According to that, what I have tried to proposed was to check $_SERVER['SERVER_NAME'] against this configured value stored by the administrator on the DB as a variable. After your comment, I understand that the $_SERVER variables, either SERVER_NAME or HTTP_HOST can be also spoofed… (I am not authorized to access https://security.drupal.org/node/61334, though).
Using the CACHE_PERMANENT option –that won't be clean unless explicitly told– wouldn't work…?
Extra note: in my attempt to produce this patch I have noticed that the git command
git clone --branch 7.x-1.x http://git.drupal.org/project/echo.gitis pulling a version that it's not includingecho.install…Comment #21
tomgf commentedI am giving it another try.
In this case, I have added a particular cache bin for this module. It's a work in progress –as there is no garbage collector and so– but maybe this provides a better approach.
And at least this cache is not emptied with the other ones. Actually, so far it's not emptied at all… (sth that I could work on if this is the way to go)
Comment #22
pillarsdotnet commentedHere's a copy of the security report.
Comment #23
kevinquillen commentedHad this same problem. Emails would be fine for a period in time, then clearing Varnish/site cache would result in any outgoing email having Access Denied page inside of it instead.
I simply threw 'return TRUE' at the start of _echo_access() to get around this for now. Its being used on a production site that sends a few dozen emails a day.
Comment #24
bsztreha commentedSame problem, and cache working:
Cache system - Working
The Echo module requires a working cache system.
Comment #25
applicity_sam commentedI have also seen this and spent the morning tracking it down. I think the issue is as follows:
A module on your site creates a transaction using db_transaction. (in my case it was commerce).
echo calls cache_set which puts the data into the database that echo needs to check access control.
As the transaction is open this is not committed.
A sub request is spun up which starts a new drupal process which is then trying to read from the cache. At this point the second process cannot see the cache data as it hasn't yet been committed. So it returns an access denied.
The original drupal process completes and the the data is then visible in the cache.
In my scenario this only breaks if I have something in my cart, as this causes an order to be loaded which holds a transaction open. I am going to log that as an issue on commerce.
Comment #26
mlecha commentedAs soon as I apply a theme in Step 2 of htmlmail config:
However "No Theme" works fine.
Using Commerce Kickstart 7.x-1.11 install profile.
Site status reports "Cache system Working".
Adding "return TRUE;" to _echo_access() doesn't work for me.
Comment #27
jos_s commentedI have this issue too, but it seems to depend on the server the website is running on. I have a local development server that gives no problems, but when I copy the site to the production server (not mine), I get the Access Denied messages in the html mail. (using echo 7.x-1.9)
Edit 17 January:
Does anybody have any ideas how this can depend on the server where the Drupal site is installed? I have spent days looking for a solution, but haven't a clue where to look for. This is a show stopper for the website I am building.
Comment #28
kevinquillen commentedLets back up for a second- what is the reason behind the /echo callback? If its just to theme the email message with the site email, something seems a bit off here to go through a lot of access checks.
Comment #29
pillarsdotnet commented@#28 -- It is not possible for Drupal to switch themes without a callback. A callback without an access check was deemed unsafe by the security team.
Comment #30
jos_s commentedAfter doing some more research I am wondering if issues with
drupal_http_request()could be causing this (see http://drupal.org/node/1664784).If no HTTP request can be made from the server, the echo will return the "Access denied" message in stead of the HTML newsletter.
Is there a way around this?
Comment #31
jos_s commentedI finally found what caused my Access Denied errors. It appeared that the production server had Apache
mod_security.cactivated. Adding the following lines to.htaccesssolved it:Found the solution by studying the server log files and then finding this page: http://drupal.org/node/109058
Comment #32
TamB commentedI seem to be still having this problem with 7.x-1.9. Wondering if anyone has found a viable solution as yet.
What are the security implications of #31. Is it a long term solution or can it be used only as a stop gap?
Comment #33
jacob.embree commented#25 is correct. This patch doesn't fix anything but will log a warning if the external request returns a 403 and a DB transaction is open.
Comment #34
letapjar commentedWhy can't echo store a variable in the db i.e. 'echo_key' and then use that variable to munge the
sha1 calls in both the echo_themed_page function as well as in the access callback?
When making the httprequest - you pass along the munged token - then re-calculate the token in the access callback and compare it to what was passed in.
This requires that during install - the 'echo_key' variable gets set, but it gets around the cache clearing issue and I *think* it serves the purpose.
Comment #35
letapjar commentedHere's a patch that implements what #34 is talking about. I could be missing the entire issue here. But this avoids the cache altogether.
Comment #36
letapjar commentedRe-rolling patch from #35 to 1) get rid of cache_set as it's not needed
2) add in hook_uninstall to delete the echo_key variable.
Comment #37
jacob.embree commentedThe issue is not that the cache is cleared in between but rather that any changes to the database during a transaction, either in a cache table or in the variables table, do not take effect until the transaction is committed.
Need a way to authenticate access to /echo that does not depend on the state of the database.
Comment #38
letapjar commentedRe #37 - Right I get it - db write during a transaction won't be saved until the transaction closes.
The approach in #36 totally avoids that. You can still read from the db during a transaction.
The echo_key I propose is bascially a secret key much like the app_secret you get when creating an app for facebook or twitter. It is set once when the module is installed and does not need to be changed later.
During the echo_themed_page() and _echo_access() callbacks - there is no need for a db write - instead we use a db_read of the secret key and on each side separately calculate the digest (passed in the internal http_request as the "token").
In _echo_access() - the only way that $calculated_token will equal the token passed in with the http_request is if they both used the same echo_key. Since the echo_key is never publicly exposed this serves the purpose of making sure requests came from the site itself and not from the outside.
There's still the separate issue that the module is not using rawurldecode() in the _echo_access() function. This should be fixed and I've filed a separate issue for that.
These changes are working nicely for my sites.
Comment #39
jacob.embree commentedMy apologies. echo-access_denied_in_email_sent-1448278-36.patch in #36 works. Next time I'll test before making a silly comment.
Comment #40
alauddin commentedcan confirm patch from #36 works...D7.34 & Echo 7.x-1.9
Comment #41
joran lafleuriel commented#36 saved my life ! Thanks a lot.
All html emailing process was corrupted before I patched Echo 7.x.19 with this.
I succeed to send html emails to gmail with this conf :
Comment #43
Nafes commentedCommitted. Thanks for the patch, letapjar!
Comment #45
timb commentedWas this really committed? I see that &x-1.9 hasn't been updated since March 2012.
Comment #46
markusd1984 commentedIs this the same error as "401 Unauthorized"?
I tried patch #36 but unfortunately it did not resolve this.
Html mails without an Email Theme work fine. Any advise how to debug / troubleshoot please?
FYI - I am using SMTP module with HTML MAIL (& SSO with LDAP over HTTPS).