Skip to content
This repository has been archived by the owner on Mar 15, 2021. It is now read-only.

Add Liota logging formatter to remove newlines #120

Merged
merged 1 commit into from
May 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/logging.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"disable_existing_loggers": false,
"formatters": {
"simple": {
"()" : "liota.lib.utilities.log_formatter.LiotaLogFormatter",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please verify the fix with the security team once.

"format": "%(asctime)s %(process)d %(levelname)s [%(threadName)s] %(name)s.%(funcName)s(%(lineno)d) - %(message)s",
"datefmt" : "%Y-%m-%d %H:%M:%S %Z"
}
Expand Down
39 changes: 39 additions & 0 deletions liota/lib/utilities/log_formatter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# -*- coding: utf-8 -*-
# ----------------------------------------------------------------------------#
# Copyright © 2015-2016 VMware, Inc. All Rights Reserved. #
# #
# Licensed under the BSD 2-Clause License (the “License”); you may not use #
# this file except in compliance with the License. #
# #
# The BSD 2-Clause License #
# #
# Redistribution and use in source and binary forms, with or without #
# modification, are permitted provided that the following conditions are met:#
# #
# - Redistributions of source code must retain the above copyright notice, #
# this list of conditions and the following disclaimer. #
# #
# - Redistributions in binary form must reproduce the above copyright #
# notice, this list of conditions and the following disclaimer in the #
# documentation and/or other materials provided with the distribution. #
# #
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"#
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE #
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE #
# ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE #
# LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR #
# CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF #
# SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS #
# INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN #
# CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) #
# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF #
# THE POSSIBILITY OF SUCH DAMAGE. #
# ----------------------------------------------------------------------------#
import logging

class LiotaLogFormatter(logging.Formatter):

def format(self, record):
record.msg = record.msg.replace('\n', '__\\n__').replace('\r', '__\\r__')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

record.msg.strip() can be explored to be used.
https://docs.python.org/2/library/string.html

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strip() removes leading and trailing characters only. Here, we need to remove the newline characters inserted somewhere in the middle of the message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strip also takes care of additional forgery including spaces, tabs, newlines and carriage returns. I believe it should be used. We should also check with the security team how to handle extra tabs if forged in between the log messages.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you point me to such documentation of strip? I didn't find it at: https://docs.python.org/2/library/string.html#string.strip.

Also, the security problem here is that, someone can introduce a complete new log message in liota, by putting arguments to the log messages as ...\nSOME_NEW_LOG_MESSAGE\n... and it will not be possible to figure out genuine vs these new logs. We can still check with the security team, nonetheless.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$python
msg = "hello\t"
msg.strip()
'hello'

I think you can try it with code then referring the documentation.

Also, we need to check with the security if there are other characters then newline which might be used for forgery in log messages and required to be removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The issue opened by security team mentions only newline character. Will check with security team if any other character can be a problem.
  2. As I mentioned before, strip() takes care of only leading and trailing characters. Thus, it will not take care of the middle '\n' in the below example, which is the actual problem from security perspective:
    $python
    msg = "\nhello\nworld\n"
    msg.strip()
    'hello\nworld'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is known strip() will take care of leading and trailing characters only. You can apply "replace" operation "post" strip on a string if it is only about handling newline characters.

return super(LiotaLogFormatter, self).format(record)