Support DSA signatures of SHA-HMAC output objects

[koolfy]

Hello,
I've been working at a ridiculous slow pace on porting the python OTRv3-ish implementation over at pure-python-otr from pycrypto to your great library.

I've hit a roadlock recently with DSA signatures, as I'm required to sign the output of a HMAC-sha construct. However, your current code does not allow for that, sadly.
I understand and am completely behind the idea if having OIDs to check we're not doing cryptocrap accidentally, but I am seeing here:

pycryptodome/lib/Crypto/Hash/HMAC.py

Line 46 in 6fb4ca4

class HMAC(object):

that HMAC objects do not have any kind of OIDs, which is actually a shame as elsewhere you do sort of associate them with one:

pycryptodome/lib/Crypto/IO/_PBES.py

Line 57 in 41300af

_OID_HMAC_SHA256 = "1.2.840.113549.2.9"

There, the OID of SHA256HMAC() is "1.2.840.113549.2.9"

However, the validation for DSA signatures is as follow:
https://github.com/Legrandin/pycryptodome/blob/master/lib/Crypto/Signature/DSS.py#L279

def _valid_hash(self, msg_hash):
    """Verify that SHA-1, SHA-2 or SHA-3 are used"""
    return (msg_hash.oid == "1.3.14.3.2.26" or
            msg_hash.oid.startswith("2.16.840.1.101.3.4.2."))

Thus, I of course hit the error:

AttributeError: 'HMAC' object has no attribute 'oid'
(on top of the OID not meeting this criteria, even if it was set as a HMAC object attribute)

Signing HMAC outputs appears to be impossible in the current state of the library, and as I discuss it here: python-otr/pure-python-otr#68 (comment) it's leaving me stuck with pycrypto, which is a maintenance nightmare for distributions downstream :(

I do understand that fixing this would require a bit of a rework, but I think DSA signature of HMAC outputs is a legitimate usecase, if not for OTR, for other people who might need such a construct in the future.

If I can be of any assistance in helping out with the effort, please let me know, if I trusted myself with such a rework I would have attempted a pull request, but I would hate to f* something up :/

[Legrandin]

Most probably you are instantiating Crypto.Signature.DSS with parameter mode='fips-186-3: as the name implies, it follows FIPS standard 183-3 which officially only works with approved hash functions defined in FIPS 180-3. HMAC is not part of that.

As an alternative, I suggest you create the signing object with mode='deterministic-rfc6979', which doesn't restrict you in the actual hash algorithm. Also as the name implies, the signature will become deterministic, i.e. the same message and the same signing key will produce always the same signature, whereas with FIPS (EC)DSA that changes every time. While that has several security benefits, please evaluate carefully if that introduces any risk into your protocol.

[koolfy]

I will try that, and I will evaluate just that, I hope it doesn't break everything, thanks for the heads up!

[koolfy]

@Legrandin I'm not 100% sure of my conclusions so I don't dare open an invalid bug report, I'll comment on this closed ticket instead so you can check how wrong I am about what follows:

I've tried what you suggested, not yet checked the protocol implications but I'm trying to get the code working in the first place.
I think I've hit a strange bug, but it's unlikely I'm the first one to hit it so I'd say there is a 9/10 chance I'm just doing something stupid.

The issue is triggerred by the deterministic-rfc6979' DSS signature of a HMAC object.

I get the following exception:

src/potr/crypt.py line 472 in calculatePubkeyAuth
  MB = self.privkey.sign(SHA256HMAC(mackey, buf))
src/potr/compatcrypto/pycrypto.py line 90 in sign
  signature = signer.sign(data)
/home/user/.local/lib/python2.7/site-packages/Crypto/Signature/DSS.py line 100 in sign
  nonce = self._compute_nonce(msg_hash)
/home/user/.local/lib/python2.7/site-packages/Crypto/Signature/DSS.py line 219 in _compute_nonce
  self._bits2octets(h1), mhash).digest()
/home/user/.local/lib/python2.7/site-packages/Crypto/Hash/HMAC.py line 213 in new
  return HMAC(key, msg, digestmod)
/home/user/.local/lib/python2.7/site-packages/Crypto/Hash/HMAC.py line 80 in __init__
  raise ValueError("Hash type incompatible to HMAC")
ValueError: Hash type incompatible to HMAC

made clearer by manually removing the try except from

pycryptodome/lib/Crypto/Hash/HMAC.py

Line 70 in 6fb4ca4

try:

which then gives us the exact line:

 /home/user/.local/lib/python2.7/site-packages/Crypto/Hash/HMAC.py line 213 in new
   return HMAC(key, msg, digestmod)
 /home/user/.local/lib/python2.7/site-packages/Crypto/Hash/HMAC.py line 71 in __init__
   if len(key) <= digestmod.block_size:
AttributeError: 'HMAC' object has no attribute 'block_size'

I nearly lost my mind, but then found that during the signature attempt, completely unrelated to my HMAC object, pycryptodome internally constructs some HMAC in compute_nonce:

pycryptodome/lib/Crypto/Signature/DSS.py

Line 215 in 39626a5

nonce_k = HMAC.new(nonce_k,

This and the few following calls to HMAC.new() all provide, as a third parameter the mhash variable, which actually contains the input data object and HMAC.new() expects this third parameter to be a digestmod (for example a SHA256 object).
Reading the associated RFC: https://tools.ietf.org/html/rfc6979#section-3.1.1 and 3.2
I find that, indeed, these HMAC constructs should use the same hash primitive as the input data, so as I understand the code intends to retrieve the hash object from the mhash (input data) and feed it into the HMAC.new() digestmod parameter.

This can only work when the input data happens to be a valid digestmod object.

The good news is the HMAC object does actually contain a reference to its hashing primitive under the ._digestmod attribute, which would actually work as a way to pass along the right object inside compute_nonce but naked hash objects don't have such a self-referencing ._digestmod attribute, thus blindly adding ._digestmod to the HMAC instanciations would break naked hash deterministic signatures.

As this does not appear to be a trivial fix in term of design I can't provide a PR, I'm not sure what design would be best :(

I hope my understanding is correct this time around, if not sorry for wasting your time again on this issue :)