Allow signing to exclude hash constant

[atagar]

Hi lovely cryptography folks. I just added the ability to sign server descriptors similar to what the tor process does but ran into one rough bit. In particular when cryptography makes an RSA signature it embeds a constant indicating the hash it uses whereas tor does not. This is buried pretty deep within cryptography's innards so I had to do an icky hack to be compatible with tor.

No doubt cryptography is doing the perfectly right thing here, but would you mind adding an option either to RSAPrivateKey's sign() method or the MultiBackend to omit this so we can drop the hack? In particular it's just two lines in the _rsa_sig_sign() method of cryptography/hazmat/backends/openssl/rsa.py we'd like to avoid...

197     res = backend._lib.EVP_PKEY_CTX_set_signature_md(
198         pkey_ctx, evp_md)
199     backend.openssl_assert(res > 0)

... to...

if include_hash_constant:
  res = backend._lib.EVP_PKEY_CTX_set_signature_md(pkey_ctx, evp_md)
  backend.openssl_assert(res > 0)

Cheers! -Damian

[alex]

What's the use case for this? We're implementing PSS and PKCS1v1.5, both of which are defined to include the hash oid. Removing this would leave us with some bizzare hybrid.

[atagar]

Hi Alex. The use case is what I mention above: I'm attempting to sign tor server descriptors in the same way tor does, and doing so requires mocking out part of cryptography's internals.

[reaperhulk]

Looks like the use case is compatibility with the way tor does signatures, although I'd like to see the actual tor signature format to understand what's going on here. Do you have a sample of a signature we can look at?

[atagar]

Hi Paul. Certainly. Here's the tor specification and example descriptors...

https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n590
https://gitweb.torproject.org/torspec.git/tree/dir-spec.txt#n542
https://collector.torproject.org/recent/relay-descriptors/server-descriptors/

[atagar]

Oh, and just to be clear - all I'm asking for here is a flag to optionally omit the oid. The default behavior should absolutely be what it presently is, just hoping for more flexibility so we don't need to mock out cryptography internals to omit the oid. That's all. :)

[alex]

Whatever solution we come to here, it will not be adding a flag to the top level API. Just adding flags everywhere burdens every developer using it at the expense of an edge case. (This is a general principle of API design, which is only exacerbated by the difficulty and subtlty of crypto)

I still don't totally understand what tor's objective is here, but if we decide to support it we're going to need to think more deeply about what the right API is.

[atagar]

Absolutely makes sense Alex. Cryptography isn't one of my strong suits (I author tor's python bindings), but reached out to Nick for his two cents as tor's C codebase author.

I'm more than happy for this to be an obscure option. 'Easy things should be easy, and hard things should be possible.' Feels odd to quote Larry Wall but simply hoping to avoid necessitating a hack on private functions - that's all.

[nmathewson]

I still don't totally understand what tor's objective is here

The only real rationale is backward compatibility. We did it that way back in 2003, and the motivation for adding an OID has never been strong enough to make a backward-compatibility break worth it.

[alex]

That's useful to know, I appreciate the context, thanks.

[atagar]

For what it's worth one quick idea to make this obscure would be a backend attribute. That is to say...

backend = default_backend()
backend.oid_in_signature = False

private_signing_key = rsa.generate_private_key(
  public_exponent = 65537,
  key_size = 1024,
  backend = backend,
}

This would avoid cluttering the constructor or sign method, but still make it possible. Just food for thought - I'd be happy to provide a patch if that would be helpful.

[reaperhulk]

I traced what doing this actually makes OpenSSL do. EVP_PKEY_CTX_set_signature_md sets md on the RSA_PKEY_CTX. When you call sign it checks for whether md is NULL (https://github.com/openssl/openssl/blob/master/crypto/rsa/rsa_pmeth.c#L127) and then when it is just bare PKCS1v15 pads it with no ASN.1 (https://github.com/openssl/openssl/blob/master/crypto/rsa/rsa_pmeth.c#L176).

So it looks like one potential path here would be to support it with a new allowable type passed to algorithm.

private_key.sign(data, padding, NoAlgorithm())

[atagar]

Certainly sounds like a fine option to me. One of the things I tried when puzzling over this was...

digest = hashlib.sha1(content)
private_signing_key.sign(digest, padding.PKCS1v15(), None)

This balked since sign requires an algorithm of some sort. If there was a NoAlgorithm that would be perfect for us.

[xzr]

Hey, I ran into the same issue while working on some code signing systems that have previously been using openssl pkeyutl for signatures. (Signatures are then enforced in the firmware of a device)

For reference:
https://www.openssl.org/docs/man1.0.2/man1/pkeyutl.html

"In PKCS#1 padding if the message digest is not set then the supplied data is signed or verified directly instead of using a DigestInfo structure. If a digest is set then the a DigestInfo structure is used and its the length must correspond to the digest type."

Would like to see this implemented so I can migrate more of the stuff over to cryptography :)

[alex]

We've made no progress on this in 3.5+ years, and there's exactly one consumer who needs this feature. I think I'm going to close this as wontfix.