dsa: expose signing and verifying of prehashed hash value

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.

[cobratbq]

Expose the ability for the user to provide a prehashed hash-value for signing/verification. In rare cases, this is needed in other protocols. I have attempted to solve this with a custom Digest implementation, but eventually ran into (forced) finalization which affects the hash value.

[aumetra]

Would you mind changing the code to expose the prehashed signing/verifying functionality via the hazmat traits of the signature crate (just for the sake of having a unified interface)?
You would need to adjust the version requirements of signature to a minimum version of 1.6 (1.6.4 specifically if you want to implement RandomizedPrehashSigner, too).

[cobratbq]

Sure, I'll have a look. Thanks for pointing out a direction.

[cobratbq]

@aumetra can you confirm that there is no component of randomness in this logic? The crate::generate::secret_number_rfc6979 does not seem to depend on randomness.

I will follow up later: I've got some things to figure out, e.g. whether the trait needs the D trait (Digest + OutputSize + ..), whether the randomness should be in and I'm currently using an undesirable implementation, etc.

[tarcieri]

@aumetra for now you can just do the deterministic API.

RFC6979 supports additional randomness, but that can be added as a followup.

[aumetra]

can you confirm that there is no component of randomness in this logic?

Yes, the logic of the function (as it currently is) is fully deterministic

[tarcieri]

@cobratbq looks like one clippy nit for a needless borrow

[tarcieri]

Yes, the logic of the function (as it currently is) is fully deterministic

@aumetra it actually also supports using an RNG:

signatures/dsa/src/signing_key.rs

Lines 108 to 124 in a440df1

	impl<D>RandomizedDigestSigner<D,Signature>forSigningKey
	where
	D:Digest,
	{
	fntry_sign_digest_with_rng(
	&self,
	mutrng:implCryptoRng + RngCore,
	digest:D,
	) -> Result<Signature, signature::Error>{
	let ks = crate::generate::secret_number(&mut rng,self.verifying_key().components())
	.ok_or_else(signature::Error::new)?;
	let hash = digest.finalize();
	self.sign_prehashed(ks,&hash)
	.ok_or_else(signature::Error::new)
	}
	}

[aumetra]

Oh yeah, I was only talking about the function crate::generate::secret_number_rfc6979 since they asked explicitly about that one

[cobratbq]

@tarcieri@aumetra quick follow-up, I'd appreciate your feedback

1. I added an assertion to ensure hash length is at least the same strength as modulus. Before it just took the minimum of two numbers.
2. verify_prehash is implemented, seems straight-forward enough. No issue there.
3. sign_prehash, IIUC, cannot be implemented with this combination of trait PrehashSigner<Signature> and the requirement of a D: Digest + BlockSizeUser + FixedOutputReset for crate::generate::secret_number_rfc6979.

Do you have an idea what to do with (3)? The type for D has to be somewhere, and on the trait seems to make most sense. (Branch, currently, does not build but does illustrate the choke point.)

[tarcieri]

@cobratbqDigestSigner is already the trait which has a D parameter.

The ECDSA crate uses a curve-specific digest when implementing PrehashSigner:

https://github.com/RustCrypto/signatures/blob/3a13e77/ecdsa/src/sign.rs#L268

Failing that you could just use e.g. SHA-256, or some other SHA2-family digest which has test vectors in RFC6979.

[cobratbq]

@tarcieri I've been looking into the ecdsa implementation. From what I can tell, I suspect that the PrehashSigner trait is currently ill-defined: you rely on that trait to define deterministic signing behavior that needs to have a Digest defined, however it is not part of this trait.

It works for ecdsa because of luck, i.e. it is coincidentally already defined on the implementation. For dsa that won't work, because the spec allows for any number of approved digests, while PrehashSigner requests/requires an input parameter prehash with a run-time bound: bit-length at least as large as bit-length of digest. (Also, DigestSigner is not necessarily present if itself not required.)

