PSK broken in net.nim, pskClientCallback can't access extraInternal

[lullius]

Setting clientGetPskFunc= on SSLContext ctx and retrieving it works as expected.

Example

import net


proc clientFunc(hint: string): tuple[identity: string, psk: string] =
    echo "identity hint: ", hint
    return ("identity", "psk")

var ctx = newContext()
ctx.clientGetPskFunc=clientFunc
var f = ctx.clientGetPskFunc
echo f("my_hint")

Output

identity hint: my_hint
(identity: "identity", psk: "psk")

But when trying to wrap a socket, pskClientCallback in net.nim can't access the context's SslContextExtraInternal. It gets the context like this:

    let ctx = SSLContext(context: ssl.SSL_get_SSL_CTX)

Then it tries to get the identity and psk:

    let (identityString, pskString) = (ctx.clientGetPskFunc)(hintString)

clientGetPskFunc calls getExtraInternal which returns ctx.extraInternal, but the context's extraInternal was never set.

Running the server and client in this example or running this code results in a SIGSEGV:

Example

import net


proc clientFunc(hint: string): tuple[identity: string, psk: string] =
    echo "identity hint: ", hint
    return ("identity", "psk")

var ctx = newContext()
ctx.clientGetPskFunc=clientFunc
let sock = newSocket()
sock.connect("localhost", Port(8800))
ctx.wrapConnectedSocket(sock, handshakeAsClient)

Output from this client:

Current Output

Traceback (most recent call last)
pskclient.nim(15)        pskclient
net.nim(674)             wrapConnectedSocket
net.nim(587)             pskClientCallback
net.nim(581)             clientGetPskFunc
SIGSEGV: Illegal storage access. (Attempt to read from nil?)

Expected Output

Should connect using PSK.

If I hardcode the identity and psk directly into pskClientCallback everything works as expected.

I'm not sure, but this issue may be related to something dom96 mentioned in #4565. Comment:

There is a reason that assert is there, fixing that reason isn't trivial: https://github.com/nim-lang/Nim/pull/4565/files#diff-df7c4ae903eab01bc8c4f64f425b48c3R374. IIRC the problem is that the PSK client callback doesn't have access to the underlying SSL context and giving it access is also not so easy.

Everything above was compiled with -d:ssl.

Additional Information

$ nim -v
Nim Compiler Version 0.19.9 [Linux: amd64]
Compiled at 2019-05-19

$ uname -r
5.0.13-arch1-1-ARCH

[shirleyquirk]

Thank you @lullius, your research was absolutely spot on

pinging @dom96, this is indeed a hard problem, would be glad for your input

Since i'm elbow-deep in net at this point, i thought i'd try to sort this out, here's my take:

• for the callback to retrieve anything from the ssl ex_data, it needs to know the index
• the index can only be known at runtime
• the index can't be reliably guessed, c.f Exception of net.newContext: result.extraInternalIndex == 0 [AssertionError] #4406
• storing the index in a global/threadvar wouldn't work or cause more problems

so my conclusion is that storing extrainternals anywhere is the wrong approach.
however, as #17728 shows, writing a psk callback is pretty fraught, so providing some abstraction isn't a crazy idea
but I believe that abstraction needs to be a compile-time injection of the users code into the callback

[shirleyquirk]

Using
pskclient.nim

import net
static: assert defined(ssl)
let sock = newSocket()
sock.connect("localhost", Port(8800))

proc clientFunc(identityHint: string): tuple[identity: string, psk: string] =
  echo "identity hint ", identityHint.repr
  return ("foo","psk-of-foo")

let context = newContext(cipherList="PSK-AES256-CBC-SHA")

context.clientGetPskFunc = clientFunc
context.wrapConnectedSocket(sock, handshakeAsClient)
context.destroyContext()

and

pskserv.nim

# Accept connection encrypted using preshared key (TLS-PSK).
import net

static: assert defined(ssl)

let sock = newSocket()
sock.bindAddr(Port(8800))
sock.listen()

let context = newContext(cipherList="PSK-AES256-CBC-SHA")
context.pskIdentityHint = "hello"
context.serverGetPskFunc = proc(identity: string): string = "psk-of-" & identity
context.sessionIdContext= "none"#can be anything just has to be set
while true:
  var client = new(Socket)
  sock.accept(client)
  sock.setSockOpt(OptReuseAddr, true)
  echo "accepted connection"
  context.wrapConnectedSocket(client, handshakeAsServer)
  echo "got connection with identity ", client.getPskIdentity()

and using https://github.com/shirleyquirk/Nim/tree/fixpsk

this works. there are bugs but not in the callbacks, at least.

so, running the client and the server, they connect, but the hint and identity aren't output

if the client is run, along with openssl s_server -accept 8800 -psk 70736B2D6F662D666F6F -psk_identity foo -psk_hint foo_server -cipher PSK-AES256-CBC-SHA -nocert -no_tls1_3 CBC-SHA -nocert -no_tls1_3

the callback is called twice, the first time not receiving the hint, the second time receiving it.
and then the server reports an error (i assume when the client drops the connection?)

if the server is run, along with openssl s_client -connect localhost:8800 -psk 70736B2D6F662D666F6F -psk_identity foo -cipher PSK-AES256-CBC-SHA -no_tls1_3 it connects and knows the identity of the client just fine.

[shirleyquirk]

oh maybe the server/client by themselves are using tls1.3, i bet that's it

yes. tls ciphersuites are never getting set due to another bug

Nim/lib/pure/net.nim

Line 613 in 957478c

if sslVersion >= 0x010101000 and not sslVersion == 0x020000000:

is always false
needs parens around (sslVersion == 0x20000000)
but fixing that breaks other stuff...

[shirleyquirk]

ok, yes, by turning off tls1.3 n pskclient i can get this to work as expected.