Add support for X509_NAME_print_ex(3)

[overhacked]

Fixes #1637, by adding two methods:

• X509NameRef.format_bytes()
• X509NameRef.format()

format_bytes() is a proxy for X509_NAME_print_ex(3), just renamed to Rust vernacular. For a more ergonomic API in the most common use case, format() modifies the flags passed by the caller to ensure that OpenSSL returns UTF-8 output, then converts the BIO bytes returned to a Rust String.

Tests included ensure that:

• The most commonly-used flags, as taken from the OpenSSL man page, return expected output
• The special case of XN_FLAG_COMPAT works as expected
• Printing the name of a certificate with Unicode characters, emoji in this case, functions with both format() and format_bytes()

[overhacked]

I'd love to get this merged, but it currently fails under the unstable BoringSSL support. bssl-sys incorrectly types the XN_FLAG_* macros as i32 when they are actually c_ulong. I handled this in openssl-sys by adding constant definitions to openssl-sys/src/x509.rs, but bssl-sys is upstream?

This could be handled for bssl-sys by using the bindgen::callbacks::ParseCallbacks::int_macro API, but that would only work for #[feature(bindgen)], because the int_macro callback isn't exposed in the bindgen CLI.

[sfackler]

You can define a cfg-dependant type for the flags, like we do in a few other places:

rust-openssl/openssl/src/lib.rs

Lines 187 to 190 in 0a34876

	#[cfg(boringssl)]
	type LenType = libc::size_t;
	#[cfg(not(boringssl))]
	type LenType = libc::c_int;

[overhacked]

I'd love to get the X509_NAME_print_ex support merged, even if it only works on non-BoringSSL builds until the upstream bug is fixed or somehow mitigated. I put some effort into figuring out how to mitigate, without success. Detailed notes follow.

Mitigation possibilities

openssl-sys doesn't control the declaration of the constant, it gets it from bssl-sys. Changing the type of the constant to u32 wouldn't help, anyway, because the BoringSSL API function (X509_NAME_print_ex) expects flags as a c_ulong. I opened an upstream bug for BoringSSL, because bssl-sys needs to solve the issue either 1️⃣ the same way openssl-sys does, by omitting constants from bindgen and maintaining them by hand, or 2️⃣ by using the ParseCallbacks::int_macro API to change the type of selected groups of macro constants.

I tried an approach to override the const types like openssl-sys does for OpenSSL and LibreSSL. That won't work with bssl_sys, because the glob re-exports conflict (rustc seems to omit the const symbol in the exported library).

An alternate solution would be to drop the use of bssl-sys altogether, and use only the bindgen code that currently only gets hit when building on a system where the default SSL is BoringSSL and feature unstable_boringssl is disabled. That would allow using the ParseCallbacks API (or even using hand-rolled consts) with BoringSSL, BUT the "bindgen CLI" building approach would have to be dropped, because the CLI doesn't support ParseCallbacks::int_macro.

I don't understand why there is a feature = "bindgen" and not(feature = "bindgen") way of calling bindgen for BoringSSL. Don't they both depend on bindgen but in different ways? I'll admit I'm far from deep enough into rust-openssl development to know for sure.

[sfackler]

What about the thing I suggested?

[overhacked]

I don’t think the cfg-gated type aliases will work, because the function signature is the same for both OpenSSL and BoringSSL. The API isn’t different, just bssl-sys generates the wrong type for the macro-defined constants.

I submitted a patch to BoringSSL… https://boringssl-review.googlesource.com/c/boringssl/+/62146

[davidben]

Ultimately the constants are due to a bindgen bug and should be fixed there. Alas there doesn't seem to be any movement in fixing that, so we'll have to workaround it. I've got a CL in flight to fix it in a way that'll work a bit better for us. (Reproducible builds are important to us, so the build.rs mess is a bit of a problem.)

That said, I would recommend rejecting this PR. This project has gotten into trouble before with using the various string-based OpenSSL APIs, and this is introducing yet more of them. Most of the flags this PR proposes to export are pretty incoherent. One even changes the return value convention of a function.

If you must encode a name as a string, I'd suggest sticking to the RFC encoding, as that's the only well-defined one. Even then, that encoding is not injective. Different names will give the same string encoding. It is also not even stable across OpenSSL versions! The encoding can change as they add support for new OIDs types because they cross-reference it against the whole OID table. (Probably not a good idea.) So at minimum, the API should come with a suitable warning about when it is and isn't safe to use.