Use of hex_encode()

[gvanem]

Just a proposal.

The hex_encode() function is duplicated in some places. Would it be better to use str_tohex() instead? And maybe speedup str_tohex() in ec_strings.c using the itoa16[] as used in src/dissectors: ec_postgresql.c, ec_mysql.c and ec_vnc.c.

[gvanem]

Btw, in ec_postgreqsl.c:

   hex_encode(ptr + 9, 4, conn_status->salt); /* save salt */  << not 0-terminated
   ...
  DISSECT_MSG("%s:$postgres$%s*%s*%s:%s:%d\n", conn_status->user, 
      conn_status->user, conn_status->salt, 
      conn_status->hash, ip_addr_ntoa(&PACKET->L3.dst, tmp), ntohs(PACKET->L4.dst));

Is it safe to assume that conn_status->salt is 0-terminated when used as above? If not so, another good reason to use str_tohex() from ec_strings.c.

Here is what I propose:

static char itoa16[16] =  "0123456789ABCDEF";

char *str_tohex(u_char *bin, size_t len, char *dst, size_t dst_len)
{
   size_t i, j;

   memset(dst, 0, dst_len);

   for (i = j = 0; i < len && j < dst_len; j += 2, i++) {
      dst[j]   = itoa16 [bin[i] >> 4];
      dst[j+1] = itoa16 [bin[i] & 0xF];
   }
   return (dst);
}

Should be a lot faster than using sprintf() in a loop. Not sure if some protocols require lower or uppercase hex-chars?

[LocutusOfBorg]

don't know...

grep str_tohex . -R ./src/protocols/ec_wifi_eapol.c: DEBUG_MSG("WPA ANonce : %s", str_tohex(sa.ANonce, WPA_NONCE_LEN, tmp, sizeof(tmp))); ./src/protocols/ec_wifi_eapol.c: DEBUG_MSG("WPA SNonce : %s", str_tohex(sa.SNonce, WPA_NONCE_LEN, tmp, sizeof(tmp))); ./src/protocols/ec_wifi_eapol.c: DEBUG_MSG("WPA PTK : %s", str_tohex(sa.ptk, WPA_PTK_LEN, tmp, sizeof(tmp))); ./src/protocols/ec_wifi_eapol.c: DEBUG_MSG("WPA MIC : %s", str_tohex(eapol_key->key_MIC, WPA_MICKEY_LEN, tmp, sizeof(tmp))); ./src/protocols/ec_wifi_eapol.c: USER_MSG("WPA KEY : %s\n", str_tohex(sa.decryption_key, WPA_DEC_KEY_LEN, tmp, sizeof(tmp))); ./src/ec_encryption.c: USER_MSG("Using WEP key: %s\n", str_tohex(tmp_wkey, tmp_wkey_len, tmp, sizeof(tmp))); ./src/ec_encryption.c: USER_MSG("Using WPA key: %s\n", str_tohex(GBL_WIFI->wkey, WPA_KEY_LEN, tmp, sizeof(tmp))); ./src/ec_encryption.c: DEBUG_MSG("Encrypted Broadcast key: %s\n", str_tohex(encrypted_key, key_len, tmp, sizeof(tmp))); ./src/ec_encryption.c: DEBUG_MSG("KeyIV: %s\n", str_tohex(eapol_key->key_IV, 16, tmp, sizeof(tmp))); ./src/ec_encryption.c: DEBUG_MSG("decryption_key: %s\n", str_tohex(sa->ptk + 16, 16, tmp, sizeof(tmp)));

I see it used in wep/wpa, I'm wondering about case sensitiveness of the key

[gvanem]

grep str_tohex . -R

Did you grep for hex_encode too?

[LocutusOfBorg]

grep hex_encode . -R ./src/dissectors/ec_postgresql.c:static inline void hex_encode(unsigned char _str, int len, unsigned char out) ./src/dissectors/ec_postgresql.c: hex_encode(ptr + 9, 4, connstatus->salt); / save salt / ./src/dissectors/ec_vnc.c:static inline void hex_encode(unsigned char str, int len, unsigned char out) ./src/dissectors/ec_vnc.c: hex_encode(conn_status->challenge, 16, challenge); ./src/dissectors/ec_vnc.c: hex_encode(ptr, 16, response); ./src/dissectors/ec_mysql.c:static inline void hex_encode(unsigned char str, int len, unsigned char out) ./src/dissectors/ec_mysql.c: hex_encode(seed, 20, output); ./src/dissectors/ec_mysql.c: hex_encode(input, 20, output);

[gvanem]

Ping?

[sgeto]

Hi,

The hex_encode() function is duplicated in some places.

True. 3 times. Kinda messy. CHANGELOG:133 mentions a cleanup of some sort. Not sure if this was before or after this issue...

Should be a lot faster than using sprintf() in a loop.

How much faster are we talking?

Not sure if some protocols require lower or uppercase hex-chars?

No they shouldn't. Although RFCs use uppercase characters and most Linux headers use lowercase. GCC couldn't care less but clang bitches about that a lot. That's something to keep in mind.

I should probably add the "help wanted" label here. MacOS is giving me a hard time over at "fix library 2" so if I'm not caught up with personal things, we're busy with that. Do you have something up and running (a patch) or is this more like a theory?

"We are saddened by a bird's cry, but not for a fish's blood. Blessed are those with voices."

-Mamoru Oshii

On Apr 6, 2018, at 8:14 PM, Gisle Vanem notifications@github.com wrote:

Ping?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[gvanem]

Do you have something up and running (a patch) or is this more like a theory?

No patch. Just my € 0.2.