Closed pieterbork closed 5 years ago
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
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
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
Allright!
Poke me when you're done, I'll take a quick look and implement one of my fixes as well after we merge (the payload comes corrupted if you have DATA, as in the ASCII string, inside).
Kindest regards, Gabriel
On Mon, May 13, 2019 at 9:10 PM Pieter Bork notifications@github.com wrote:
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
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/huuck/ADBHoney/pull/27?email_source=notifications&email_token=AAZSVROEFCO6XSISGWAKOATPVGVIZA5CNFSM4HMMPN5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVJDVMY#issuecomment-491928243, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZSVRPVQDIWVXCKAO55B3LPVGVIZANCNFSM4HMMPN5A .
Okay, I think that should do it... let me know if you find any more bugs.
Cheers, Pieter
So after looking a bit more I still have a few issues:
Cheers, Gabriel
Okay, just made a few changes.
Added separate logging thread/queue back in.
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
?
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
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!
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
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
Thanks for the commit, testing right now :D The log with the errors can be found here: http://hux.site/nohup.out (I just do sanity testing by hand and let the honeypot listen for payloads on a 24h run and compare with a baseline in the morning).
Kindest regards, Gabriel
On Wed, Jun 5, 2019 at 9:30 AM Pieter Bork notifications@github.com wrote:
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
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/huuck/ADBHoney/pull/27?email_source=notifications&email_token=AAZSVRJHILY5WFJ4SXMPFKTPY5MQBA5CNFSM4HMMPN5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW6XPAI#issuecomment-498956161, or mute the thread https://github.com/notifications/unsubscribe-auth/AAZSVROSG35FADSGLDW574TPY5MQBANCNFSM4HMMPN5A .
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
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.