feat: X509.new now takes a keyword arguments

[mcr]

allowing the openssl propq to be set.
Unclear what else might be required going forward, which is why keyword arguments.

It uses X509_new_ex() to create the X509 object, allowing a provider to be set. If the provider is not set, then a key residing in a provider (such as a tpm2) can not be used

I am looking for review: I do not feel confident that I've handled all situations, but the unit tests do pass.

[rhenium]

Like I asked in your other PRs, please add a test case and adjust code formatting to match the existing style. We use 4 spaces per indentation level and put a space between if (condition).

This patch adds something ruby/openssl doesn't currently support, and I agree it could be useful. Adding as a keyword argument makes sense to me, too.

It uses X509_new_ex() to create the X509 object, allowing a provider to be set. If the provider is not set, then a key residing in a provider (such as a tpm2) can not be used

But I don't think it works as described. The "propq" string is used for "fetching algorithms", and within X509 I think the only algorithms fetched are EVP_MD (perhaps more, but not EVP_PKEY keys). Wouldn't you need to specify propq when fetching key objects, e.g., in OpenSSL::PKey.read or #934 instead?

[rhenium]

X509.new(string, der: another_string) is ambiguous and I don't think a keyword argument for the input is very useful. I'd prefer to allow just propq this time.

Also, because we now use C99, we can simplify it:

ID table[] = { rb_intern("propq") };

[rhenium]

Suggested change

	propq = (char *)RSTRING_PTR(values[1]);
	propq = StringValueCStr(values[1]);

We have to check that the argument is actually a String object and that the content is NUL-terminated.

[mcr]

But I don't think it works as described. The "propq" string is used for "fetching algorithms", and within X509 I think the only algorithms fetched are EVP_MD (perhaps more, but not EVP_PKEY keys). Wouldn't you need to specify propq when fetching key objects, e.g., in OpenSSL::PKey.read or #934 instead?

First, it's a draft. I will fix the formatting.
Test cases will be complex: they will require a provider to be loaded, which means available. Easiest is swtpm.
I have test cases that do that, but they aren't going to be easy to run.

As to whether this is useful or required... I am arguing your point on the openssl-users list.
Yet, they do not yet agree.

I agree that (string, :der => string) is ambiguous. So one or other, do you think?
I didn't want to do something weirder, moving towards keywords arguments seemed like the least confusing thing.

[rhenium]

We just have to ensure that the code paths in ruby/openssl are actually reached and that the propq string is correctly passed to OpenSSL. I think you may be able to use MD4 in the "legacy" provider, for example, and check that different propq strings show different behaviors.

As to whether this is useful or required... I am arguing your point on the openssl-users list. Yet, they do not yet agree.

I imagine it depends heavily on the particular provider and the use case. The propq on X509 would be useful sometimes, but I was just wondering if the use case described in the commit message actually needs it.

It would be very helpful if you could provide a working example code, whether in C or with some OpenSSL bindings.

I agree that (string, :der => string) is ambiguous. So one or other, do you think? I didn't want to do something weirder, moving towards keywords arguments seemed like the least confusing thing.

The existing positional parameter doesn't have to be converted at the same time. Let's just support Certificate.new(der_string, propq: "propq string").