Add Liota logging formatter to remove newlines #120
Conversation
| @@ -3,6 +3,7 @@ | |||
| "disable_existing_loggers": false, | |||
| "formatters": { | |||
| "simple": { | |||
| "()" : "liota.lib.utilities.log_formatter.LiotaLogFormatter", | |||
There was a problem hiding this comment.
Please verify the fix with the security team once.
| class LiotaLogFormatter(logging.Formatter): | ||
|
|
||
| def format(self, record): | ||
| record.msg = record.msg.replace('\n', '__\\n__').replace('\r', '__\\r__') |
There was a problem hiding this comment.
record.msg.strip() can be explored to be used.
https://docs.python.org/2/library/string.html
There was a problem hiding this comment.
strip() removes leading and trailing characters only. Here, we need to remove the newline characters inserted somewhere in the middle of the message.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
$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.
There was a problem hiding this comment.
- The issue opened by security team mentions only newline character. Will check with security team if any other character can be a problem.
- 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'
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Successfully tested with the following strings and corresponding outputs.
logging.info("test")
logging.info("\ntest2")
logging.info("\ntest3\n3\n3")
logging.info("\nntest4\nn44\r\n")
logging.error("\nntest4\nn44\r\n")
logging.info("test%s%d%n123@")
logging.info("test@1%1{1}")
$ cat liota.log
2017-05-04 18:21:19 PDT 957 INFO [MainThread] root.(1) - test
2017-05-04 18:21:31 PDT 957 INFO [MainThread] root.(1) - \n__test2
2017-05-04 18:21:50 PDT 957 INFO [MainThread] root.(1) - \n__test3\n__3\n__3
2017-05-04 18:22:17 PDT 957 INFO [MainThread] root.(1) - \nntest4\nn44__\r____\n__
2017-05-04 18:22:30 PDT 957 ERROR [MainThread] root.(1) - \nntest4\nn44__\r____\n__
2017-05-04 18:23:31 PDT 957 INFO [MainThread] root.(1) - test%s%d%n123@
2017-05-04 18:24:04 PDT 957 INFO [MainThread] root.(1) - test@1%1{1}
$ cat liota.errors.log
2017-05-04 18:22:30 PDT 957 ERROR [MainThread] root.(1) - \nntest4\nn44__\r____\n__
|
As security team approved the changes merge it. |
No description provided.