You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by arshadmohammad <gi...@git.apache.org> on 2017/08/21 19:15:10 UTC

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

GitHub user arshadmohammad opened a pull request:

    https://github.com/apache/zookeeper/pull/338

    ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

    Audit logging in ZooKeeper Servers.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/arshadmohammad/zookeeper ZOOKEEPER-1260-AuditLog

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/338.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #338
    
----
commit 502fbf7b2127cc4cf8284d8ecc181ae47e02b2d6
Author: Mohammad Arshad <ar...@apache.org>
Date:   2017-07-11T14:42:13Z

    ZOOKEEPER-1260:Audit logging in ZooKeeper Servers.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135347450
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/ZKAuditLogger.java ---
    @@ -0,0 +1,118 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import org.apache.zookeeper.server.ServerCnxnFactory;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +public class ZKAuditLogger {
    +    public static final String SYSPROP_AUDIT_ENABLED = "zookeeper.audit.enabled";
    +    private static final Logger LOG = LoggerFactory.getLogger(ZKAuditLogger.class);
    +    // By default audit logging is disabled
    +    public static final boolean isAuditEnabled = Boolean.getBoolean(SYSPROP_AUDIT_ENABLED);
    +    public static final boolean isAuditDisabled = !isAuditEnabled;
    --- End diff --
    
    not convinced this needs to be its own variable


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134566167
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
    @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888</programlisting>
                   feature. Default is "true"</para>
                 </listitem>
               </varlistentry>
    -
    +          <varlistentry>
    +            <term>audit.enabled</term>
    +            <listitem>
    +                <para>(Java system property:
    +                    <emphasis role="bold">zookeeper.audit.enabled</emphasis>)
    +                </para>
    +                <para>
    +                    <emphasis role="bold">New in 3.5.3:</emphasis>
    --- End diff --
    
    corrected it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135343845
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, RFAAUDIT
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    --- End diff --
    
    I'm sure this is a very dumb question, why is this called RFAAUDIT?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136173824
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    --- End diff --
    
    Good suggestion.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134382155
  
    --- Diff: src/docs/src/documentation/content/xdocs/site.xml ---
    @@ -52,6 +52,7 @@ See http://forrest.apache.org/docs/linking.html for more info.
           <jmx   label="JMX"                    href="zookeeperJMX.html" />
           <observers label="Observers Guide" href="zookeeperObservers.html" />
           <reconfig label="Dynamic Reconfiguration" href="zookeeperReconfig.html" />
    +      <reconfig label="Audit Logging" href="zookeeperAuditLogs.html" />
    --- End diff --
    
    Shouldn't this be something like `<audit label="Audit Logging" href="zookeeperAuditLogs.html" />` rather than `<reconfig `?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136155728
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    --- End diff --
    
    Done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136176181
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    --- End diff --
    
    This is important part of the audit log configuration. These will rarely be changed. I think it is ok to keep it here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r218073947
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
    @@ -0,0 +1,37 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +public class AuditConstants {
    +    public static final String SUCCESS = "success";
    +    public static final String FAILURE = "failure";
    +    // operation is performed, result is not known yet
    +    public static final String INVOKED = "invoked";
    +    public static final String KEY_VAL_SEPARATOR = "=";
    +    public static final char PAIR_SEPARATOR = '\t';
    +
    +    public static final String OP_START = "serverStart";
    --- End diff --
    
    Operation names are used multiple places through constants. So keeping the AuditConstants as it is. Also not exposing operation name through ZooDefs  constants.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136150155
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    --- End diff --
    
    corrected


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r217179644
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/ZKAuditLogger.java ---
    @@ -0,0 +1,117 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import org.apache.zookeeper.server.ServerCnxnFactory;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +public class ZKAuditLogger {
    +    public static final String SYSPROP_AUDIT_ENABLED = "zookeeper.audit.enabled";
    +    private static final Logger LOG = LoggerFactory.getLogger(ZKAuditLogger.class);
    +    // By default audit logging is disabled
    +    public static final boolean isAuditEnabled = Boolean.getBoolean(SYSPROP_AUDIT_ENABLED);
    +
    +    /**
    +     *
    +     * Prints audit log based on log level specified
    +     *
    +     */
    +    public static enum LogLevel {
    +        ERROR {
    +            @Override
    +            public void printLog(String logMsg) {
    +                LOG.error(logMsg);
    +            }
    +        },
    +        INFO {
    +            @Override
    +            public void printLog(String logMsg) {
    +                LOG.info(logMsg);
    +            }
    +        };
    +        public abstract void printLog(String logMsg);
    +    }
    +
    +    public static enum Keys {
    +        USER, OPERATION, RESULT, IP, ACL, ZNODE, SESSION;
    +    }
    +
    +    public static void logInvoked(String user, String operation) {
    +        log(LogLevel.INFO, user, operation, AuditConstants.INVOKED);
    +    }
    +
    +    public static void logSuccess(String user, String operation) {
    +        log(LogLevel.INFO, user, operation, AuditConstants.SUCCESS);
    +    }
    +
    +    public static void logFailure(String user, String operation) {
    +        log(LogLevel.ERROR, user, operation, AuditConstants.FAILURE);
    +    }
    +
    +    private static void log(LogLevel level, String user, String operation, String logType) {
    +        level.printLog(createLog(user, operation, null, null, null, null, logType));
    +    }
    +
    +    public static void logSuccess(String user, String operation, String znode, String acl, String session, String ip) {
    +        LogLevel.INFO.printLog(createLog(user, operation, znode, acl, session, ip, AuditConstants.SUCCESS));
    +    }
    +
    +    public static void logFailure(String user, String operation, String znode, String acl, String session, String ip) {
    +        LogLevel.ERROR.printLog(createLog(user, operation, znode, acl, session, ip, AuditConstants.FAILURE));
    +    }
    +
    +    /**
    +     * A helper api for creating an audit log string.
    +     */
    +    public static String createLog(String user, String operation, String znode, String acl, String session, String ip,
    +            String status) {
    +        ZKAuditLogFormatter fmt = new ZKAuditLogFormatter();
    --- End diff --
    
    ZKAuditLogFormatter is not putting info for which value is null. I think we can not do with String.format()


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344946
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    --- End diff --
    
    for the setAcl operation


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136154472
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    --- End diff --
    
    figure is giving more information. I think it is good to have. May be we can  re-frame the sentence, any suggestion?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    @pmoust I was pointing to comments: [1](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612302&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612302), [2](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612315&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612315) and [3](https://issues.apache.org/jira/browse/ZOOKEEPER-1260?focusedCommentId=16612318&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16612318)


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r218066854
  
    --- Diff: src/java/main/org/apache/zookeeper/ZKUtil.java ---
    @@ -168,4 +169,41 @@ private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path,
                 return; // ignore
             }
         }
    +
    +    /**
    +     * @param perms
    +     *            ACL permissions
    +     * @return string representation of permissions
    +     */
    +    public static String getPermString(int perms) {
    --- End diff --
    
    Good suggestion
    selecting dynamic caching


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134566109
  
    --- Diff: src/docs/src/documentation/content/xdocs/site.xml ---
    @@ -52,6 +52,7 @@ See http://forrest.apache.org/docs/linking.html for more info.
           <jmx   label="JMX"                    href="zookeeperJMX.html" />
           <observers label="Observers Guide" href="zookeeperObservers.html" />
           <reconfig label="Dynamic Reconfiguration" href="zookeeperReconfig.html" />
    +      <reconfig label="Audit Logging" href="zookeeperAuditLogs.html" />
    --- End diff --
    
    yes it should be <audit. Corrected it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135361252
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +490,129 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request,ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request,ServerCnxn cnxn, String op, String path, String acl) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl,
    +                getSessionId(cnxn), getHostAddress(cnxn));
    +    }
    +
    +    private void addFailureAudit(Request request,ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request,ServerCnxn cnxn, String op, String path, String acl) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl,
    +                getSessionId(cnxn), getHostAddress(cnxn));
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, getSessionId(cnxn),
    +                    getHostAddress(cnxn));
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, getSessionId(cnxn),
    +                    getHostAddress(cnxn));
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        String op = AuditConstants.OP_CREATE;
    +        if (request.cnxn == null) {
    +            return;
    +        }
    +        String path=null;
    +        long sessionId = -1;
    +        String address = null;
    +        String acls = null;
    +        boolean exceptionOccured = false;
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        try {
    +            sessionId = request.cnxn.getSessionId();
    +            switch (request.type) {
    +            case OpCode.create:
    +            case  OpCode.create2:
    +            case  OpCode.createContainer:
    +                op = AuditConstants.OP_CREATE;
    +                CreateRequest createRequest = new CreateRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, createRequest);
    +                path=createRequest.getPath();
    +                break;
    +            case OpCode.delete:
    +            case OpCode.deleteContainer:
    +                op = AuditConstants.OP_DELETE;
    +                //path = new String(request.request.array());
    +                DeleteRequest deleteRequest = new DeleteRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, deleteRequest);
    +                path=deleteRequest.getPath();
    +                break;
    +            case OpCode.setData:
    +                op = AuditConstants.OP_SETDATA;
    +                SetDataRequest setDataRequest = new SetDataRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, setDataRequest);
    +                path=setDataRequest.getPath();
    +                break;
    +            case OpCode.setACL:
    +                op = AuditConstants.OP_SETACL;
    +                SetACLRequest setACLRequest = new SetACLRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +                path=setACLRequest.getPath();
    +                acls = ZKUtil.aclToString(setACLRequest.getAcl());
    +                break;
    +            case OpCode.multi:
    +                op = AuditConstants.OP_MULTI_OP;
    +                break;
    +            case OpCode.reconfig:
    +                op = AuditConstants.OP_RECONFIG;
    +                break;
    +            }
    +            if (request.cnxn != null
    +                    && request.cnxn.getRemoteSocketAddress() != null
    +                    && request.cnxn.getRemoteSocketAddress().getAddress() != null) {
    +                address = request.cnxn.getRemoteSocketAddress().getAddress()
    +                        .getHostAddress();
    +            }
    +        } catch (Throwable e) {
    +            exceptionOccured = true;
    +            LOG.error("Failed to audit log request {} failure", request.type, e);
    +        }
    +        if (!exceptionOccured) {
    +            if (ZKAuditLogger.isAuditEnabled) {
    --- End diff --
    
    nit: we can combine these if statements
    
    alternatively you can return in the catch block


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136150114
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
    @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888</programlisting>
                   feature. Default is "true"</para>
                 </listitem>
               </varlistentry>
    -
    +          <varlistentry>
    +            <term>audit.enabled</term>
    +            <listitem>
    +                <para>(Java system property:
    +                    <emphasis role="bold">zookeeper.audit.enabled</emphasis>)
    +                </para>
    +                <para>
    +                    <emphasis role="bold">New in 3.5.4:</emphasis>
    +                    By default audit logs are disabled. Set to "true" to enable this feature. Default is "false". See
    --- End diff --
    
    corrected.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136157257
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    --- End diff --
    
    corrected


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    Thanks @hanm @afine  for reviewing this feature. I have addressed all the comments, Please have a look.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136158981
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    +    <para>By default there are only four authentication provider</para>
    +    <itemizedlist>
    +        <listitem>
    +            <para>IPAuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>SASLAuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>X509AuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>DigestAuthenticationProvider</para>
    +        </listitem>
    +    </itemizedlist>
    +    <para>User is decided based on the configured authentication provider.</para>
    --- End diff --
    
    corrected.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136158827
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    +    <para>By default there are only four authentication provider</para>
    --- End diff --
    
    correct


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136662232
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    --- End diff --
    
    I agree the figure is great! My point was that you can just say "as depicted below". 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    Hi all, please find performance impact analysis in jira comments.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136149088
  
    --- Diff: src/docs/src/documentation/content/xdocs/index.xml ---
    @@ -66,6 +66,7 @@
           <li><a href="zookeeperHierarchicalQuorums.html">Hierarchical quorums</a></li>
           <li><a href="zookeeperObservers.html">Observers</a> - non-voting ensemble members that easily improve ZooKeeper's scalability</li>
           <li><a href="zookeeperReconfig.html">Dynamic Reconfiguration</a> - a guide on how to use dynamic reconfiguration in ZooKeeper</li>
    +      <li><a href="zookeeperAuditLogs.html">Audit Logging</a> - a guide on how to configure audit logs in ZooKeeper Server and what contents are logged.</li>
    --- End diff --
    
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136157192
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    --- End diff --
    
    In audit log there is information about who has done a zk operation. There are many different authentication mechanisms, some take kerberos principal, some take ip, some take simplly user name as user. Second sentence is directing reader to this detail.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135361699
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
             return request.cnxn == null;
         }
     
    +    private String getHostAddress(ServerCnxn cnxn) {
    +        try {
    +            if (cnxn == null) {
    +                return "";
    +            }
    +            InetSocketAddress remoteSocketAddress = cnxn
    +                    .getRemoteSocketAddress();
    +            if (remoteSocketAddress == null) {
    +                return "";
    +            }
    +            InetAddress address = remoteSocketAddress.getAddress();
    +            if (address == null) {
    +                return "";
    +            }
    +            return address.getHostAddress();
    +        } catch (Exception e) {
    +            // ignore
    +        }
    +        return "";
    +    }
    +
    +    private String getSessionId(ServerCnxn cnxn) {
    --- End diff --
    
    perhaps we can move this string formatting code to somewhere more appropriate


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183432327
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/ZKAuditLogger.java ---
    @@ -0,0 +1,117 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import org.apache.zookeeper.server.ServerCnxnFactory;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +public class ZKAuditLogger {
    +    public static final String SYSPROP_AUDIT_ENABLED = "zookeeper.audit.enabled";
    +    private static final Logger LOG = LoggerFactory.getLogger(ZKAuditLogger.class);
    +    // By default audit logging is disabled
    +    public static final boolean isAuditEnabled = Boolean.getBoolean(SYSPROP_AUDIT_ENABLED);
    +
    +    /**
    +     *
    +     * Prints audit log based on log level specified
    +     *
    +     */
    +    public static enum LogLevel {
    +        ERROR {
    +            @Override
    +            public void printLog(String logMsg) {
    +                LOG.error(logMsg);
    +            }
    +        },
    +        INFO {
    +            @Override
    +            public void printLog(String logMsg) {
    +                LOG.info(logMsg);
    +            }
    +        };
    +        public abstract void printLog(String logMsg);
    +    }
    +
    +    public static enum Keys {
    +        USER, OPERATION, RESULT, IP, ACL, ZNODE, SESSION;
    +    }
    +
    +    public static void logInvoked(String user, String operation) {
    +        log(LogLevel.INFO, user, operation, AuditConstants.INVOKED);
    +    }
    +
    +    public static void logSuccess(String user, String operation) {
    +        log(LogLevel.INFO, user, operation, AuditConstants.SUCCESS);
    +    }
    +
    +    public static void logFailure(String user, String operation) {
    +        log(LogLevel.ERROR, user, operation, AuditConstants.FAILURE);
    +    }
    +
    +    private static void log(LogLevel level, String user, String operation, String logType) {
    +        level.printLog(createLog(user, operation, null, null, null, null, logType));
    +    }
    +
    +    public static void logSuccess(String user, String operation, String znode, String acl, String session, String ip) {
    +        LogLevel.INFO.printLog(createLog(user, operation, znode, acl, session, ip, AuditConstants.SUCCESS));
    +    }
    +
    +    public static void logFailure(String user, String operation, String znode, String acl, String session, String ip) {
    +        LogLevel.ERROR.printLog(createLog(user, operation, znode, acl, session, ip, AuditConstants.FAILURE));
    +    }
    +
    +    /**
    +     * A helper api for creating an audit log string.
    +     */
    +    public static String createLog(String user, String operation, String znode, String acl, String session, String ip,
    +            String status) {
    +        ZKAuditLogFormatter fmt = new ZKAuditLogFormatter();
    --- End diff --
    
    If I got it right, this is the only place where you use `ZKAuditLogFormatter`. Why don't you just use `String.format()` instead of a custom formatter?


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135363607
  
    --- Diff: src/java/test/org/apache/zookeeper/audit/ZKAuditLoggerTest.java ---
    @@ -0,0 +1,377 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
    +import static org.junit.Assert.assertEquals;
    +
    +import java.io.ByteArrayOutputStream;
    +import java.io.IOException;
    +import java.io.LineNumberReader;
    +import java.io.StringReader;
    +import java.net.InetAddress;
    +import java.net.InetSocketAddress;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.apache.log4j.Layout;
    +import org.apache.log4j.Level;
    +import org.apache.log4j.Logger;
    +import org.apache.log4j.SimpleLayout;
    +import org.apache.log4j.WriterAppender;
    +import org.apache.zookeeper.CreateMode;
    +import org.apache.zookeeper.KeeperException;
    +import org.apache.zookeeper.KeeperException.Code;
    +import org.apache.zookeeper.Op;
    +import org.apache.zookeeper.PortAssignment;
    +import org.apache.zookeeper.ZKTestCase;
    +import org.apache.zookeeper.ZKUtil;
    +import org.apache.zookeeper.ZooDefs;
    +import org.apache.zookeeper.ZooKeeper;
    +import org.apache.zookeeper.data.ACL;
    +import org.apache.zookeeper.data.Stat;
    +import org.apache.zookeeper.server.Request;
    +import org.apache.zookeeper.server.ServerCnxn;
    +import org.apache.zookeeper.server.quorum.QuorumPeerTestBase.MainThread;
    +import org.apache.zookeeper.test.ClientBase;
    +import org.apache.zookeeper.test.ClientBase.CountdownWatcher;
    +import org.junit.After;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ZKAuditLoggerTest extends ZKTestCase {
    +    private static final Logger LOG = Logger.getLogger(ZKAuditLoggerTest.class);
    +    private static int SERVER_COUNT = 3;
    +    private MainThread[] mt;
    +    private ZooKeeper zk;
    +
    +    @Test
    +    public void testAuditLogs() throws Exception {
    --- End diff --
    
    I really like this integration test. I think it would be great if it was a little more rigorous. 
    
    Would it be possible to use multiple auth providers oruse multiple servers and make sure logs are only appearing on the correct server? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136157705
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    --- End diff --
    
    you are right.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136168432
  
    --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java ---
    @@ -54,6 +54,7 @@
          */
         static final ByteBuffer closeConn = ByteBuffer.allocate(0);
     
    +    private static String loginUser = System.getProperty("user.name", "<NA>");
    --- End diff --
    
    loginUser is the user who started zk server. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344574
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    --- End diff --
    
    "captures detailed information"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135346005
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    +    <para>By default there are only four authentication provider</para>
    --- End diff --
    
    "providers"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135343934
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, RFAAUDIT
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    --- End diff --
    
    Why don't we use the same conversion pattern as the other log types?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135362550
  
    --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java ---
    @@ -54,6 +54,7 @@
          */
         static final ByteBuffer closeConn = ByteBuffer.allocate(0);
     
    +    private static String loginUser = System.getProperty("user.name", "<NA>");
    --- End diff --
    
    Not sure I understand what this represents


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by pmoust <gi...@git.apache.org>.
Github user pmoust commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    @arshadmohammad I'd also be curious as to the perf implications of this, have you run a load test-suite against this?


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r138181648
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
    @@ -0,0 +1,37 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +public class AuditConstants {
    +    public static final String SUCCESS = "success";
    +    public static final String FAILURE = "failure";
    +    // operation is performed, result is not known yet
    +    public static final String INVOKED = "invoked";
    +    public static final String KEY_VAL_SEPARATOR = "=";
    +    public static final char PAIR_SEPARATOR = '\t';
    +
    +    public static final String OP_START = "serverStart";
    --- End diff --
    
    I think we should not expose the internal op codes, etc to end users, through API or through the public constant definition included in this patch. We can get rid of the constants definitions here and instead, whenever we want to audit log something we create the operation string on site and directly pass the string to the audit event function call. Something like:
    `case OpCode.create:
          subResult = new CreateResult(subTxnResult.path);
          addSuccessAudit(request, cnxn, "create", subTxnResult.path);
          break;
    `
    This approach seems make the maintain overhead low as we don't have to worry about the correlations between various op codes definitions and the only catch is we might have to duplicate the op code string in different places if we audit logging same thing in different places - but in this case it seems the cost of duplication is low comparing to maintaining explicit op constants. 
    
    I also checked HDFS code base and it took a similar approach.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344479
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    --- End diff --
    
    "where a client is connected as depicted below" (no need for figure)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136166969
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
             return request.cnxn == null;
         }
     
    +    private String getHostAddress(ServerCnxn cnxn) {
    +        try {
    +            if (cnxn == null) {
    +                return "";
    +            }
    +            InetSocketAddress remoteSocketAddress = cnxn
    +                    .getRemoteSocketAddress();
    +            if (remoteSocketAddress == null) {
    +                return "";
    +            }
    +            InetAddress address = remoteSocketAddress.getAddress();
    +            if (address == null) {
    +                return "";
    +            }
    +            return address.getHostAddress();
    +        } catch (Exception e) {
    +            // ignore
    +        }
    +        return "";
    +    }
    +
    +    private String getSessionId(ServerCnxn cnxn) {
    --- End diff --
    
    moved to ServerCnxn 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135362695
  
    --- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java ---
    @@ -0,0 +1,58 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.server.util;
    +
    +import static org.junit.Assert.*;
    --- End diff --
    
    nit: avoid * imports


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135347188
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
    @@ -0,0 +1,37 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +public class AuditConstants {
    +    public static final String SUCCESS = "success";
    +    public static final String FAILURE = "failure";
    +    // operation is performed, result is not known yet
    +    public static final String INVOKED = "invoked";
    +    public static final String KEY_VAL_SEPARATOR = "=";
    +    public static final char PAIR_SEPARATOR = '\t';
    +
    +    public static final String OP_START = "serverStart";
    --- End diff --
    
    I'm concerned about having another listing of ZooKeeper operations. Can we possibly do something in ZooDefs that keeps these string versions of the operations closer to the Opcodes?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r217179296
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +488,127 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    --- End diff --
    
    corrected


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183434909
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +488,127 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    --- End diff --
    
    Typo here


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136162453
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +490,129 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request,ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request,ServerCnxn cnxn, String op, String path, String acl) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl,
    +                getSessionId(cnxn), getHostAddress(cnxn));
    +    }
    +
    +    private void addFailureAudit(Request request,ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request,ServerCnxn cnxn, String op, String path, String acl) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl,
    +                getSessionId(cnxn), getHostAddress(cnxn));
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, getSessionId(cnxn),
    +                    getHostAddress(cnxn));
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, getSessionId(cnxn),
    +                    getHostAddress(cnxn));
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    +        if (ZKAuditLogger.isAuditDisabled) {
    +            return;
    +        }
    +        String op = AuditConstants.OP_CREATE;
    +        if (request.cnxn == null) {
    +            return;
    +        }
    +        String path=null;
    +        long sessionId = -1;
    +        String address = null;
    +        String acls = null;
    +        boolean exceptionOccured = false;
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        try {
    +            sessionId = request.cnxn.getSessionId();
    +            switch (request.type) {
    +            case OpCode.create:
    +            case  OpCode.create2:
    +            case  OpCode.createContainer:
    +                op = AuditConstants.OP_CREATE;
    +                CreateRequest createRequest = new CreateRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, createRequest);
    +                path=createRequest.getPath();
    +                break;
    +            case OpCode.delete:
    +            case OpCode.deleteContainer:
    +                op = AuditConstants.OP_DELETE;
    +                //path = new String(request.request.array());
    +                DeleteRequest deleteRequest = new DeleteRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, deleteRequest);
    +                path=deleteRequest.getPath();
    +                break;
    +            case OpCode.setData:
    +                op = AuditConstants.OP_SETDATA;
    +                SetDataRequest setDataRequest = new SetDataRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, setDataRequest);
    +                path=setDataRequest.getPath();
    +                break;
    +            case OpCode.setACL:
    +                op = AuditConstants.OP_SETACL;
    +                SetACLRequest setACLRequest = new SetACLRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +                path=setACLRequest.getPath();
    +                acls = ZKUtil.aclToString(setACLRequest.getAcl());
    +                break;
    +            case OpCode.multi:
    +                op = AuditConstants.OP_MULTI_OP;
    +                break;
    +            case OpCode.reconfig:
    +                op = AuditConstants.OP_RECONFIG;
    +                break;
    +            }
    +            if (request.cnxn != null
    +                    && request.cnxn.getRemoteSocketAddress() != null
    +                    && request.cnxn.getRemoteSocketAddress().getAddress() != null) {
    +                address = request.cnxn.getRemoteSocketAddress().getAddress()
    +                        .getHostAddress();
    +            }
    +        } catch (Throwable e) {
    +            exceptionOccured = true;
    +            LOG.error("Failed to audit log request {} failure", request.type, e);
    +        }
    +        if (!exceptionOccured) {
    +            if (ZKAuditLogger.isAuditEnabled) {
    --- End diff --
    
    corrected, have a look.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136144283
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, RFAAUDIT
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    --- End diff --
    
    It is very valid question. By RFAAUDIT i meant Rolling File Audit. Also one A is extra by mistake. I think it is better to change the name to AUDITFILE.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r218067686
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, AUDITFILE
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    --- End diff --
    
     log4j.additivity.org.apache.zookeeper.audit = false  is superset configuration of log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false 
    As all the all audit logging is done through ZKAuditLogger it is ok to configure only this


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183433194
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -250,30 +267,36 @@ public void processRequest(Request request) {
                     lastOp = "CREA";
                     rsp = new Create2Response(rc.path, rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_CREATE, rc.path, null, err);
                     break;
                 }
                 case OpCode.delete:
                 case OpCode.deleteContainer: {
                     lastOp = "DELE";
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_DELETE, rc.path, null, err);
                     break;
                 }
                 case OpCode.setData: {
                     lastOp = "SETD";
                     rsp = new SetDataResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETDATA, rc.path, null, err);
                     break;
                 }
                 case OpCode.reconfig: {
                     lastOp = "RECO";
                     rsp = new GetDataResponse(((QuorumZooKeeperServer)zks).self.getQuorumVerifier().toString().getBytes(), rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_RECONFIG, rc.path, null, err);
                     break;
                 }
                 case OpCode.setACL: {
                     lastOp = "SETA";
                     rsp = new SetACLResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETACL, rc.path, getACLs(request),
    --- End diff --
    
    I believe this approach has some performance impact even if audit logging is disabled. The flag gets checked within the method, therefore `getACLs()` will be evaluated even if there's no need for the result.
    Passing only `request` and calling `getACLs()` from `addAuditLog()` would be slightly faster.  


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134382180
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
    @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888</programlisting>
                   feature. Default is "true"</para>
                 </listitem>
               </varlistentry>
    -
    +          <varlistentry>
    +            <term>audit.enabled</term>
    +            <listitem>
    +                <para>(Java system property:
    +                    <emphasis role="bold">zookeeper.audit.enabled</emphasis>)
    +                </para>
    +                <para>
    +                    <emphasis role="bold">New in 3.5.3:</emphasis>
    --- End diff --
    
    This should be 3.5.4 at least. We've shipped 3.5.3 already :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136865979
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/AuditConstants.java ---
    @@ -0,0 +1,37 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +public class AuditConstants {
    +    public static final String SUCCESS = "success";
    +    public static final String FAILURE = "failure";
    +    // operation is performed, result is not known yet
    +    public static final String INVOKED = "invoked";
    +    public static final String KEY_VAL_SEPARATOR = "=";
    +    public static final char PAIR_SEPARATOR = '\t';
    +
    +    public static final String OP_START = "serverStart";
    --- End diff --
    
    - Operation names are something internal to ZooKeeper server. Shall we expose this end user through API?  I am not sure. I have tried to segregate operation names though a different class. Please have a look. 
    
    - org.apache.zookeeper.ZooDefs.opNames is exposing operation names, this operation name list is not complete. Also it  has operation names which do not exist like getMaxChildren and setMaxChildren. I have no idea why operation name should be exposed and how it is used.  Any idea on this ?


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136154783
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    --- End diff --
    
    corrected


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136166929
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
             return request.cnxn == null;
         }
     
    +    private String getHostAddress(ServerCnxn cnxn) {
    --- End diff --
    
    moved


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344230
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
    @@ -931,7 +931,19 @@ server.3=zoo3:2888:3888</programlisting>
                   feature. Default is "true"</para>
                 </listitem>
               </varlistentry>
    -
    +          <varlistentry>
    +            <term>audit.enabled</term>
    +            <listitem>
    +                <para>(Java system property:
    +                    <emphasis role="bold">zookeeper.audit.enabled</emphasis>)
    +                </para>
    +                <para>
    +                    <emphasis role="bold">New in 3.5.4:</emphasis>
    +                    By default audit logs are disabled. Set to "true" to enable this feature. Default is "false". See
    --- End diff --
    
    nit: We don't need to tell the user what the default is twice.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136866016
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    --- End diff --
    
    corrected.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344860
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    --- End diff --
    
    nit: We should consistently capitalize the first letter of the first word in these descriptions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r138179709
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, AUDITFILE
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    --- End diff --
    
    I think something like this would be better here (see how HDFS defines audit log properties for example).
    log4j.additivity.org.apache.zookeeper.audit = false
    or
    log4j.additivity.org.apache.zookeeper.audit.enabled = false


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344048
  
    --- Diff: src/docs/src/documentation/content/xdocs/index.xml ---
    @@ -66,6 +66,7 @@
           <li><a href="zookeeperHierarchicalQuorums.html">Hierarchical quorums</a></li>
           <li><a href="zookeeperObservers.html">Observers</a> - non-voting ensemble members that easily improve ZooKeeper's scalability</li>
           <li><a href="zookeeperReconfig.html">Dynamic Reconfiguration</a> - a guide on how to use dynamic reconfiguration in ZooKeeper</li>
    +      <li><a href="zookeeperAuditLogs.html">Audit Logging</a> - a guide on how to configure audit logs in ZooKeeper Server and what contents are logged.</li>
    --- End diff --
    
    nit: I don't think we need the "and what contents are logged."


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135361808
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -476,6 +624,33 @@ private boolean connClosedByClient(Request request) {
             return request.cnxn == null;
         }
     
    +    private String getHostAddress(ServerCnxn cnxn) {
    --- End diff --
    
    perhaps we could do this logic in ServerCnxn?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136161116
  
    --- Diff: src/java/main/org/apache/zookeeper/audit/ZKAuditLogger.java ---
    @@ -0,0 +1,118 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import org.apache.zookeeper.server.ServerCnxnFactory;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +public class ZKAuditLogger {
    +    public static final String SYSPROP_AUDIT_ENABLED = "zookeeper.audit.enabled";
    +    private static final Logger LOG = LoggerFactory.getLogger(ZKAuditLogger.class);
    +    // By default audit logging is disabled
    +    public static final boolean isAuditEnabled = Boolean.getBoolean(SYSPROP_AUDIT_ENABLED);
    +    public static final boolean isAuditDisabled = !isAuditEnabled;
    --- End diff --
    
    not a problem, we can use isAuditEnabled itself, removed isAuditDisabled 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136169721
  
    --- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java ---
    @@ -0,0 +1,58 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.server.util;
    +
    +import static org.junit.Assert.*;
    +
    +import org.apache.zookeeper.data.Id;
    +import org.junit.Test;
    +
    +public class AuthUtilTest {
    +
    +    @Test
    +    public void testGetUserFromAllAuthenticationScheme() {
    +        System.setProperty("zookeeper.authProvider.sasl",
    --- End diff --
    
    only digest authentication is different, rest all are same. That is why not needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344338
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    --- End diff --
    
    "from version"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    @arshadmohammad Audit logging, when enabled, will for sure degrade performance as it adds additional IO cost. Have you done any sort of performance evaluation so we can roughly get an idea of what kind of performance expectation we should have when audit logging is enabled? 
    
    Depends on how the performance degraded on different use cases, you might want to consider adding an async version of audit logging otherwise this feature might not be usable (note I am just guessing on the perf numbers here). Similar case happened on HDFS where an async audit logging was added to avoid degrading performance on hotspots.


---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by pmoust <gi...@git.apache.org>.
Github user pmoust commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    @arshadmohammad hm - they don't seem to be any(?)


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r217179715
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +488,127 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        String op = AuditConstants.OP_CREATE;
    +        String path=null;
    +        long sessionId = -1;
    +        String address = null;
    +        String acls = null;
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        try {
    +            sessionId = request.cnxn.getSessionId();
    +            switch (request.type) {
    +            case OpCode.create:
    +            case  OpCode.create2:
    +            case  OpCode.createContainer:
    +                op = AuditConstants.OP_CREATE;
    +                CreateRequest createRequest = new CreateRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, createRequest);
    +                path=createRequest.getPath();
    +                break;
    +            case OpCode.delete:
    +            case OpCode.deleteContainer:
    +                op = AuditConstants.OP_DELETE;
    +                //path = new String(request.request.array());
    --- End diff --
    
    removed


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135345935
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    --- End diff --
    
    I'm still a little confused by what is meant by "taken as user". If I'm understanding correctly, it may be a little clearer to say "How is the user determined for the audit logs?"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135345461
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    --- End diff --
    
    It's a little concerning to repeat this code in two places and things could get out of sync if this code is changed. Do we really need to have it in the documentation?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136865485
  
    --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java ---
    @@ -54,6 +54,7 @@
          */
         static final ByteBuffer closeConn = ByteBuffer.allocate(0);
     
    +    private static String loginUser = System.getProperty("user.name", "<NA>");
    --- End diff --
    
    changed to serverUser


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135362745
  
    --- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java ---
    @@ -0,0 +1,58 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.server.util;
    +
    +import static org.junit.Assert.*;
    +
    +import org.apache.zookeeper.data.Id;
    +import org.junit.Test;
    +
    +public class AuthUtilTest {
    +
    +    @Test
    +    public void testGetUserFromAllAuthenticationScheme() {
    +        System.setProperty("zookeeper.authProvider.sasl",
    --- End diff --
    
    Do we have/need a test for custom authentication schemes?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134382249
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.3. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys</para>
    --- End diff --
    
    Nit: missing full stop after `following keys`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136172475
  
    --- Diff: src/java/test/org/apache/zookeeper/audit/ZKAuditLoggerTest.java ---
    @@ -0,0 +1,377 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.audit;
    +
    +import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
    +import static org.junit.Assert.assertEquals;
    +
    +import java.io.ByteArrayOutputStream;
    +import java.io.IOException;
    +import java.io.LineNumberReader;
    +import java.io.StringReader;
    +import java.net.InetAddress;
    +import java.net.InetSocketAddress;
    +import java.util.ArrayList;
    +import java.util.List;
    +
    +import org.apache.log4j.Layout;
    +import org.apache.log4j.Level;
    +import org.apache.log4j.Logger;
    +import org.apache.log4j.SimpleLayout;
    +import org.apache.log4j.WriterAppender;
    +import org.apache.zookeeper.CreateMode;
    +import org.apache.zookeeper.KeeperException;
    +import org.apache.zookeeper.KeeperException.Code;
    +import org.apache.zookeeper.Op;
    +import org.apache.zookeeper.PortAssignment;
    +import org.apache.zookeeper.ZKTestCase;
    +import org.apache.zookeeper.ZKUtil;
    +import org.apache.zookeeper.ZooDefs;
    +import org.apache.zookeeper.ZooKeeper;
    +import org.apache.zookeeper.data.ACL;
    +import org.apache.zookeeper.data.Stat;
    +import org.apache.zookeeper.server.Request;
    +import org.apache.zookeeper.server.ServerCnxn;
    +import org.apache.zookeeper.server.quorum.QuorumPeerTestBase.MainThread;
    +import org.apache.zookeeper.test.ClientBase;
    +import org.apache.zookeeper.test.ClientBase.CountdownWatcher;
    +import org.junit.After;
    +import org.junit.Assert;
    +import org.junit.Test;
    +
    +public class ZKAuditLoggerTest extends ZKTestCase {
    +    private static final Logger LOG = Logger.getLogger(ZKAuditLoggerTest.class);
    +    private static int SERVER_COUNT = 3;
    +    private MainThread[] mt;
    +    private ZooKeeper zk;
    +
    +    @Test
    +    public void testAuditLogs() throws Exception {
    --- End diff --
    
    Authentication provider make difference only for user attribute, rest all audit log attributes will remain same. 
    Test is already running on 3 node cluster. It is validating number of audit entries but in total. Not on which server. In all the cases expected number of entry is either 1 or three. In case of 1 entry test case currently not checking on which specific server entry is expected. But I think it is fine.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183430034
  
    --- Diff: src/java/main/org/apache/zookeeper/ZKUtil.java ---
    @@ -168,4 +169,41 @@ private static void visitSubTreeDFSHelper(ZooKeeper zk, final String path,
                 return; // ignore
             }
         }
    +
    +    /**
    +     * @param perms
    +     *            ACL permissions
    +     * @return string representation of permissions
    +     */
    +    public static String getPermString(int perms) {
    --- End diff --
    
    Have you considered caching these values in some way?
    I see 2 options to make this faster:
    1. dynamic caching of already generated values,
    2. static caching of all possible values in a static hashmap (32 possible values if I'm not mistaken)


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135344750
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    --- End diff --
    
    "Comma separated list of users associated with a client session." 
    
    I'm not sure I understand the second sentence, can you clarify?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135346090
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    +                        for serverStop operation because stop is logged before ensuring that server actually stopped.
    +                    </entry>
    +                </row>
    +            </tbody>
    +        </tgroup>
    +    </table>
    +    <para>Below are sample audit logs for all operations, where client is connected from 192.168.1.2, client principal is
    +        zkcli@HADOOP.COM, server principal is zookeeper/192.168.1.3@HADOOP.COM</para>
    +    <programlisting>
    +        user=zookeeper/192.168.1.3 operation=serverStart   result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/a    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setAcl    znode=/a    acl=world:anyone:cdrwa  result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=setData   znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/b    result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=multiOperation    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=failure
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=delete    znode=/a    result=success
    +        session=0x19344730001   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=create    znode=/ephemral result=success
    +        session=0x19344730001   user=zookeeper/192.168.1.3   operation=ephemeralZNodeDeletionOnSessionCloseOrExpire  znode=/ephemral result=success
    +        session=0x19344730000   user=192.168.1.2,zkcli@HADOOP.COM  ip=192.168.1.2    operation=reconfig  znode=/zookeeper/config result=success
    +        user=zookeeper/192.168.1.3 operation=serverStop    result=invoked
    +    </programlisting>
    +  </section>
    +  <section id="ch_auditConfig">
    +    <title>ZooKeeper Audit Log Configuration</title>
    +    <para>By default audit logs are disabled. To enable audit logs configure audit.enable=true in conf/zoo.cfg. Audit
    +        logging is done using log4j. Following is the default log4j configuration for audit logs in conf/log4j.properties
    +    </para>
    +    <programlisting>
    +        #
    +        # zk audit logging
    +        #
    +        zookeeper.auditlog.file=zookeeper_audit.log
    +        zookeeper.auditlog.threshold=INFO
    +        audit.logger=INFO, RFAAUDIT
    +        log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +        log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +        log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +        log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +        log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +        log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    +        log4j.appender.RFAAUDIT.Threshold=${zookeeper.auditlog.threshold}
    +
    +        # Max log file size of 10MB
    +        log4j.appender.RFAAUDIT.MaxFileSize=10MB
    +        log4j.appender.RFAAUDIT.MaxBackupIndex=10
    +    </programlisting>
    +    <para>Change above configuration to customize the auditlog file, number of backups, max file size etc.</para>
    +  </section>
    +  <section id="ch_zkAuditUser">
    +    <title>Who is taken as user in audit logs?</title>
    +    <para>By default there are only four authentication provider</para>
    +    <itemizedlist>
    +        <listitem>
    +            <para>IPAuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>SASLAuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>X509AuthenticationProvider</para>
    +        </listitem>
    +        <listitem>
    +            <para>DigestAuthenticationProvider</para>
    +        </listitem>
    +    </itemizedlist>
    +    <para>User is decided based on the configured authentication provider.</para>
    --- End diff --
    
    "The user is determined"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r135345079
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.4. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys.</para>
    +    <table>
    +        <title>Audit Log Content</title>
    +        <tgroup cols="5" align="left" colsep="1" rowsep="4">
    +            <thead>
    +                <row>
    +                    <entry>Key</entry>
    +                    <entry>Value</entry>
    +                </row>
    +            </thead>
    +            <tbody>
    +                <row>
    +                    <entry>session</entry>
    +                    <entry>client session id</entry>
    +                </row>
    +                <row>
    +                    <entry>user</entry>
    +                    <entry>
    +                        comma separated list of users who are associate with a client session. To know who is taken as user in audit logs
    +                        refer section
    +                        <xref linkend="ch_zkAuditUser"/>
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>ip</entry>
    +                    <entry>client IP address</entry>
    +                </row>
    +                <row>
    +                    <entry>operation</entry>
    +                    <entry>any one of the selected operations for audit. Possible values are
    +                        (serverStart| serverStop| create| delete| setData| setAcl| multiOperation| reconfig| ephemeralZNodeDeleteOnSessionClose)
    +                    </entry>
    +                </row>
    +                <row>
    +                    <entry>znode</entry>
    +                    <entry>path of the znode</entry>
    +                </row>
    +                <row>
    +                    <entry>acl</entry>
    +                    <entry>String representation of znode ACL like cdrwa(create, delete,read, write, admin). This is logged
    +                        only for setAcl operation</entry>
    +                </row>
    +                <row>
    +                    <entry>result</entry>
    +                    <entry>result of the operation. Possible values are (success|failure|invoked). Result "invoked" is used
    --- End diff --
    
    "is only used" may be clearer (if correct)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136146630
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, RFAAUDIT
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    +log4j.appender.RFAAUDIT.File=${zookeeper.log.dir}/${zookeeper.auditlog.file}
    +log4j.appender.RFAAUDIT.layout=org.apache.log4j.PatternLayout
    +log4j.appender.RFAAUDIT.layout.ConversionPattern=%d{ISO8601} %p %c{2}: %m%n
    --- End diff --
    
    Logging is done only from one class, which thread is logging is not important. so only required parameters are logged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r134566240
  
    --- Diff: src/docs/src/documentation/content/xdocs/zookeeperAuditLogs.xml ---
    @@ -0,0 +1,205 @@
    +<?xml version="1.0" encoding="UTF-8"?>
    +<!--
    +  Copyright 2002-2004 The Apache Software Foundation
    +
    +  Licensed under the Apache License, Version 2.0 (the "License");
    +  you may not use this file except in compliance with the License.
    +  You may obtain a copy of the License at
    +
    +      http://www.apache.org/licenses/LICENSE-2.0
    +
    +  Unless required by applicable law or agreed to in writing, software
    +  distributed under the License is distributed on an "AS IS" BASIS,
    +  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +  See the License for the specific language governing permissions and
    +  limitations under the License.
    +-->
    +<!DOCTYPE article PUBLIC "-//OASIS//DTD Simplified DocBook XML V1.0//EN"
    +"http://www.oasis-open.org/docbook/xml/simple/1.0/sdocbook.dtd">
    +<article id="ar_auditLogs">
    +  <title>ZooKeeper Audit Logging</title>
    +  <articleinfo>
    +    <legalnotice>
    +      <para>Licensed under the Apache License, Version 2.0 (the "License");
    +      you may not use this file except in compliance with the License. You may
    +      obtain a copy of the License at <ulink
    +      url="http://www.apache.org/licenses/LICENSE-2.0">http://www.apache.org/licenses/LICENSE-2.0</ulink>.</para>
    +
    +      <para>Unless required by applicable law or agreed to in writing,
    +      software distributed under the License is distributed on an "AS IS"
    +      BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
    +      implied. See the License for the specific language governing permissions
    +      and limitations under the License.</para>
    +    </legalnotice>
    +
    +    <abstract>
    +        <para>This document contains information about Audit Logs in ZooKeeper.</para>
    +    </abstract>
    +  </articleinfo>
    +  <section id="ch_auditLogs">
    +    <title>ZooKeeper Audit Logs</title>
    +    <para>Apache ZooKeeper supports audit logs form version 3.5.3. By default audit logs are disabled. To enable audit
    +    logs configure audit.enable=true in conf/zoo.cfg. Audit logs are not logged on all the ZooKeeper servers, but logged
    +    only on the servers where client is connected as depicted in bellow figure.</para>
    +    <mediaobject id="fg_audit" >
    +        <imageobject>
    +            <imagedata fileref="images/zkAuditLogs.jpg"/>
    +        </imageobject>
    +    </mediaobject>
    +    <para>The audit log captures the detailed information for the operations that are selected to be audited. The audit
    +    information is written as a set of key=value pairs for the following keys</para>
    --- End diff --
    
    Corrected it


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r217179236
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -250,30 +267,36 @@ public void processRequest(Request request) {
                     lastOp = "CREA";
                     rsp = new Create2Response(rc.path, rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_CREATE, rc.path, null, err);
                     break;
                 }
                 case OpCode.delete:
                 case OpCode.deleteContainer: {
                     lastOp = "DELE";
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_DELETE, rc.path, null, err);
                     break;
                 }
                 case OpCode.setData: {
                     lastOp = "SETD";
                     rsp = new SetDataResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETDATA, rc.path, null, err);
                     break;
                 }
                 case OpCode.reconfig: {
                     lastOp = "RECO";
                     rsp = new GetDataResponse(((QuorumZooKeeperServer)zks).self.getQuorumVerifier().toString().getBytes(), rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_RECONFIG, rc.path, null, err);
                     break;
                 }
                 case OpCode.setACL: {
                     lastOp = "SETA";
                     rsp = new SetACLResponse(rc.stat);
                     err = Code.get(rc.err);
    +                addAuditLog(request, cnxn, AuditConstants.OP_SETACL, rc.path, getACLs(request),
    --- End diff --
    
    corrected, added one more check at the before making audit log itself
    if (ZKAuditLogger.isAuditEnabled()) {
                        addAuditLog(request, cnxn, AuditConstants.OP_SETACL, rc.path, getACLs(request), err);
                    }


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136660599
  
    --- Diff: src/java/main/org/apache/zookeeper/server/ServerCnxnFactory.java ---
    @@ -54,6 +54,7 @@
          */
         static final ByteBuffer closeConn = ByteBuffer.allocate(0);
     
    +    private static String loginUser = System.getProperty("user.name", "<NA>");
    --- End diff --
    
    Perhaps this can be renamed? Maybe "serverUser" or something to clarify this is not the same thing as the user "logged in" on the client. Or at least a comment.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r138180155
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, AUDITFILE
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.AUDITFILE=org.apache.log4j.RollingFileAppender
    --- End diff --
    
    How is the rolling of the audit log file defined? Is it controlled by the `log4j.appender.AUDITFILE.Threshold` property?
    
    Should we also add something like DailyRollingFileAppender and a date pattern property as another alternative for rolling logs?


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r183435384
  
    --- Diff: src/java/main/org/apache/zookeeper/server/FinalRequestProcessor.java ---
    @@ -465,6 +488,127 @@ public void processRequest(Request request) {
             }
         }
     
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addSuccessAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addSuccessAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path) {
    +        addFailureAudit(request, cnxn, op, path, null);
    +    }
    +
    +    private void addFailureAudit(Request request, ServerCnxn cnxn, String op, String path,
    +            String acl) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                cnxn.getHostAddress());
    +    }
    +
    +    private void addAuditLog(Request request, ServerCnxn cnxn, String op, String path, String acl,
    +            Code err) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        if (err == Code.OK) {
    +            ZKAuditLogger.logSuccess(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        } else {
    +            ZKAuditLogger.logFailure(request.getUsers(), op, path, acl, cnxn.getSessionIdHex(),
    +                    cnxn.getHostAddress());
    +        }
    +    }
    +
    +    private String getACLs(Request request)
    +    {
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        SetACLRequest setACLRequest = new SetACLRequest();
    +        try {
    +            ByteBufferInputStream.byteBuffer2Record(reqData, setACLRequest);
    +        } catch (IOException e) {
    +            e.printStackTrace();
    +        }
    +        return ZKUtil.aclToString(setACLRequest.getAcl());
    +    }
    +
    +    private void addFailedTxnAduitLog(Request request) {
    +        if (!ZKAuditLogger.isAuditEnabled) {
    +            return;
    +        }
    +        String op = AuditConstants.OP_CREATE;
    +        String path=null;
    +        long sessionId = -1;
    +        String address = null;
    +        String acls = null;
    +        ByteBuffer reqData = request.request.duplicate();
    +        reqData.rewind();
    +        try {
    +            sessionId = request.cnxn.getSessionId();
    +            switch (request.type) {
    +            case OpCode.create:
    +            case  OpCode.create2:
    +            case  OpCode.createContainer:
    +                op = AuditConstants.OP_CREATE;
    +                CreateRequest createRequest = new CreateRequest();
    +                ByteBufferInputStream.byteBuffer2Record(reqData, createRequest);
    +                path=createRequest.getPath();
    +                break;
    +            case OpCode.delete:
    +            case OpCode.deleteContainer:
    +                op = AuditConstants.OP_DELETE;
    +                //path = new String(request.request.array());
    --- End diff --
    
    Please the commented line.


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r218068308
  
    --- Diff: conf/log4j.properties ---
    @@ -63,3 +63,20 @@ log4j.appender.TRACEFILE.File=${zookeeper.tracelog.dir}/${zookeeper.tracelog.fil
     log4j.appender.TRACEFILE.layout=org.apache.log4j.PatternLayout
     ### Notice we are including log4j's NDC here (%x)
     log4j.appender.TRACEFILE.layout.ConversionPattern=%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L][%x] - %m%n
    +#
    +# zk audit logging
    +#
    +zookeeper.auditlog.file=zookeeper_audit.log
    +zookeeper.auditlog.threshold=INFO
    +audit.logger=INFO, AUDITFILE
    +log4j.logger.org.apache.zookeeper.audit.ZKAuditLogger=${audit.logger}
    +log4j.additivity.org.apache.zookeeper.audit.ZKAuditLogger=false
    +log4j.appender.AUDITFILE=org.apache.log4j.RollingFileAppender
    --- End diff --
    
    rolling of the audit log file is defined by following properties
    log4j.appender.RFAAUDIT=org.apache.log4j.RollingFileAppender
    # Max log file size of 10MB
    log4j.appender.RFAAUDIT.MaxFileSize=10MB
    log4j.appender.RFAAUDIT.MaxBackupIndex=10


---

[GitHub] zookeeper pull request #338: ZOOKEEPER-1260:Audit logging in ZooKeeper serve...

Posted by arshadmohammad <gi...@git.apache.org>.
Github user arshadmohammad commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/338#discussion_r136175120
  
    --- Diff: src/java/test/org/apache/zookeeper/server/util/AuthUtilTest.java ---
    @@ -0,0 +1,58 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.zookeeper.server.util;
    +
    +import static org.junit.Assert.*;
    --- End diff --
    
    Done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #338: ZOOKEEPER-1260:Audit logging in ZooKeeper servers.

Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:

    https://github.com/apache/zookeeper/pull/338
  
    @arshadmohammad Can we move on with this great new feature?


---