GNOME Bugzilla – Bug 518920
Exchange connector does not handle form action with incomplete URL; also does not handle redirects
Last modified: 2009-01-12 18:07:47 UTC
Steps to reproduce: 1. Try to create an account for an Exchange server where the form action is simply "owaauth.dll" 2. For part 2, this should also be a redirected server. Mine redirects to "https://mail.<mydomain>/exchweb/bin/auth/owalogon.asp?url=https://mail.<mydomain>/exchweb&reason=0&replaceCurrent=1" from a shorter URL such as "/exchange" or even "/exchweb" 3. Stack trace: Stack trace omitted. See explanation below. Other information: e2k_context_fba, around line 599 of e2k-context.c, simply checks whether the form action begins with a /. If not, it uses the value as is in the call to e2k_soup_message_new_full. Since the action is not a URL here but simply "owaauth.dll", soup_uri_new returns a NULL URI pointer. This in turn returns a NULL message, which is never checked as such. The redirection problem comes from the fact that e2k_autoconfig_get_context gets the redirect message, then calls e2k_context_fba when it determines that the new URL contains owalogon.asp, but the e2k_context_fba is still generating URLs based on the pre-redirect URL.
Updated because this is still the case with Evolution 2.22 on Gnome Desktop 2.22
Created attachment 125865 [details] [review] Detect owaauth.dll in action and prepend /exchweb/bin/auth/ to it.
Patch looks good, I've only three questions, a) why do you chdir(getenv("HOME")); there? b) I'm fine with hardcoded path, but I wonder whether there is some way to read the full path from the responses of the server. Maybe it's there in some previous response, or just in the other response header? Can you check that and reply back, please? c) What name/email do you want to have in a ChangeLog entry for this? (Or even better resend patch with it filled too.) Thanks in advance. Note: I cannot test it myself, but I believe it works.
The second part is still opened. Would you mind to look at it too?
Oh my god. I'm so sorry. I was debugging a segfault one of my previous revisions of the patch caused, and that's an artefact from there. I was attempting to write to a logfile in $HOME. I'll update the patch shortly. As for the full path... I poked around the source code, and cannot find a way to get it. It was in one of the other functions, but short of creating a global variable, or modifying the E2kContext struct, I can't think of a way to get the full path. The global variable method would probably leak memory, and the E2kContext struct.. If I modify it I don't know what I could end up breaking.
Created attachment 126124 [details] [review] Updated patch Removed chdir(getenv("HOME")) and exchanged spaces in indentation for tabs.
I forgot to mention, that part 2 doesn't hold much significance. Well, at least the patch fixes the issue, and the Exchange server I connect to matches both said aspects.
It seems to me that the proper way is to get the exact path in the same way a web browser would. I'm pretty sure I did that while mucking around (but still found other problems popping up with my Exchange server and didn't have time to mess with them). I think Libsoup has functions for getting the new URL given the base and the link. That should be all you need, and would be more future-proof too.
Yes I believe that the proper way would be that too. If you managed to do that, I'd like to see your patch, even if it is incomplete. I could perhaps help to fix it. Even if libsoup has functions for getting the new URL based on the base and link, we don't even have the original link in the function I patched. The link we're looking for is in the variable called "location" in e2k_autoconfig_get_context(), in servers/exchange/lib/e2k-autoconfig.c. We need to somehow pass this into e2k-context.c, e2k_context_fba().
Okay, I just saw a backtrace in #534729 that clarified everything for me. I'll go make a patch without the hardcoded URI now. It seems e2k_autoconfg_get_context calls e2k_context_fba. If I modify the signature of e2k_context_fba, I can pass in the location to e2k_context_fba!
There doesn't seem to be any need to extend the e2k_context_fba at all, just get the location same as it is in the e2k_autoconfig_get_context. Only be sure you'll handle NULL there, in case it's not set in fba_timeout_handler. location = soup_message_headers_get (msg->response_headers, "Location"); By the way, try to run Evolution as this: E2K_DEBUG=5 evolution >&eex.log and just login to your exchange server and close evolution. Then open eex.log file and search for "owaauth.dll", there will be shown a message sent from the server, where you can see all available info in the 'msg' structure. Regarding the redirections, do you think it got fixed meanwhile or it's just other wording for the first problem? I'm not sure of the meaning here, I'm sorry.
Hoo boy. Don't I feel like an idiot now. It was sitting there all the while. As for the redirections, I think it's other wording for the first problem. The only problem I'm fixing here is that the path for action is wrong.
(In reply to comment #12) > Hoo boy. Don't I feel like an idiot now. It was sitting there all the while. :) > As for the redirections, I think it's other wording for the first problem. The > only problem I'm fixing here is that the path for action is wrong. Daniel, do you also agree that redirection will be no problem when the full path for the dll file will be used?
Hyperair, do not forget for credits in a ChangeLog entry, please add it to your final patch too. Thanks.
By redirects, I saw that when certain URL's were requested, the server would return a redirect to another URL instead of the page itself. It's been a while since I last tried the OWA connect, but I suspect that this may still cause problems unless this has already been handled in more recent releases.
I don't think the query string has anything to do with it. The stuff passed into the query string was for the form to render. In the case of my university's exchange server, it is stored in a hidden input field. If you go to this page: https://webmail.ntu.edu.sg/exchweb/bin/auth/owalogon.asp?url=rawr&reason=0&replaceCurrent=1 and view source, you will notice that there's a <input type="hidden" name="destination" value="rawr" /> in the form. This is handled in e2k_context_fba(), shortly after the part parsing the value of the "action" attribute in the form.
Created attachment 126141 [details] [review] Even more up to date patch Added ChangeLog entry, and removed hard-coding of path.
Milan, it fine for stable/trunk? It might miss today's release though
Hyperair, in the earlier patch you check for value = "owaauth.dll", in the second you check whether value does not begin with "http". The second seem to me incorrect, go rather with the first checking approach. I also see you do not check whether the "Location" header exists, whether soup_uri_new returned non-NULL value (the Location contained correct URL), and you forget to do something with a return value of soup_uri_decode (is that call necessary at all?).
(In reply to comment #15) > By redirects, I saw that when certain URL's were requested, the server would > return a redirect to another URL instead of the page itself. It's been a while > since I last tried the OWA connect, but I suspect that this may still cause > problems unless this has already been handled in more recent releases. I looked into the code and eex handles redirects by its own way, but it seems as not always. It uses SOUP_MESSAGE_NO_REDIRECT to be able to handle this itself, and a redirect_handler function, which is used always but not in e2k_context_fba. Heperair, I do not have environment to test this, can you try to just use this redirection method there, with unset SOUP_MESSAGE_NO_REDIRECT on the message, whether it'll solve both things? Thanks in advance.
Created attachment 126269 [details] test patch for evolution-data-server; Please revert all patches, apply this one to a clean sources and try to reproduce the issue. Did anything change?
Created attachment 126285 [details] [review] Fixed patch Removed soup_message_decode() calls, and did checks for location and suri -- if NULL, fail.
Created attachment 126294 [details] [review] Updated patch Instead of failing immediately location could not be found, we skip processing action and leave it alone.
Looks good, thanks for your work and sorry for a long run.
Committed to trunk. Committed revision 9924. Committed to gnome-2-24. Committed revision 9925.