Is this something you would want to change/fix or should I work around it, like effectively (probably by accident) happened with ecdsa?

[aumetra]

The logic in ecdsa isn't really ill-defined. Incompatible (in specific cases) with implementations that allow free definition of the used digest function, sure, but not wrong.
The crux is that ECDSA has recommended hash digests for each curve. P256 -> SHA-256, P384 -> SHA-384, you get the picture.
To cite the FIPS 186-4:

It is recommended that the security strength associated with the bit length of n and the security
strength of the hash function be the same unless an agreement has been made between
participating entities to use a stronger hash function

The ecdsa crate uses the associated type of the recommended digest to generate the ad-hoc key with.

DSA however doesn't really have a "recommended" hash digest. Most of the standards reference SHA-1 which is actually pretty awful because, you know.
This is actually tangentially related to a discussion in #520, where we are practically still trying to figure out which digest should be used for a potential Signer/Verifier implementation.

---

Also regarding your change where you added the assertion on the hash length and the size of parameter q, if I read the standard correctly, this restriction only applies to RSA (specifically RSA key pair generation).

The full quote is as follows:

An approved hash function, as specified in FIPS 180, shall be used during the generation of key
pairs and digital signatures. When used during the generation of an RSA key pair (as specified in
this Standard), the length in bits of the hash function output block shall meet or exceed the
security strength associated with the bit length of the modulus n (see SP 800-57).

The quote can be found in FIPS 186-4, under section 5.1 "RSA Key Pair Generation"

[tarcieri]

@cobratbq the current shape of the ecdsa crate is based on years of iteration and hard-won knowledge about how RFC6979 is deployed alongside ECDSA in many real-world scenarios. In fact, we have to accommodate cases where the digest used for instantiating HMAC-DRBG is different from the one used to hash the message.

We previously did what I suspect you're suggesting: always use the D parameter of DigestSigner to instantiate HMAC-DRBG. That felt great at first, but we ended up with many, many requests to match existing test vectors for systems which use two different hashes, namely Keccak256 for message hashing and SHA-256 for RFC6979 as commonly used in the Ethereum ecosystem.

Unless you have such specific requirements as well, you can instantiate RFC6979 with the secure cryptographic digest of your choice, which is why I recommended SHA-256 as a reasonable default. Again, it doesn't have to match what was used to hash the message, and really the only party who can discern what hash was used was the signer. The only time it really matters at all is if you are trying to match specific deterministic test vectors.

PrehashSigner doesn't have a D parameter by design: the DigestSigner trait provides that. If you really, really want to have a generic parameter for D when using PrehashSigner, it needs to be defined on the type, not the trait.

[cobratbq]

@aumetra: okay, you are right. Let me rephrase. Given the way DSA is defined, there is a open variable for the digest to be used. (Instead of a single prescribed digest, a set of possible values is defined.) So, this is effectively requires extending the trait, i.e. introducing a new variable component. For ecdsa these can be hard-coded to the prescribed digest, just not for dsa. For practical convenience, maybe it needs to be structured differently so that the definition of PrehashSigner itself isn't changed. I'm questioning the trait definition, because:

1. I'd like to avoid introducing a specific hash-implementation where only a abstract concept is needed. (Sha1 i.s.o. Digest, for example, as sha1 is only a dev-dependency now.)
2. From my limited understanding, it seems that PrehashSigner closely relates to the secret_number_rfc6979, except that the one "variable" is exposed.
3. I'd like to avoid contributing an incomplete workaround, if possible/reasonable.

@aumetra thanks for catching that, I see that in traversing the spec, I accidentally ended up with something RSA specific. That's my fuck-up.

@tarcieri actually, it's the inverse: with my current understanding, I would define a D type on PrehashSigner. In implementations of ecdsa it may end up "hard-coded", in other implementations it may correspond to D in DigestSigner, however in the basis it is a free variable. (And if I understand your examples, this should not be an issue given there is no constraint to DigestSigner.)
You mentioned: "... no D defined by design, because the DigestSigner provides that." Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?) depend on PrehashSigner, but the reverse is not true, IIUC.

