Skip to content

Standardise Error Logging - #13608

Merged
adfoster-r7 merged 1 commit into
rapid7:masterfrom
agalway-r7:standardise-elog
Jun 23, 2020
Merged

Standardise Error Logging#13608
adfoster-r7 merged 1 commit into
rapid7:masterfrom
agalway-r7:standardise-elog

Conversation

@agalway-r7

@agalway-r7 agalway-r7 commented Jun 12, 2020

Copy link
Copy Markdown
Contributor

Important changes are in logging/log_dispatcher.rb, everything else is just cleanup.

This PR attempts to standardise elog usage by restricting it's API to three variables; a message, a source, and an Exception.

  • msg is simple string. If this is the only variable passed to elog, no additional parsing is performed.
  • src dictates the LogSource. This is most commonly core but can also be hdwbridge and Meterpreter
  • error is the exception that caused the error, if any. The .class & .message values of an exception are always extracted and added to the log message. If the global log level is >= 1, the the Exception .backtrace is also added.

Standardizing error logging will allow us to send error logs to framework.log and improve functionality of the debug command #13430.

Happy Friday! One of the files I cleaned up has an Easter Egg in it, first one to find it gets a prize.

method_vars_simp
method_vars_blind_simp

space-r7
space-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/core/auxiliary/scanner.rb Outdated
Comment thread lib/msf/core/db_manager.rb Outdated
Comment thread modules/exploits/windows/http/geutebrueck_gcore_x64_rce_bo.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/core/db_manager/migration.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/core/module/platform.rb
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/ui/console/command_dispatcher/core.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/ui/console/command_dispatcher/db.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/msf/ui/console/module_command_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/json_hash_file.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/post/hwbridge/ui/console/command_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/post/hwbridge/ui/console/command_dispatcher/automotive.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/post/meterpreter/packet_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread lib/rex/post/meterpreter/packet_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 12, 2020
View reviewed changes
Comment thread modules/exploits/windows/local/alpc_taskscheduler.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 15, 2020
View reviewed changes
Comment thread modules/exploits/windows/fileformat/adobe_pdf_embedded_exe.rb
dwelch-r7
dwelch-r7 reviewed Jun 15, 2020
View reviewed changes
Comment thread lib/rex/post/hwbridge/ui/console/command_dispatcher.rb Outdated
dwelch-r7
dwelch-r7 reviewed Jun 15, 2020
View reviewed changes
Comment thread lib/rex/post/meterpreter/ui/console/command_dispatcher/core.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 15, 2020
View reviewed changes
Comment thread lib/rex/post/meterpreter/ui/console.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 16, 2020
View reviewed changes
Comment thread lib/msf/base/simple/post.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb Outdated

global_log_level = get_log_level(src)

# If the source has no associated log_level, the default log level is used

@adfoster-r7 adfoster-r7 Jun 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In what scenario does this code path occur?

@agalway-r7 agalway-r7 Jun 16, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If the LogDispatcher doesn't have a source set, the above src will eventually reach this line, where src isn't a key in log_levels and nil is returned.

Comment thread lib/rex/logging/log_dispatcher.rb Outdated
Comment thread lib/rex/post/meterpreter/ui/console/command_dispatcher/core.rb Outdated
Comment thread lib/rex/proto/http/response.rb Outdated
Comment thread modules/auxiliary/admin/scada/phoenix_command.rb Outdated
Comment thread modules/exploits/windows/local/ntapphelpcachecontrol.rb Outdated
Comment thread modules/post/multi/manage/zip.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 16, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 16, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 17, 2020
View reviewed changes
Comment thread modules/auxiliary/dos/misc/ibm_tsm_dos.rb Outdated
adfoster-r7
adfoster-r7 reviewed Jun 17, 2020
View reviewed changes
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
@agalway-r7
agalway-r7 force-pushed the standardise-elog branch 3 times, most recently from 68afe85 to fd2f10e Compare June 17, 2020 12:21
@agalway-r7 agalway-r7 mentioned this pull request Jun 17, 2020
Merged
acammack-r7
acammack-r7 previously requested changes Jun 18, 2020
View reviewed changes

@acammack-r7 acammack-r7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good start on fixing up the elog copypasta! There's also a lot of logging in the rex gems to consider as well with this sort of change.

Comment thread lib/metasploit/framework/data_service/remote/http/response_data_helper.rb Outdated
Comment thread lib/msf/core/modules/metadata/store.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb
adfoster-r7
adfoster-r7 previously requested changes Jun 19, 2020
View reviewed changes
Comment thread lib/msf/core/exploit/smb/client/psexec.rb Outdated
Comment thread lib/msf/core/framework.rb Outdated
Comment thread lib/rex/logging/log_dispatcher.rb
Comment thread lib/rex/post/meterpreter/ui/console/command_dispatcher/core.rb Outdated
Comment thread modules/auxiliary/dos/misc/ibm_tsm_dos.rb Outdated
Comment thread plugins/msfd.rb Outdated
@agalway-r7
agalway-r7 force-pushed the standardise-elog branch 2 times, most recently from 50b346c to 0cca7e3 Compare June 22, 2020 11:12
@adfoster-r7
adfoster-r7 dismissed stale reviews from acammack-r7 and themself June 22, 2020 11:23

Requests actioned

@adfoster-r7 adfoster-r7 added the rn-enhancement release notes enhancement label Jun 22, 2020
@adfoster-r7
adfoster-r7 merged commit fceb96e into rapid7:master Jun 23, 2020
@adfoster-r7

adfoster-r7 commented Jun 23, 2020 •
edited by tperry-r7
Loading

Copy link
Copy Markdown
Contributor

Release Notes

Updates the Error Logging API to take an error object and updates the existing elog calls within the codebase to use this new API.

@adfoster-r7 adfoster-r7 mentioned this pull request Jul 7, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dwelch-r7 dwelch-r7 dwelch-r7 left review comments
@adfoster-r7 adfoster-r7 adfoster-r7 left review comments
+2 more reviewers
@space-r7 space-r7 space-r7 left review comments
@acammack-r7 acammack-r7 acammack-r7 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement rn-enhancement release notes enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@agalway-r7 @adfoster-r7 @acammack-r7 @dwelch-r7 @space-r7