Crash on server side and then client side when psk is null

[Ifilehk]

Hello there,

I am back with one more bug :-)

Experiencing a crash on the server side when the PSK is null for the connecting client. Unhandled exception: DtlsException: error:1419E044:SSL routines:tls_process_cke_psk_preamble:internal error

At the same time the client crashes with:

Cause: null pointer dereference
    r0  00000000  r1  00000049  r2  00000000  r3  7d0eb340
    r4  7d0eb340  r5  7e806c00  r6  7d608041  r7  75830180
    r8  5a5bbc9c  r9  00000005  r10 7582fe00  r11 682fba80
    ip  5a5a90c0  sp  682fba68  lr  7d5852e4  pc  5a5a9110
backtrace:
      #00 pc 00017110  /data/app/~~JnsSwGDasSwrdFQa0sEWUw==/com.ptt.ptt_client-l6OQ4oTeid1oEwTl35MROg==/base.apk!libssl.so (dtls1_ctrl+80)

Will try to narrow it down if you don't have a clue.

[JKRhb]

Hi ;) Thank you for the report! In what sense is the PSK null? Does the callback at the server side return null? Or is the client not providing one during the handshake?

[Ifilehk]

Exact. The callback on the server returns null because the the client has an unknown identity.

This leads to a server crash and I suppose the "bad" client crashes too because of continuing the handshake but not sure for that. I didn't put yet markers to identify the state.

[JKRhb]

I think for the server I already found a (pretty trivial) fix in #68 :) Could you try out if that solves the server-side problem? For the client, I also need to conduct further investigation. Let me know if you find anything :)

[Ifilehk]

OK will give a try

[Ifilehk]

With #68 the server ends with:

Unhandled exception:
DtlsException: error:0A0000DF:SSL routines::psk identity not found

Process finished with exit code 255

What is not the expected behavior. Server should not exit.

By the wait, with this fix the client does not crash anymore. It throws also:

DtlsException: error:0A00045B:SSL routines::reason(1115)

But strange that on client side pskCredentialsCallback is called with the identity hint sent from the server "This is the identity hint!"

[Ifilehk]

More info:

_connect_to_peer is invoked twice even if the first call has ret = 0.

I suppose because the else condition reports _handleError(ret, _connectCompleter.completeError); and void _maintainState() { checks only if (_connectCompleter.isCompleted) { and not the result of the completion ?

[JKRhb]

_connect_to_peer is invoked twice even if the first call has ret = 0.

I think that is actually necessary since the first time DTLSv1_listen is being called, it sends a Hello Verify Request if the Client Hello did not contain a cookie and then returns 0 (see the documentation for more information).

As I understand it now, the main problem is that connected is set to true too early (i.e., as soon as DTLSv1_listen returns 1), when in fact at this point the handshake is not yet completed. Maybe there is a state between "connected" and "not connected" needed to reflect that.

[JKRhb]

I now updated #68 with an additional fix that should take the handshake result into account properly, partly by adding additional states a server connection can transition to. Not sure if all edge cases are covered yet – could you give it another try? :)

[Ifilehk]

I agree with your comments.

Your last fix is OK. Thank you!

Just missing a way to catch this state on server side :-)

By the way the client throws DtlsException: error:0A00045B:SSL routines::reason(1115)

[JKRhb]

Your last fix is OK. Thank you!

Great, thanks for the feedback :)

Just missing a way to catch this state on server side :-)

I now exposed the state of a DtlsConnection in general. I think there is still some room for improvement regarding state management, but it should be a step in the right direction :)

By the way the client throws DtlsException: error:0A00045B:SSL routines::reason(1115)

Hmm, besides providing a better error message, what would you say should be the behavior here? From my understanding throwing an exception wouldn't be wrong per se, since the connection attempt has failed. But I would assume that at the moment, the current behavior sort of leads to a memory leak since the allocated resources cannot be cleaned up anymore, right?

[JKRhb]

Could you maybe test out #70 for further fixes on the client side? :)

[Ifilehk]

Hmm, besides providing a better error message, what would you say should be the behavior here? From my understanding throwing an exception wouldn't be wrong per se, since the connection attempt has failed. But I would assume that at the moment, the current behavior sort of leads to a memory leak since the allocated resources cannot be cleaned up anymore, right?

Throwing the exception is perfect. Taking care of the cleanup should be done to avoid memleaks, yes !

[Ifilehk]

Could you maybe test out https://github.com/JKRhb/dtls2/pull/70 for further fixes on the client side? :)

Sure, will let you know. Thanks for your always quick responses !!!

[Ifilehk]

Bom dia !

70 leads immediately to DtlsException: DTLS Handshake has failed. event without connecting to the server.

[JKRhb]

Bom dia !

Olá! ;)

70 leads immediately to DtlsException: DTLS Handshake has failed. event without connecting to the server.

Oh, sorry, I think I messed up the logic a bit 😅 Could you try out #70 once more? :) I hope that it is now fixed.

[Ifilehk]

No problem !

[Ifilehk]

OK, doing the expected job in my client server setup. Thank you!!!

[JKRhb]

OK, doing the expected job in my client server setup. Thank you!!!

Awesome, thank you for your confirmation! :) Just released version 0.14.0, let me know if you spot any more bugs! (Fingers crossed that we are close to a bug-free state 😉)

[Ifilehk]

Will update to 0.14 and for sure bother you again if something shows up :-)

Thank you for your support !!!