On 08/26/2011 11:02 AM, Miroslav Suchý wrote:
> On 08/25/2011 06:55 PM, Johannes Renner wrote:
>> Well, I can count three, while only one (!) of them is public. But we
>> can still rename the other ones, no problem.
>
> I count:
> private static Logger log = Logger.getLogger(AuditLog.LOGGER_NAME);
> as well.
While this is not a method and also not a function, however ...
>> - How can you distinguish between interesting and uninteresting requests?
>> - There is some interesting requests that are not POST requests, e.g.
>> logouts.
>> - There is some POST requests that are not interesting, e.g user selects all
>> entries of a list.
>> - Log events cannot be categorized for a later filtering. At a single entry
>> point it is very hard to see what really has happened.
>
> I thought that there will be configuration file, where you state what
> and how will be logged. All based on URI similary to struts config file.
> E.g.
>
> /rhn/LoginSubmit.do {
> key = "LOGIN"
> value = "user=${POST.username};pass=${POST.password}"
> }
>
> /rhn/admin/config/GeneralConfig.do {
> key = "CONF"
> value = "email=${POST.email};....."
> }
>
> etc. you probably got the idea now.
> And those url not specified will not be logged.
I got the idea and I was even researching such an approach already. For the
webapp I started with a ServletFilter (see my attached patch as an example)
that simply logs all POST requests to the backend using my helper classes
from the patches I already sent. The main thing that's missing would be the
integration of a configuration file like the above.
I will now continue to investigate in this approach since I agree with you
that it will be much easier to maintain than having log statements all over
the code. However there is also some drawbacks:
- Performance might be worse, since _every_ request this filter is registered
for (e.g. all *.do) will be checked if it needs to be logged or not
- Sometimes the same URL is used for different actions, e.g. creating and
updating an object, so classification of log events might be difficult or
even not possible
- Sometimes you only want to log the request in case a certain parameter is
there, so there would need to be a something like a list of "parameters
required for logging" for each URL in the config
- does it make sense to have a whitelist of interesting parameters for each
URL or rather take everything and maintain a global blacklist?
>> - When using an external entry point (like mod_security), you can't actually
>> see from the logs which user was involved since it is not possible to map
>> between uid, sid, ... and real world 'objects'.
>
> I said "something liek mod_security". I can imagine build something upon
> existing project, but even something new written from scratch just for
> Spacewalk.
> And translating sid to user is not so big problem. You can have config
> file where you specify how you translate sid to user. Ie.
> [translate]
> user = "select login from web_contact join pxtsession on
> web_user_id=web_contact.id where pxt.id = :sid"
>
> and in logging config have:
> /rhn/admin/config/GeneralConfig.do {
> key = "CONF"
> translate[user] = sid
> value = "logged=${user};email=${POST.email};....."
> }
>
> This way it can be even Spacewalk independent and you can use it on
> different project where they have different tables.
Yes, but I actually think it would make sense to do this specifically within
spacewalk-java, because there is already code to determine all the stuff from
the database. To me it would make sense to reuse this code, so we don't need
to rewrite all those queries?
>> I agree with you completely on the fact that getting the big picture is hard,
>> but generic logging of request data does somehow not satisfy our needs :-/
>
> So there is place to write one :)
> Just think that after some years customer will ask you "and which
> events/action/url are logged? Can you give me the list."
> And you will have hard time to provide such list.
Yes, and such a configuration could even be modified by a customer itself.
--
SUSE LINUX Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
diff --git a/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java b/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java
new file mode 100644
index 0000000..df3c728
--- /dev/null
+++ b/java/code/src/com/redhat/rhn/frontend/servlets/AuditLogFilter.java
@@ -0,0 +1,70 @@
+/**
+ * Copyright (c) 2011 Novell
+ *
+ * This software is licensed to you under the GNU General Public License,
+ * version 2 (GPLv2). There is NO WARRANTY for this software, express or
+ * implied, including the implied warranties of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE. You should have received a copy of GPLv2
+ * along with this software; if not, see
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
+ *
+ * Red Hat trademarks are not licensed under GPLv2. No permission is
+ * granted to use or replicate Red Hat trademarks that are incorporated
+ * in this software or its documentation.
+ */
+
+package com.redhat.rhn.frontend.servlets;
+
+import java.io.IOException;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.struts.Globals;
+
+import com.redhat.rhn.common.conf.Config;
+import com.redhat.rhn.common.conf.ConfigDefaults;
+import com.redhat.rhn.common.logging.AuditLog;
+
+/**
+ * AuditLogFilter for generic logging of HTTP requests.
+ *
+ * @version $Rev$
+ */
+public class AuditLogFilter implements Filter {
+
+ FilterConfig filterConfig;
+
+ /** {@inheritDoc} */
+ public void init(FilterConfig filterConfig) {
+ this.filterConfig = filterConfig;
+ }
+
+ /** {@inheritDoc} */
+ public void destroy() {
+ this.filterConfig = null;
+ }
+
+ /** {@inheritDoc} */
+ public void doFilter(ServletRequest req, ServletResponse resp,
+ FilterChain chain) throws IOException, ServletException {
+
+ // Finished for now
+ chain.doFilter(req, resp);
+
+ // Check if audit logging is enabled
+ if (Config.get().getBoolean(ConfigDefaults.AUDIT)) {
+ HttpServletRequest request = (HttpServletRequest) req;
+ // Log POST requests that do not have errors
+ if (request.getMethod().equals("POST")
+ && request.getAttribute(Globals.ERROR_KEY) == null) {
+ AuditLog.getInstance().log(null, request, null);
+ }
+ }
+ }
+}
diff --git a/java/code/webapp/WEB-INF/web.xml b/java/code/webapp/WEB-INF/web.xml
index 4e60f04..33bb88d 100644
--- a/java/code/webapp/WEB-INF/web.xml
+++ b/java/code/webapp/WEB-INF/web.xml
@@ -40,6 +40,11 @@
</filter-class>
</filter>
+ <filter>
+ <filter-name>AuditLogFilterStruts</filter-name>
+ <filter-class>com.redhat.rhn.frontend.servlets.AuditLogFilter</filter-class>
+ </filter>
+
<!-- DumpFilter dumps the entire request to the log file as log debug. -->
<!--
<filter>
@@ -121,6 +126,11 @@
</filter-mapping>
-->
+ <filter-mapping>
+ <filter-name>AuditLogFilterStruts</filter-name>
+ <url-pattern>*.do</url-pattern>
+ </filter-mapping>
+
<!--
If you need a filter to operate on more than one
url pattern, you must declare it for each of the patterns.
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
