I can agree with that. In any case, the excludes needed for setup should be removed after the setup has been completed.
However, we should try to preserve the functionality, as some third party products (such as custom plugins) rely on it. If we now simply disable the possiblilty to exclude something, without offering an alternative, we’ll end up upsetting a number of Openfire admins.
I thought about exact matches. I think most plugins will not need any changes since there is only one single exclude.
String url = request.getServletPath();
boolean doExclude = excludes.contains(url);
This is also a performance improvement since it does now make sense to use an HashSet…
However, I think this would require also the following changes:
public static void addExclude(String exclude) {
excludes.add("/plugins/" + exclude);
} public static void removeExclude(String exclude) {
excludes.remove("/plugins/" + exclude);
}
All official plugins should confirm with that. Methods addExclude() and removeExclude() are not used by Openfire itself.
Message was edited by: Coolcat
Hm, it’s not that easy…currently there is an exclude for index.jsp?logout=true
getServletPath() does turncate the query string and it’s not a good idea to exclude index.php itself.
I think we can just remove the exclude for index.jsp?logout=true
The logout parameter in index.jsp is handled like this:
<% // Simple logout code
if (“true”.equals(request.getParameter(“logout”))) {
session.removeAttribute(“jive.admin.authToken”);
response.sendRedirect(“index.jsp”);
return;
}
%>
This will still work if the user is authenticated.
If the user is not authenticated he will be directed to the login page by the AuthFilter. This also happens right now as index.jsp?logout=true redirects to index.jsp which requires authentication. There is no need to remove the authToken from the session if the user is not authenticated.
We get an directory listing for
and *.css, *.js and image files can be accessed without exclusion.
AuthCheck is only applied to *.jsp files.
We should apply the filter to everything:
<filter-mapping>
<filter-name>AuthCheck</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
and then create a whitelist for everything that is requried for login. For the last two I’m not sure if this is required.
<filter>
<filter-name>AuthCheck</filter-name>
<filter-class>org.jivesoftware.admin.AuthCheckFilter</filter-class>
<init-param>
<param-name>excludes</param-name>
<param-value>
/login.jsp,
/style/global.css,
/style/login.css,
/images/login_logo.gif,
/images/error-16x16.gif,
/images/jive-login-form-bg-gray.gif,
/images/jive-login-bg.gif,
/error-serverdown.jsp,
/setup/clearspace-integration-prelogin.jsp
</param-value>
</init-param>
</filter>
For AuthCheckFilter we take the following, so setup will work.
String srvpath = request.getServletPath();
boolean doExclude = XMPPServer.getInstance().isSetupMode() || excludes.contains(srvpath);
However, for some strage reason setup does not work for me, but it is possible that is some other problem not related with that change.
java.lang.NullPointerException
at org.jivesoftware.openfire.admin.index_jsp._jspService(index_jsp.java:131)
at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:97)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:820)
at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:487)
(...)
Message was edited by: Coolcat
Argh, I just about finished a section that adds/removes the proper setup excludes before/after setup. Simply allowing everything during setup is much easier . Let me look into that NullPointer…
Guus and I worked on a patch for this. I posted it to JM-1489. However, the problem with plugins that exclude them self from AuthCheckFilter is still not solved.
I think we have three options for org.jivesoftware.admin.AuthCheckFilter#addExclude(String):
We could discuss this on the community chat starting in two hours.
Well. I dont understand most of this stuff. But i prefer to reinstall some plugins than being insecure. Must say, that i’m secured anyway (using it in LAN with admin console firewalled), but i’m trying to represent those admins feelings I think their would be much happier to know they are secure, than not knowing anything about the troubles. Anyway, i think the message about Openfire vulnerabilities soon fly around the world (already reported in a couple of sec.advisories).
Todd, you already a Top Participant You dont have to work hard here anymore
But i prefer to reinstall some plugins than being insecure.
Securtiy problem would be solved with this patch in any way. The only problem is that some plugins will not longer work. However, for these plugins it would be a one line quick fix.
My vote goes to doing this properly and change the plugins.
Hi,
while also I can see and reproduce these problems with my Openfire installation I did setup a long time ago a reverse proxy in front of Openfire to get an access log of the Openfire console. The proxy does also restrict access and normalizes the URL so I had to bypass the proxy to reproduce this problem.
Is the auth filter bypass vunerability a Jetty bug?
LG
Is the auth filter bypass vunerability a Jetty bug?
No, it’s a bad implementation in org.jivesoftware.admin.AuthCheckFilter
An obvious thing to do would also be adding session checks to the relevant jsp files just to be sure. It doesn’t cost much after all.
In the meantime JM-1489 has been closed as fixed but I think it’s not been fixed actually (see its comments). I sent a mail to Gaston and Matt. I hope this doesn’t get released as 3.6.1 (it was tagged as 3.6.1 in SVN already…).
I have to agree Muesli, this is not fixed! Currently downloading SVN to be sure. Also Matt’s solution is a very convoluted solution. In security related code this is not a good idea…
I’m sure now: this is not fixed.
Hey coolcat,
I reopened the ticket at the risk of loosing my elevated Jira privs This issue is a microcasm of what is wrong with the release / development process as it stands.
PS. I am pretty darn annoyed that the two trivial fix typo bugs were ignored for this ‘release’. They both break basic functionality, oh well.
daryl
Hey Guys,
Thanks for testing the fix. When we tested it we missed that combination. We are still in time to include a better fix for 3.6.1.
Hey Daryl,
Why that presumption that we would remove your permissions? We do appreciate your effort and contribution to the community. Keep up your work!!!
– Gato
Daryl,
PS. I am pretty darn annoyed that the two trivial fix typo bugs were ignored for this ‘release’. They both break basic functionality, oh well.
No need to be annoyed! I missed the bugs that you are referring to. BTW, do you have a link to them? I can still include them in 3.6.1.
Thanks,
– Gato
Hi Gato,
JM-1487
JM-1460
I had gone thru and prioritized these two bugs as blockers for the release…
PS. Would sure be nice to have auto JM- linking working again in CS…
daryl