edit@tarcieri comment regarding evolution of ecdsa should not be contradicted or negated by my comments. If you think so, LMK.

[tarcieri]

I would define a D type on PrehashSigner.

Again, PrehashSigner doesn't have a D parameter by design. PrehashSigner is explicitly designed to be able to handle different hash inputs at runtime. The ecdsa crate implements this logic:

See:

• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/sign.rs#L264
• https://github.com/RustCrypto/signatures/blob/8c73683/ecdsa/src/hazmat.rs#L227-L251

Even if we were to abandon that requirement, a D parameter doesn't make sense in all of the potential contexts where PrehashSigner would be used.

Doesn't that mean that you are creating a dependency that is often not valid? (As per some mentioned examples.) DigestSigner may (in all cases?)

Adding a D parameter to PrehashSigner would do just that: it's primarily useful for RFC6979, and unless you're using a digest to instantiate HMAC-DRBG or some similar KDF, it provides no value, and indeed there would be nothing for a user to pass, so what are they supposed to pass in that case?

And again, there's already a similar trait with a D parameter: DigestSigner.

That's why I suggested that if you really, really want to expose it as a user-configurable parameter, it needs to be on the type, not the trait.

If you want a concrete example, PrehashSigner could be used to support Ed25519ph. Internally Ed25519ph is explicitly mandated to use SHA-512 to perform RFC6979-like deterministic derivation of a once-per-key-message-pair epehemeral scalar. Use anything else but SHA-512, and the construction ceases to be "Ed25519ph".

[cobratbq]

@tarcieri just to check, I propose a D to pass on to crate::generate::secret_number_rfc6979::<D>(&self, prehash). The value prehash is a slice of unknown length and that is fine, i.e. works exactly as expected.

edit if your suggestion is to hard-code SHA-256 for it and leave it at that, then that's fine by me. (IIUC that works for me)

[tarcieri]

@cobratbq aah, that's fine

[cobratbq]

@cobratbq aah, that's fine

I'm glad this is clarified. I had wanted to align the solution direction, but discussions got a bit more complicated. To get back to my original questions/concerns. So,

• regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?
• And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

[tarcieri]

regarding the HMAC-DRBG, would it be better to: (1.) hardcode for DSA? Or (2.) define a generic D that is specific to the use-cases of variable or multiple digests (as discussed above)?

If #520 is resolved, you can use the same approach as ECDSA, leveraging dsa::Signature::Digest.

Alternatively, you could add a generic parameter to SigningKey for the digest to instantiate HMAC-DRBG with for RFC6979, with a reasonable default (e.g. Sha256).

And are there preferences regarding: modifying PrehashSigner vs. introducing an (intermediary) trait such that the generic type can be defined there? (given interoperability, backwards-compatibility, etc.)

If you really feel strongly about this, please open an issue on https://github.com/RustCrypto/traits/issues with your concrete proposal.

If you're suggesting adding an additional trait, please note there's something of a "trait explosion" to handle all possible cases: PrehashSigner has an accompanying RandomizedPrehashSigner that is parameterized by an RNG, DigestSigner also has a RandomizedDigestSigner, and all of these need accompanying traits in async-signature.

Note that there are other things we'd potentially like to add, like domain separation, which would further compound this trait explosion.

So I think the bar would be pretty high to add an additional trait, especially given DigestSigner and PrehashSigner already exist.

[cobratbq]

I get the impression you have some reservations. I understand, especially given another issue is involved.

My goal is to have prehash signing and verification exposed as part of the API. Given the complications as discussed above, what do you recommend?

[tarcieri]

Use Sha256 for now and we can circle back on a more generalized solution in #520

[cobratbq]

@aumetra@tarcieri I made the necessary changes. I think my changes have not impacted the other build failures. Can you let me know if there is anything left to do? Also, consider squashing the merge, because the history contains mostly useless confusing changes.