Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor, external config, output plugins, docker container #27

Merged
merged 19 commits into from
Jun 14, 2019

Conversation

pieterbork
Copy link

Check out these changes and let me know if you'd like me to go back over or redo something.

This would take care of issues #11 and #7 and make it easier to add logging plugins as well as more complex configurations.

@huuck
Copy link
Owner

huuck commented May 13, 2019

Hi!

It looks like binary file send is broken right now (crashes when computing the SHA256 because of encoding issues and content doesn't look right at the end of the wire). I'll take a look into it later this night. Poke me if you find the solution in the meantime.

Kindest regards,
Gabriel

@huuck
Copy link
Owner

huuck commented May 13, 2019

Also, please reintegrate the original command-line arguments as we don't want to bring breaking changes into the other projects that integrate this repo. Other than the small issues I commented above, all looks good. As soon as you fix those I'm ready to approve the merge.

Kindest regards,
Gabriel

@pieterbork
Copy link
Author

I've found a solution to the binary file send issue, looks like I just forgot to return dropped_file from recv_binary. I'd like to do some additional cleanup before I commit so will get around to resolving remaining issues later tonight or tomorrow.

Cheers,
Pieter

@huuck
Copy link
Owner

huuck commented May 13, 2019 via email

@pieterbork
Copy link
Author

Okay, I think that should do it... let me know if you find any more bugs.

Cheers,
Pieter

@huuck
Copy link
Owner

huuck commented May 14, 2019

So after looking a bit more I still have a few issues:

  • Why did you remove the separate logging thread? It helped in keeping the timings for the protocol right.
  • Why did you change the format of the output JSON? Some external projects have that format as a dependency.
  • It also looks like shell commands are being truncated (didn't have this issue in the previous version). Could you look into this as well?
  • I'm also getting less shell commands hitting me on your version. I'll take a look into this today.

image

Cheers,
Gabriel

@pieterbork
Copy link
Author

pieterbork commented May 16, 2019

Okay, just made a few changes.

  1. Added separate logging thread/queue back in.

  2. Added a few of the fields back to the json output. I removed many of them originally because some felt like they were duplicating data:
    message is a combination of the other fields in the json, does this need to be there?
    utctime and unixtime are both timestamps, feels like we should just have utctime?

  3. Fixed commands being truncated

Thanks for catching all of this stuff! Were you able to discern why you were seeing less shell commands with this version?

Also, you've mentioned multiple external projects that use this one. I know Tpot is one of them, do you have a list of these that we can add to the README?

Cheers,
Pieter

@huuck
Copy link
Owner

huuck commented May 16, 2019

I also have the stable version of the honeypot running on a server and comparing the outputs. On the other hand, I'm seeing new hits in your version (maybe because of the more realistic shell responses). I'm gonna test it tonight and get back at you later.

Cheers!

@huuck
Copy link
Owner

huuck commented May 17, 2019

image

File name extraction crashing. Please revert to the old logic, even if it was really dumb before. The protocol has many corner cases that had to be taken into account.

All looks good with a minor exception: ALL logging should be done on a SEPARATE thread, not only json dumping. I/O is expensive and it sometimes impedes with the proper functioning of the honeypot.

Kindest regards,
Gabriel

@pieterbork
Copy link
Author

Okay, I think all logging is happening in a separate thread now. I switched the filename parsing because I was getting some weird characters/bytes included in the filename when I pushed. Could you pull latest and see if you're still getting any errors? I wasn't able to replicate the parsing error you posted, could you provide command/file to reproduce if you still see it?

Cheers,
Pieter

@huuck
Copy link
Owner

huuck commented Jun 5, 2019 via email

@huuck
Copy link
Owner

huuck commented Jun 13, 2019

Good new!

Testing looks good. Found a few issues which I'm gonna fix with a quick commit after integrating your pull request. Gonna wrap it up tonight/tomorrow.

Thanks A LOT for the hard work and keep in touch!

Kindest regards,
Gabriel

@huuck huuck merged commit 56abef7 into huuck:master Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants