o5logon dissector overhaul. #742

magnumripper · 2016-07-04T21:34:00Z

This is needed for newer versions of Oracle. It also fixes various problems in the original code but ultimately I need a fix for #741 to do it good.

kholia · 2016-07-05T05:42:53Z

Here are some .pcap files for testing these changes,

https://raw.githubusercontent.com/kholia/ctf/master/ggyy-99b3ed3ed5e2a293c697784ac94ee8c6.pcap

http://openwall.info/wiki/_media/john/Oracle-O5LOGON.tar.gz

And pcap files from hashcat/hashcat#371

kholia · 2017-02-08T05:59:52Z

@koeppea @LocutusOfBorg Ping, it would be great if you could review, and possibly merge this PR.

LocutusOfBorg · 2017-02-08T14:06:51Z

I would like to see a fix for #741 before merging this one... isn''t it better?

koeppea · 2017-02-08T14:14:50Z

I'd also like to add some annotations for this one. Could yet manage to afford the time.

koeppea

I've reviewed the pull request and have some things to be addressed.

koeppea · 2017-02-08T19:08:50Z

src/dissectors/ec_o5logon.c

+/* PKCS#7 padding used? */
+#define MAYBE 0
+#define YES   1
+#define NO    2


pre-processor directives and variables should be unique.
please add somwhat like EC_O5LOGON_ to the pre-processor variables.

koeppea · 2017-02-08T19:14:53Z

src/dissectors/ec_o5logon.c

@@ -46,6 +65,24 @@ void o5logon_init(void);

 /************************************************/

+#undef memrchr
+#define memrchr my_memrchr


What is the purpose of this renaming? You shouldn't replace standard function names.
Why do you implement your own version of this standard function?

That function is not present in eg. OSX so I implemented it. The define saves us on eg. Linux where it is present.

I don't understand cmake but I presume this could be dealt with in some more clever way (ie. use system version if present) - but this works fine as-is on Linux and OSX.

So we'd should only use it when being on a BSD or DARWIN a.k.a. MacOS operating system. The pre-processor variables are set by cmake.
I'd even recommend putting it into a "common" file like ec_utils.c that make it easier to be reused by other functions in future.
Or we could put it into a dedicated source / header file and include it into compilation processes depending on Cmake tests.

I could also help you with the Cmake part if you want.

Just checked FreeBSD. There the memrchr() function exists. MacOSX apparently is the only OS lacking memrchr() .

Added 40979f2 which moves it to ec_utils.c and wraps with #if OS_DARWIN. Tested on OSX, "should work" on others (famous last words).

koeppea · 2017-02-08T19:16:54Z

src/dissectors/ec_o5logon.c

+         return NULL;
+      }
+
+      u_char *ano;


Please declare variables only at the beginning of a block, otherwise this will generate a warning.

koeppea · 2017-02-08T19:21:05Z

src/dissectors/ec_o5logon.c


 #include <ec.h>
 #include <ec_decode.h>
 #include <ec_dissect.h>
 #include <ec_session.h>

+//#define O5DEBUG


I'd rather recommend defining this (prefixed with EC_) with the value 0 for off and 1 for on.
to include such a block you the do #if EC_O5DEBUG instead of #ifdef ....
But this is really somewhat cosmetic and flavor.

koeppea · 2017-02-08T19:27:04Z

src/dissectors/ec_o5logon.c

   u_char user[129];
+   u_char srv_addr[22];


What is the purpose of srv_addr[22]? Holding the string representation of a IP address?

it won't fit an IPv6 address

managing IP addresses string or human representrationwise is somewhat painful.

I rather recommend using the type struct ip_addr * instead and use the ip_addr_* functions to initialize, handle and represent the IP address.

AUTH_SESSKEY in case we can no longer exploit known padding (version 12).

magnumripper · 2017-02-08T20:10:04Z

Thank you for reviewing. Requested changes are now implemented and it's also rebased onto current tree. Tested on OSX (prior to last commit it was also tested on Linux).

magnumripper · 2017-02-08T20:16:15Z

I would like to see a fix for #741 before merging this one... isn''t it better?

It would be beneficial for JtR and Hashcat users to have it merged right away. If/when #741 is fixed I will promptly make a PR with enhancements taking advantage of the functionality.

gvanem

memrchr() should be added for OS_WINDOWS too. And IMHO, the prototype should be in put in include/ec_strings.h; similar to memmem() etc.

koeppea · 2017-02-10T20:48:45Z

@magnumripper may I request contribution / write permissions to your fork?
I'd like to add a generic Cmake test-based approach to determine whether or not this function exist.
I can also move it to ec_strings.c as @gvanem suggested, as memrchr() is natively also defined in string.h.

magnumripper · 2017-02-12T08:58:14Z

@koeppea you are now a collaborator.

koeppea · 2017-02-12T19:03:36Z

Thanks.

koeppea

I've tested the dissector with some of the PCAP files referenced.

I'm not a Oracle expert, but I'd expected some less "verbose" information. In contrast some information is only be printed with DEBUG enabled which I'd expected the other way round.

Just a comment, but I'm afraid that the messages pane gets flooded so that other potentially important information is getting lost when one or more Oracle sessions are going over the wire.

What do you think?

koeppea · 2017-02-13T21:34:33Z

src/dissectors/ec_o5logon.c

+#if EC_O5LOGON_DEBUG
+         DISSECT_MSG("O5LOGON: client ver %u.%u.%u.%u.%u (PKCS)\n", 
+               ano[0], ano[1] >> 4, ano[1] & 15 , ano[2], ano[3]);
+#endif


Wouldn't this being worth to be print w/o DEBUG?

koeppea · 2017-02-13T21:35:30Z

src/dissectors/ec_o5logon.c

+                     ntohs(PACKET->L4.dst), 
+                     conn_status->user, 
+                     conn_status->flags.c_ano == 1 ? "" : "(no ANO seen)");
+#endif


This as well?

koeppea · 2017-02-13T21:36:21Z

src/dissectors/ec_o5logon.c

+#if EC_O5LOGON_DEBUG
+                  DISSECT_MSG("O5LOGON: server ver %u.%u.%u.%u.%u\n", ano[0], 
+                        ano[1] >> 4, ano[1] & 15 , ano[2], ano[3]);
+#endif


This as well?

I personally had no need for either of them except while debugging / trying to understand the protocol), but I'm fine with including them w/o debug.

What do you think about the amount of information in the messages pane:

$ ettercap -Tqr ~/Downloads/ggyy-99b3ed3ed5e2a293c697784ac94ee8c6.pcap ettercap 0.8.2 copyright 2001-2015 Ettercap Development Team Reading from /home/koeppea/Downloads/ggyy-99b3ed3ed5e2a293c697784ac94ee8c6.pcap Libnet failed IPv4 initialization. Don't send IPv4 packets. Libnet failed IPv6 initialization. Don't send IPv6 packets. 34 plugins 42 protocol dissectors 57 ports monitored 20530 mac vendor fingerprint 1766 tcp OS fingerprint 2182 known services Starting Unified sniffing... O5LOGON: AASH@192.168.131.1:$o5logon$209C317916169AEFD4ACA354CBA5588902933CBB2C31909898F1B4A7B1B4612A423FE59958FB8E07400FC69577B7BE34*ED3BE8DDEBF2FE6FB36F O5LOGON: AASH@192.168.131.1:$o5logon$209C317916169AEFD4ACA354CBA5588902933CBB2C31909898F1B4A7B1B4612A423FE59958FB8E07400FC69577B7BE34*ED3BE8DDEBF2FE6FB36F O5LOGON: AASH@192.168.131.1:$o5logon$209C317916169AEFD4ACA354CBA5588902933CBB2C31909898F1B4A7B1B4612A423FE59958FB8E07400FC69577B7BE34*ED3BE8DDEBF2FE6FB36F O5LOGON: AASH@192.168.131.1:$o5logon$209C317916169AEFD4ACA354CBA5588902933CBB2C31909898F1B4A7B1B4612A423FE59958FB8E07400FC69577B7BE34*ED3BE8DDEBF2FE6FB36F O5LOGON: AASH@192.168.131.1:$o5logon$209C317916169AEFD4ACA354CBA5588902933CBB2C31909898F1B4A7B1B4612A423FE59958FB8E07400FC69577B7BE34*ED3BE8DDEBF2FE6FB36F ....

What can be done with this information?

Like I wrote below it's password hashes. I could try to reject dupes tho, but I'm not sure it's worth the effort. It shouldn't be a problem IRL.

@LocutusOfBorg what do you think about the deduplication? I have no feeling about the real world occurrence.
On the other hand, we don't have such mechanisms on other dissectors and omitting it keeps the processing lightweight.

I don't think it is worth the effort...

somebody might want to sort and count how many people have the same password hash maybe?

koeppea · 2017-02-13T22:05:03Z

Tested on Linux, MacOSX and FreeBSD.
@LocutusOfBorg: Good to go for me.

magnumripper · 2017-02-13T22:11:59Z

I'm not a Oracle expert, but I'd expected some less "verbose" information.

The output is password hashes, ready to crack in JtR or Hashcat.

magnumripper · 2017-02-13T22:13:56Z

Oh, and thanks a lot for reviewing and fixing the last issues.

LocutusOfBorg · 2017-02-15T09:14:01Z

src/dissectors/ec_o5logon.c


 #include <ec.h>
 #include <ec_decode.h>
 #include <ec_dissect.h>
 #include <ec_session.h>
+#include <ec_utils.h>
+
+#define EC_O5LOGON_DEBUG       0


@koeppea what about "if not defined then define it to 0?" this way people might be able to feed this value from the environment, without having to change the source code.
it looks good to me, even if I didn't test it. Feel free to merge whenever you want :)

LocutusOfBorg · 2017-02-16T11:10:57Z

merged thanks you all :)

regel · 2017-06-15T10:02:34Z

@magnumripper This is awesome work. Thanks! To document the observed traffic, is it possible to summarize e.g. in a table the byte format observed on the wire versus the Oracle client and Oracle server versions actually in use?

magnumripper · 2017-06-15T20:48:43Z

I wouldn't mind that, but unfortunately I don't have any time to invest.

magnumripper mentioned this pull request Jul 4, 2016

Oracle auth_session and auth_password hashcat/hashcat#371

Open

magnumripper mentioned this pull request Jul 20, 2016

Dissector "atexit" functionality #741

Open

magnumripper mentioned this pull request Aug 14, 2016

o5logon length leak openwall/john#2155

Closed

magnumripper mentioned this pull request Aug 25, 2016

oracle old version openwall/john#2159

Open

koeppea requested changes Feb 8, 2017

View reviewed changes

magnumripper added 4 commits February 8, 2017 21:04

Copy/paste typo in doc/decoders

d131115

Some enhancements to o5logon dissector, for supporting varying formats.

03204fd

o5logon dissector: Add fields for AUTH_PASSWORD and client's

48886da

AUTH_SESSKEY in case we can no longer exploit known padding (version 12).

o5logon dissector: Requested changes. See Ettercap#742.

873248c

magnumripper force-pushed the master branch from 1f09cf9 to 873248c Compare February 8, 2017 20:05

Add memrchr() for OSX in ec_utils.h (and drop it from o5logon dissector)

40979f2

gvanem suggested changes Feb 10, 2017

View reviewed changes

Alexander Koeppe added 3 commits February 13, 2017 21:54

conditionally include memrchr()

4b54167

prefixing dissector messages

e6c2351

apply code style

b236fb1

koeppea reviewed Feb 13, 2017

View reviewed changes

koeppea approved these changes Feb 13, 2017

View reviewed changes

LocutusOfBorg reviewed Feb 15, 2017

View reviewed changes

increased verbosity through compiler flag

3d95791

LocutusOfBorg merged commit 60aca98 into Ettercap:master Feb 16, 2017

magnumripper mentioned this pull request Oct 14, 2017

o5logon improvements? #842

Closed

o5logon dissector overhaul. #742

o5logon dissector overhaul. #742

Conversation

magnumripper commented Jul 4, 2016

kholia commented Jul 5, 2016

kholia commented Feb 8, 2017

LocutusOfBorg commented Feb 8, 2017

koeppea commented Feb 8, 2017 via email

koeppea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magnumripper Feb 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magnumripper Feb 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magnumripper commented Feb 8, 2017 • edited Loading

magnumripper commented Feb 8, 2017 • edited Loading

gvanem left a comment • edited Loading

Choose a reason for hiding this comment

koeppea commented Feb 10, 2017

magnumripper commented Feb 12, 2017

koeppea commented Feb 12, 2017

koeppea left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koeppea commented Feb 13, 2017

magnumripper commented Feb 13, 2017

magnumripper commented Feb 13, 2017

Choose a reason for hiding this comment

LocutusOfBorg commented Feb 16, 2017

regel commented Jun 15, 2017

magnumripper commented Jun 15, 2017

magnumripper Feb 8, 2017 •

edited

Loading

magnumripper Feb 10, 2017 •

edited

Loading

magnumripper commented Feb 8, 2017 •

edited

Loading

magnumripper commented Feb 8, 2017 •

edited

Loading

gvanem left a comment •

edited

Loading