[WIP] SSL support for eval #9009

Simn · 2019-12-05T08:57:19Z

This PR starts adding SSL support for eval. To that end, we currently include the entire mbedtls code in libs/mbedtls, have a FFI layer in mbedtls_stubs.c and mbedtls.ml and then Haxe extern definitions in eval/_std/mbedtls. As a result, the diff is a mess.

There are several TODOs here:

LIB_PARAMS

~~I have added this to Makefile.win in order to link crypt32 on Windows:~~

~~ifeq ($(STATICLINK),0) LIB_PARAMS = -cclib -lpcre -cclib -lz -cclib -lcrypt32 endif~~

~~This probably isn't ideal.~~

Update: This is fine.

Default certificate loading

For Linux, this is handled in Mbedtls.hx with the code being copied from neko/HL. For Windows, it is handled at C-level through Mbedtls.loadDefaults with that code also being copied from neko/HL. This requires some testing.

There's currently no implementation for Mac. I was hoping that @Aurel300 could look into setting that up. It should be a matter of copying the code from https://github.com/HaxeFoundation/hashlink/blob/master/libs/ssl/ssl.c#L364-L397 and then figuring out how to handle the linking.

Amount of .c and .h files

~~We probably don't need to include all these files, so we should look into removing some of them.~~

Update: The C files have been removed.

SNI stuff

The neko/HL implementations have some code in sys.ssl.Socket which, from my understanding, is about server-side sockets. I ignored that completely as I don't plan to run https servers in eval. We have to verify that this is fine to ignore.

Rest of the sys.ssl package

The sys.ssl package has some other files:

~~Certificate~~
Digest
~~DigestAlgorithm~~
~~Key~~

Update: Certificate is done. DigestAlgorithm is just an enum.

I think Digest and Key can be added easily, but I don't know what to do with Certificate. It goes beyond being a simple extern for an mbedtls certificate and provides some public API that deals with its wrapping nature, such as next. I don't want to replicate this wrapping because it's a bad pattern for eval, so we have to think about the public API for this.

~~I don't think this should stop us from merging the Socket part this PR focuses on though.~~

Update: I'll just shut up and follow the API.

Update: Key has been added.

Overall testing

The CI is green so building works, but we don't actually have any https tests.

Packaging and licensing

~~This is something I usually forget about, so this time around I'd like to check with @andyli if the approach taken here causes any issues for packaging.~~

~~On a maybe related note, I have no idea about licensing and if we can just drop the mbedtls source code into our project.~~

Update: The C files have been removed.

Makefile

RealyUniqueName · 2019-12-05T09:19:43Z

I have only a slight knowledge of SSL, so I can't properly review that.
But I don't like mbedtls being a top-level package. I think it should be somewhere in subpackages of eval.
Same for sys.net.NativeSocket which is eval-specific.

andyli · 2019-12-05T09:21:39Z

Don't commit mbedtls source code into our repo. I think you can let dune to download and extract a mbedtls archive, when it doesn't exist globally or in the project folder. It's mainly because the Linux packages have to use the distros' mbedtls.

Simn · 2019-12-05T09:40:14Z

But I don't like mbedtls being a top-level package. I think it should be somewhere in subpackages of eval.

The mbedtls API isn't eval-specific.

Simn · 2019-12-05T09:48:43Z

Same for sys.net.NativeSocket which is eval-specific.

That used to be a private class so it didn't matter, but you're right. I've moved it to eval.vm now, which is not a good package name but follows the neko layout.

RealyUniqueName · 2019-12-05T10:20:52Z

The mbedtls API isn't eval-specific.

Maybe move it into sys.ssl?

Simn · 2019-12-05T10:54:25Z

Don't commit mbedtls source code into our repo. I think you can let dune to download and extract a mbedtls archive, when it doesn't exist globally or in the project folder.

I can't find anything about it downloading sources. The foreign build section explicitly states "put all the foreign code in a sub-directory".

The best alternative I can think of right now is using a submodule...

libs/mbedtls/dune

libs/mbedtls/mbedtls_stubs.c

src/macro/eval/evalSsl.ml

std/eval/_std/mbedtls/Certificate.hx

std/eval/_std/sys/ssl/Mbedtls.hx

std/eval/_std/sys/ssl/Socket.hx

std/sys/Http.hx

libs/mbedtls/mbedtls_stubs.c

Aurel300 · 2019-12-11T15:26:18Z

libs/mbedtls/mbedtls_stubs.c

+	mbedtls_x509_crt* cert = X509Crt_val(chain);
+	mbedtls_x509_time *t = &cert->valid_to;
+	time_t time = time_to_time_t(t);
+	CAMLreturn(caml_copy_double((double)time));


I thought time_t was usually an integer type? Also will this work on 32-bit systems?

I expect the C compiler to handle this accordingly, but if you have an alternative suggestion I'll be happy to change this.

std/eval/_std/mbedtls/X509Crt.hx

Simn · 2019-12-12T09:42:00Z

std/eval/_std/sys/ssl/Certificate.hx

-	private function get_altNames():Array<String> {
-		throw "Not implemented";
-	}
+	extern private function get_altNames():Array<String>;


I don't know if this actually works because no Certificate I come across has a value for this, so all we get are empty arrays.

Simn · 2019-12-12T09:46:41Z

I have completed the Certificate API. Not sure if this actually works as expected because we have no tests, so I guess time will tell.

I'll add the APIs for Digest and Key and then we just need to deal with the OS X default certificate loading. After that, I think we can merge.

Simn · 2019-12-12T09:48:50Z

libs/mbedtls/mbedtls_stubs.c

+	CAMLlocal1(obj);
+	mbedtls_x509_crt* cert = X509Crt_val(chain);
+	if (cert->ext_types & MBEDTLS_X509_EXT_SUBJECT_ALT_NAME == 0 || &cert->subject_alt_names == NULL) {
+		obj = Atom(0);


@Aurel300 fyi, I found another potential trap here:

Atom(t) returns an “atom” (zero-sized block) with tag t. Zero-sized blocks are preallocated outside of the heap. It is incorrect to try and allocate a zero-sized block using the functions below. For instance, Atom(0) represents the empty array.

This means that whenever we want to return an array from C externs, we have to deal with the length == 0 special case.

Simn · 2020-01-23T11:18:32Z

@Aurel300 Could you look into the OS X default certificate situation some time soon? I want to merge SSL support for the next release and this is blocking it at the moment.

Aurel300 · 2020-02-04T11:43:56Z

@Simn Sorry for the delay, this should be good now. Python failure is unrelated. For reference, the reason the LIB_PARAMS change in the Makefile didn't work on its own is because apparently if you specify a variable directly on the command line, it completely overrides it in the Makefile, so even things like += don't work.

Simn · 2020-02-04T12:12:04Z

Alright, thanks! I'm gonna merge this now.

For people building Haxe, please refer to the CI-related changes here. I think we've made this as straightforward as possible.

Simn added 6 commits December 4, 2019 17:46

start on eval-ssl

4e45bbb

add authmode config, use it for now because we don't have certs yet

e98f4d3

apply Aurel fixes

fc18c9c

clean up a bit

0d23788

start on certs

54703de

use int return like everyone else

becf5bc

Simn requested review from Aurel300, RealyUniqueName and kLabz December 5, 2019 08:57

kLabz reviewed Dec 5, 2019

View reviewed changes

Makefile Outdated Show resolved Hide resolved

remove rogue -cclib

83110ad

skial mentioned this pull request Dec 5, 2019

Haxe Roundup 508 skial/haxe.io#699

Closed

1 task

move NativeSocket to eval.vm

c2d8557

fix stupidity

678ca15

Aurel300 requested changes Dec 5, 2019

View reviewed changes

Simn added 9 commits December 5, 2019 13:36

apply more Aurel fixes

6f5dc09

break everything

cf96a93

see if this magically works

91ad8a0

remove unused include

a94586f

link

a2ec796

package files

c912fba

try mac

ba7538e

more libs

0f9089c

try building on linux

9984923

Simn added 5 commits December 11, 2019 08:57

rename native Certificate to X509Crt

1846024

add some Certificate functions

acb0ec4

deal with next and fix issuer/subject returns

58efece

Merge branch 'refs/heads/development' into eval-ssl

5211a66

be honest about what isn't implemented

804bc86

Aurel300 reviewed Dec 11, 2019

View reviewed changes

libs/mbedtls/mbedtls_stubs.c Show resolved Hide resolved

Aurel300 reviewed Dec 11, 2019

View reviewed changes

libs/mbedtls/mbedtls_stubs.c Outdated Show resolved Hide resolved

Aurel300 reviewed Dec 11, 2019

View reviewed changes

std/eval/_std/mbedtls/X509Crt.hx Show resolved Hide resolved

Simn added 4 commits December 12, 2019 08:43

clean up

21239a9

Merge branch 'refs/heads/development' into eval-ssl

fa11d3e

work towards API completion

277db4b

implement Certificate.get_altNames

cd8bafc

Simn commented Dec 12, 2019

View reviewed changes

Simn added 2 commits December 12, 2019 12:44

add Key API

39c108d

Merge branch 'refs/heads/development' into eval-ssl

26fc881

RealyUniqueName added this to the Release 4.1 milestone Dec 19, 2019

RealyUniqueName added the platform-eval Everything related to the Haxe 4 eval macro interpreter label Dec 19, 2019

Merge branch 'development' into eval-ssl

1befcbd

Aurel300 added 2 commits January 23, 2020 13:38

load root certificates on OS X

2d01975

fix lib params in azure build

9a2cff5

Aurel300 approved these changes Feb 4, 2020

View reviewed changes

Simn merged commit 2351868 into development Feb 4, 2020

Simn deleted the eval-ssl branch February 4, 2020 12:12

skial mentioned this pull request Feb 5, 2020

Haxe Roundup 515 skial/haxe.io#715

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] SSL support for eval #9009

[WIP] SSL support for eval #9009

Simn commented Dec 5, 2019 •

edited

Loading

RealyUniqueName commented Dec 5, 2019

andyli commented Dec 5, 2019

Simn commented Dec 5, 2019

Simn commented Dec 5, 2019

RealyUniqueName commented Dec 5, 2019

Simn commented Dec 5, 2019

Aurel300 Dec 11, 2019

Simn Dec 11, 2019

Simn Dec 12, 2019

Simn commented Dec 12, 2019

Simn Dec 12, 2019

Simn commented Jan 23, 2020

Aurel300 commented Feb 4, 2020

Simn commented Feb 4, 2020

[WIP] SSL support for eval #9009

[WIP] SSL support for eval #9009

Conversation

Simn commented Dec 5, 2019 • edited Loading

LIB_PARAMS

Default certificate loading

Amount of .c and .h files

SNI stuff

Rest of the sys.ssl package

Overall testing

Packaging and licensing

RealyUniqueName commented Dec 5, 2019

andyli commented Dec 5, 2019

Simn commented Dec 5, 2019

Simn commented Dec 5, 2019

RealyUniqueName commented Dec 5, 2019

Simn commented Dec 5, 2019

Aurel300 Dec 11, 2019

Choose a reason for hiding this comment

Simn Dec 11, 2019

Choose a reason for hiding this comment

Simn Dec 12, 2019

Choose a reason for hiding this comment

Simn commented Dec 12, 2019

Simn Dec 12, 2019

Choose a reason for hiding this comment

Simn commented Jan 23, 2020

Aurel300 commented Feb 4, 2020

Simn commented Feb 4, 2020

Simn commented Dec 5, 2019 •

edited

Loading