On Wed, Jun 01, 2022 at 11:00:12AM +1000, Alex Wilson wrote:
> I'm trying to sign X509 and X509_CRL objects without using X509_sign et al
> -- since the key I'm signing with isn't in the memory of the process doing
> this and you can't write an ENGINE for something that can't sign a
> pre-calculated hash and needs the whole data.
>
> To do this I have to alter the signing algorithm, both in the outer X509 and
> the inner X509_CINF (and similar on the X509_CRL). Since X509 and X509_CRL
> became opaque, I've had to use some hacks to achieve this:
>
> For the outer signature and algo I can use X509_get0_signature() and
> X509_CRL_get0_signature() and then cast off the const on the X509_ALGOR.
> Then I can use X509_ALGOR_set0() to set the algorithm in use.
>
> For the X509 inner algorithm I can use X509_get0_tbs_sigalg() and cast the
> const off there, too. But there is no equivalent of this for X509_CRL.
This sounds quite familiar...
> As a result I have a local patch which adds X509_get_tbs_sigalg() (returning
> a non-const pointer) and X509_CRL_get{,0}_tbs_sigalg(). It's very short, so
> I was hoping somebody could let me know if this is a terrible idea and
> whether there is any possibility of it going back in LibreSSL (or if I
> should try to take it up with OpenSSL and then port it back or something?)
>
> I have also considered adding an X509_get_signature() which returns
> non-const, as well. If any of this seems like a decent idea at all, I'm
> happy to add that and write some tests.
I would be ok with adding X509_CRL_get0_tbs_sigalg(), as this looks like
a straight up oversight. Since OpenSSL are quite liberal with adding
accessors, this should also be easy to upstream.
That one function should be good enough for your needs even if it means
some annoying casting away of const. It seems a bit of a fringe use
case, so I would rather not add more functions to the public API for
this.
As an aside, using _get_ is unfortunate and I'd rather avoid it. This
could mean get0, get1, have const or no const, depending on the phase of
Venus... I don't know OpenSSL's current stance on such cases, i.e.,
whether they added more stuff like X509_CRL_get0_tbs_sigalg_noconst().
For now I'd suggest you send a patch for X509_CRL_get0_tbs_sigalg()
with the prototype gated with #if defined(LIBRESSL_CRYPTO_INTERNAL). We
would then expose that with the next library bump, which will almost
certainly happen before 3.6 goes -stable.
If you want to add a short regress, that would be great, but it's not
strictly required for a simple accessor. I'd probably extend
regress/lib/libcrypto/x509/. You'll want static linking and you'll need
to extend CFLAGS with something like
LDADD_foo = ${CRYPTO_INT}
CFLAGS += -DLIBRESSL_CRYPTO_INTERNAL
>
> Brief patch attached.
> commit 6f3f7990686517b788278fc26e706e2f0c7472cf
> Author: Alex Wilson
> Date: Wed Jun 1 10:49:08 2022 +1000
>
> libcrypto: want to alter tbs sigalg for X509 and X509_CRL
>
> diff lib/libcrypto/asn1/x_crl.c lib/libcrypto/asn1/x_crl.c
> --- lib/libcrypto/asn1/x_crl.c
> +++ lib/libcrypto/asn1/x_crl.c
> @@ -722,6 +722,18 @@ X509_CRL_get_lastUpdate(X509_CRL *crl)
> return crl->crl->lastUpdate;
> }
>
> +const X509_ALGOR *
> +X509_CRL_get0_tbs_sigalg(const X509_CRL *crl)
> +{
> + return crl->crl->sig_alg;
> +}
> +
> +X509_ALGOR *
> +X509_CRL_get_tbs_sigalg(X509_CRL *crl)
> +{
> + return crl->crl->sig_alg;
> +}
> +
> const ASN1_TIME *
> X509_CRL_get0_nextUpdate(const X509_CRL *crl)
> {
> diff lib/libcrypto/x509/x509.h lib/libcrypto/x509/x509.h
> --- lib/libcrypto/x509/x509.h
> +++ lib/libcrypto/x509/x509.h
> @@ -399,6 +399,8 @@ X509_NAME *X509_CRL_get_issuer(const X509_CRL *crl);
> STACK_OF(X509_REVOKED) *X509_CRL_get_REVOKED(X509_CRL *crl);
> void X509_CRL_get0_signature(const X509_CRL *crl, const ASN1_BIT_STRING
> **psig,
> const X509_ALGOR **palg);
> +const X509_ALGOR *X509_CRL_get0_tbs_sigalg(const X509_CRL *crl);
> +X509_ALGOR *X509_CRL_get_tbs_sigalg(X509_CRL *crl);
>
> int X509_REQ_get_signature_nid(const X509_REQ *req);
>
> @@ -768,6 +770,7 @@ int ASN1_item_sign_ctx(const ASN1_ITEM *it,
>
> const STACK_OF(X509_EXTENSION) *X509_get0_extensions(const X509 *x);
> const X509_ALGOR *X509_get0_tbs_sigalg(const X509 *x);
> +X509_ALGOR * X509_get_tbs_sigalg(X509 *x);
> int X509_set_version(X509 *x, long version);
> long X509_get_version(const X509 *x);
> int X509_set_serialNumber(X509 *x, ASN1_INTEGER *serial);
> diff lib/libcrypto/x509/x509_set.c lib/libcrypto/x509/x509_set.c
> --- lib/libcrypto/x509/x509_set.c
> +++ lib/libcrypto/x509/x509_set.c
> @@ -77,6 +77,12 @@ X509_get0_tbs_sigalg(const X509 *x)
> return x->cert_info->signature;
> }
>
> +X509_ALGOR *
> +X509_get_tbs_sigalg(X509 *x)
> +{
> + return x->cert_info->signature;
> +}
> +
> int
> X509_set_version(X509 *x, long version)
> {
