Add support for TLS-PSK ciphersuites to net.nim #3472

Merged
merged 3 commits into from Jun 3, 2016

Projects

None yet

3 participants

@zielmicha
Contributor

From Wikipedia:

Transport Layer Security pre-shared key ciphersuites (TLS-PSK) is a set of cryptographic protocols that provide secure communication based on pre-shared keys (PSKs). These pre-shared keys are symmetric keys shared in advance among the communicating parties.

@dom96 dom96 commented on the diff Oct 24, 2015
lib/pure/net.nim
@@ -180,6 +188,22 @@ when defined(ssl):
var errStr = ErrErrorString(err, nil)
raise newException(SSLError, $errStr)
+ proc getSslContextExtraDataIndex*(): cint =
+ ## Retrieves unique index for storing extra data in SSLContext.
+ return SSL_CTX_get_ex_new_index(0, nil, nil, nil, nil)
+
+ proc setExtraData*(ctx: SSLContext, index: cint, data: pointer) =
@dom96
dom96 Oct 24, 2015 Member

I feel like this should be a bit more idiomatic and take a RootRef instead of a pointer.

@Araq thoughts?

@zielmicha
zielmicha Oct 24, 2015 Contributor

This won't work, because OpenSSL expects non-managed pointer, not reference.

@dom96
dom96 Mar 18, 2016 Member

I think it will as long as you call GC_ref on the ref. You should then be able to safely cast it to a ptr.

@dom96 dom96 commented on the diff Oct 24, 2015
lib/pure/net.nim
@@ -180,6 +188,22 @@ when defined(ssl):
var errStr = ErrErrorString(err, nil)
raise newException(SSLError, $errStr)
+ proc getSslContextExtraDataIndex*(): cint =
+ ## Retrieves unique index for storing extra data in SSLContext.
+ return SSL_CTX_get_ex_new_index(0, nil, nil, nil, nil)
@dom96
dom96 Oct 24, 2015 Member

What does the 0 mean here? Shouldn't an instance of SSLContext be passed in?

Wouldn't it make more sense to create an overload for setExtraData which generates the index for us (and which returns the index too)?

@zielmicha
zielmicha Oct 24, 2015 Contributor

I don't think so, the index here is global for all SSLContexts. Something more fancy (generic generating global call togetSslContextExtraDataIndex?) might work.

@dom96 dom96 and 1 other commented on an outdated diff Oct 24, 2015
lib/pure/net.nim
@@ -241,7 +265,72 @@ when defined(ssl):
discard newCTX.SSLCTXSetMode(SSL_MODE_AUTO_RETRY)
newCTX.loadCertificates(certFile, keyFile)
- return SSLContext(newCTX)
+
+ result = SSLContext(newCTX)
+ # this is never freed, but SSLContext can't be freed anyway yet
+ let extraInternal = new(SslContextExtraInternal)
+ GC_ref(extraInternal)
@dom96
dom96 Oct 24, 2015 Member

When will it be freed then?

@zielmicha
zielmicha Oct 24, 2015 Contributor

Ok, I will add destructor, this comment only means that there already is a memory leak involving SSLContexts.

@dom96
Member
dom96 commented Oct 24, 2015

Please make sure that all procedures you are exporting are documented.

zielmicha added some commits Oct 24, 2015
@zielmicha zielmicha net.nim: support for TLS-PSK ciphersuites
ba61a8d
@zielmicha zielmicha net.nim: destroyContext for destroying SSLContext
3ecf33f
@zielmicha
Contributor

I've added dealloction proc and documented one missing proc.

@dom96 dom96 self-assigned this Oct 28, 2015
@Araq
Member
Araq commented May 29, 2016

Dom96 and me agreed on moving these parts to rawsockets instead (as far as it's reasonable). I will hit dom from time to time until it's done.

@dom96 dom96 merged commit 3ecf33f into nim-lang:devel Jun 3, 2016

1 of 2 checks passed

continuous-integration/appveyor AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dom96 dom96 added a commit that referenced this pull request Jun 3, 2016
@dom96 dom96 Modified #3472 to make its API more idiomatic. 5390c25
@dom96
Member
dom96 commented Jun 3, 2016

I decided to keep it in net, but I modified it rather heavily to make its interface (hopefully) safer.

@dom96
Member
dom96 commented Jun 3, 2016

Btw on Mac OS X I can't get the client/server examples working, I get:

could not import: SSL_CTX_set_psk_client_callback

And similar for the server. I guess my OpenSSL is too old?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment