sys.ssl.Certificate API

[Simn]

While working on #9009 I noticed that the existing sys.ssl.Certificate API (on neko, cpp and hl) offers various functions that create new certificates. https://github.com/HaxeFoundation/hashlink/blob/master/libs/ssl/ssl.c calls mbedtls_x509_crt_init in these functions:

• cert_load_file
• cert_load_path
• cert_load_defaults
• cert_add_pem
• cert_add_der

The mbedtls x509_crt API works differently in that functions operate on a single chain instance and then "add them to the chained list". This means that through the Haxe API, we couldn't create a certificate chain that involves, for instance, two parse_file calls.

It seems more straightforward to have a real constructor for Certificate and then define all these chaining operations as instance methods:

var cert = new Certificate();
cert.loadFile(file1);
cert.loadFile(file2);

I don't know if this is something worth changing, but we should at least discuss it. I'd be willing to work on this, though I'm concerned about the fact that we have zero tests for this stuff.

[Simn]

Another thing I don't understand in the actual implementations is this __h value:

class Certificate {
	var __h:Null<Certificate>;
	var __x:CertificatePtr;

	@:allow(sys.ssl.Socket)
	function new(x:CertificatePtr, ?h:Null<Certificate>) {
		__x = x;
		__h = h;
	}

The only place this is read at is the next function:

	public function next():Null<Certificate> {
		var n = NativeSsl.cert_get_next(__x);
		return n == null ? null : new Certificate(n, __h == null ? this : __h);
	}

Seems to me like this is entirely pointless because all we do with it is pass it through to other instances.

[Aurel300]

It looks like that code is almost trying to emulate something like a chain in mbedtls. __h is the "head" of the certificate list, __x are the native instances. Is it used in other code at all? Maybe via untyped? Or maybe from native Neko/HL/Hxcpp parts?

[Simn]

Another thing I find strange is that various functions have a built-in looping mechanism like so:

HL_PRIM vbyte *HL_NAME(cert_get_issuer)(hl_ssl_cert *cert, vbyte *objname) {
	mbedtls_x509_name *obj;
	int r;
	const char *oname, *rname;
	obj = &cert->c->issuer;
	if (obj == NULL)
		hl_error("Invalid issuer");
	rname = (char*)objname;
	while (obj != NULL) {
		r = mbedtls_oid_get_attr_short_name(&obj->oid, &oname);
		if (r == 0 && strcmp(oname, rname) == 0)
			return asn1_buf_to_string(&obj->val);
		obj = obj->next;
	}
	return NULL;
}

This corresponds to the Haxe function public function issuer(field:String):Null<String>. This makes it look like some sort of convenience function, but the underlying inconvenience isn't exposed: issuer is a data field on the mbedtls_x509_crt structure, which we cannot access because all we expose is this looping function. At the same time, we do expose next, so in theory we can browse a certificate chain. It's just that we can't do anything meaningful with it.

Has this API ever been used before? If so, I'd love to see some usage sample so I get a better understanding of what exactly we're exposing here and why.

[Simn]

Oh I see, this isn't the next from the certificate chain but instead belongs to this data structure: https://tls.mbed.org/api/structmbedtls__asn1__named__data.html

And fields corresponds to this from https://www.ssl.com/faqs/common-name/:

Organization (O), Organizational Unit (OU), Country (C), State (S), Locality (L), and Common Name (CN)

[Simn]

Key has this API:

	static function readPEM(data:String, isPublic:Bool, ?pass:String):Key;
	static function readDER(data:haxe.io.Bytes, isPublic:Bool):Key;

This won't allow a DER format with a password, which is not something mbedtls_pk_parse_key forbids:

Parse a private key in PEM or DER format.

In fact, the HL implementation calls the same function with or without password in the PEM case:

	else if (pass == NULL)
		r = mbedtls_pk_parse_key(pk, buf, len, NULL, 0);
	else
		r = mbedtls_pk_parse_key(pk, buf, len, (const unsigned char*)pass, strlen((char*)pass));

It seems like the only distinction at Haxe-level should be between the Bytes vs. String argument. Natively, the functions mbedtls_pk_parse_public_key and mbedtls_pk_parse_key are both documented to support DER and PEM, so there shouldn't be a reason to restrict this in the API.

[Simn]

Well, I guess nobody really cares about this.

[Aidan63]

I ran into some of these issues and more while attempting to implement a non mbedtls ssl for hxcpp (HaxeFoundation/hxcpp#1135).

For subject and issuer based on the implementation you're expected to pass in a short name string (e.g. 'CN'), but some distinguished names have multiple short names. For example "State or Province" can be represented by 'ST', 'SP', or 'S', mbedtls uses 'ST' but wincrypt uses 'S'...

Could we change this to say that you must pass in a valid OID string instead (e.g. '2.5.4.3' for CommonName) to remove this ambiguity? We could provide an enum abstract with common OIDs since they're not very intuative.

enum abstract DistinguishedNames(String) to String {
    var CommonName = '2.5.4.3';
}

When I first ran into this I looked around github and it seems the only "real" (non test code) user of these two functions is an OpenFL certificate class, and the function which uses them doesn't seem to be used in any other projects so while technically breaking behaviour the number of people effected would probably be close to zero.