Support for ECDSA deterministic signing (RFC 6979) #10369

facutuesca · 2024-02-08T19:02:55Z

Part of #9795. Now that the Rust bindings needed are in place (sfackler/rust-openssl#2144), this PR adds support for ECDSA deterministic signing (RFC 6979), a new feature in OpenSSL >= 3.2.0.

The way to enable it is by passing a new boolean parameter deterministic_signing to ec.ECDSA():

signature = private_key.sign(
    data,
    ec.ECDSA(hashes.SHA256(), deterministic_signing=True)
)

When using anything other than OpenSSL >= 3.2.0, this will raise an UnsupportedAlgorithm exception. It will also raise if FIPS is enabled.
The new deterministic_signing parameter defaults to False, so that old code that does not specify it maintains the same behavior.

alex · 2024-02-19T21:44:24Z

rust-openssl has had a release.

Also can you please make a seperate PR adding the test vectors?

facutuesca · 2024-02-20T06:52:09Z

Also can you please make a seperate PR adding the test vectors?

@alex : #10438

facutuesca · 2024-02-20T15:00:36Z

I rebased now that the test vectors are merged

alex · 2024-02-21T12:45:30Z

src/rust/src/backend/ec.rs

@@ -274,14 +274,35 @@ impl ECPrivateKey {
            ));
        }

-        let (data, _) = utils::calculate_digest_and_algorithm(
+        let (data, _algo) = utils::calculate_digest_and_algorithm(


if this is used, the name shouldn't have a leading prefix

it's only used when OpenSSL >=3.2.0, so without it it complains when compiling with older OpenSSLs (or libre/boring). I could do something like this instead?:

cfg_if::cfg_if! { if #[cfg(CRYPTOGRAPHY_OPENSSL_320_OR_GREATER)]{ let (data, _algo) = utils::calculate_digest_and_algorithm( py, data.as_bytes(), signature_algorithm.getattr(pyo3::intern!(py, "algorithm"))?, )?; } else { let (data, _) = utils::calculate_digest_and_algorithm( py, data.as_bytes(), signature_algorithm.getattr(pyo3::intern!(py, "algorithm"))?, )?; }

You can do let _ = algo; in the other branch.

oh, that is a lot better! fixed

alex · 2024-02-21T12:45:44Z

src/rust/src/backend/ec.rs

+            .extract()?;
+        cfg_if::cfg_if! {
+            if #[cfg(CRYPTOGRAPHY_OPENSSL_320_OR_GREATER)]{
+                match deterministic {


This can be an if, no need for a match

alex · 2024-02-21T12:46:15Z

src/rust/src/backend/ec.rs

+                        let hash_function_name = _algo.getattr(pyo3::intern!(py, "name"))?.extract::<&str>()?;
+                        let hash_function = openssl::md::Md::fetch(None, hash_function_name, None)?;


There's a function in the hashes module that you can use for this.

I tried, but I'm having trouble constructing the appropriate type. The hashes function I tried to use returns a openssl::hash::MessageDigest:

pub(crate) fn message_digest_from_algorithm( py: pyo3::Python<'_>, algorithm: &pyo3::PyAny, ) -> CryptographyResult<openssl::hash::MessageDigest> {

The function I'm calling that uses the hash function takes an openssl::md::MdRef:

pub fn set_signature_md(&self, md: &MdRef) -> Result<(), ErrorStack>

I can convert one to the other by adding unsafe code:

let hash_function = hashes::message_digest_from_algorithm(py, _algo)?; unsafe { signer.set_signature_md(&openssl::md::Md::from_ptr(hash_function.as_ptr().cast_mut()))?; }

But not sure if that's what we want (as opposed to directly using openssl::md::Md::fetch(), which already encapsulates the unsafe use)

alex · 2024-02-21T12:47:20Z

tests/utils.py

+        elif not line:
+            if data:
+                vectors.append(data)
+                data = dict()


Suggested change

data = dict()

data = {}

alex · 2024-02-21T12:47:36Z

docs/hazmat/primitives/asymmetric/ec.rst

@@ -47,6 +47,16 @@ Elliptic Curve Signature Algorithms
    :param algorithm: An instance of
        :class:`~cryptography.hazmat.primitives.hashes.HashAlgorithm`.

+    :param bool deterministic_signing: A boolean flag defaulting to ``False``
+        that specifies whether the signing procedure should be deterministic
+        or not, as defined in :rfc:`6979`.


Docs should say that this has no impact on verification.

Fixed. The new docstring now says:

:param bool deterministic_signing: A boolean flag defaulting to ``False`` that specifies whether the signing procedure should be deterministic or not, as defined in :rfc:`6979`. This only impacts the signing process, verification is not affected (the verification process is the same for both deterministic and non-deterministic signed messages).

Tarnum-tst · 2024-02-25T11:57:56Z

Merge, please!

alex · 2024-02-26T00:33:34Z

CHANGELOG.rst

@@ -26,6 +26,7 @@ Changelog
  and :class:`~cryptography.hazmat.primitives.ciphers.algorithms.ARC4` into
  :doc:`/hazmat/decrepit/index` and deprecated them in the ``cipher`` module.
  They will be removed from the ``cipher`` module in 48.0.0.
+* Added support for deterministic ECDSA (:rfc:`6979`)


Please add a link to the relevant code -- making ECDSA a link to the class should be enough.

alex · 2024-02-26T00:34:32Z

src/rust/src/backend/ec.rs

@@ -274,14 +274,35 @@ impl ECPrivateKey {
            ));
        }

-        let (data, _) = utils::calculate_digest_and_algorithm(
+        let (data, _algo) = utils::calculate_digest_and_algorithm(


You can do let _ = algo; in the other branch.

alex · 2024-02-26T00:35:22Z

tests/hazmat/primitives/test_ec.py

@@ -508,6 +513,70 @@ def test_signature_failures(self, backend, subtests):
                        signature, vector["message"], ec.ECDSA(hash_type())
                    )

+    def test_unsupported_deterministic_nonce(self, backend, subtests):


Suggested change

def test_unsupported_deterministic_nonce(self, backend, subtests):

def test_unsupported_deterministic_nonce(self, backend):

alex · 2024-02-26T00:35:55Z

tests/hazmat/primitives/test_ec.py

+            key = bytes("\n".join(vector["key"]), "utf-8")
+            if "digest_sign" in vector:
+                algorithm = vector["digest_sign"]
+                assert algorithm in supported_hash_algorithms


Suggested change

assert algorithm in supported_hash_algorithms

Not needed, in tests it's fine for the next line to raise KeyError

alex · 2024-02-26T00:36:24Z

tests/hazmat/primitives/test_ec.py

+                f" backend {backend}"
+            )
+
+        supported_hash_algorithms: typing.Dict[


It's simpler if you just make the values here instance of the hash algorithm, rather than instantiating them below.

facutuesca · 2024-02-26T14:37:07Z

Rebased now that #10486 is merged

Tarnum-tst · 2024-02-26T14:41:26Z

@facutuesca
openssl-sys is 0.9.101 in cryptography-openssl currently.

reaperhulk · 2024-02-26T16:52:18Z

src/rust/src/backend/ec.rs

+                }
+            } else {
+                let _ = algo;
+                assert!(!deterministic);


So this assert won't trigger unless you do something dumb (instantiate ECDSA then set obj._deterministic = True yourself on non-OpenSSL 3.2.0) but I'd rather it wasn't possible to cause program termination even if you do something truly stupid.

Makes sense, I removed the assert and didn't add any code (therefore completely ignoring deterministic's value) as per @alex's suggestion.

… >= 3.2.0

tests/hazmat/primitives/test_ec.py

facutuesca force-pushed the deterministic-nonce branch 7 times, most recently from 441b799 to c4f3bd9 Compare February 8, 2024 20:47

facutuesca marked this pull request as ready for review February 8, 2024 21:09

alex added this to the Forty Third Release milestone Feb 10, 2024

facutuesca force-pushed the deterministic-nonce branch from c4f3bd9 to 01388ce Compare February 20, 2024 06:45

facutuesca force-pushed the deterministic-nonce branch from 01388ce to bd354d1 Compare February 20, 2024 15:00

alex reviewed Feb 21, 2024

View reviewed changes

facutuesca force-pushed the deterministic-nonce branch from bd354d1 to b8dfa6c Compare February 21, 2024 16:52

facutuesca requested a review from alex February 21, 2024 16:59

alex reviewed Feb 26, 2024

View reviewed changes

facutuesca force-pushed the deterministic-nonce branch from 6801f81 to 0342369 Compare February 26, 2024 09:29

facutuesca added 4 commits February 26, 2024 15:36

Update openssl-sys to 0.9.100 in cryptography-openssl

ec4c85b

Add support for deterministic ECDSA (RFC 6979)

4582219

Address review comments

915168d

Address review comments

dd28add

facutuesca force-pushed the deterministic-nonce branch from 0342369 to dd28add Compare February 26, 2024 14:36

Update openssl-sys to 0.9.101 in cryptography-openssl

fcf08f8

reaperhulk reviewed Feb 26, 2024

View reviewed changes

Return error rather than panic

0b9fd57

facutuesca force-pushed the deterministic-nonce branch from 0685c47 to 087f948 Compare February 26, 2024 17:22

Remove incorrect code and ignore deterministic ECDSA when OpenSSL not…

dd6d63d

… >= 3.2.0

facutuesca force-pushed the deterministic-nonce branch from 087f948 to dd6d63d Compare February 26, 2024 17:24

Ignore deterministic field with incompatible OpenSSL versions

ca593a9

facutuesca force-pushed the deterministic-nonce branch from 090da44 to ca593a9 Compare February 26, 2024 17:29

reaperhulk reviewed Feb 26, 2024

View reviewed changes

tests/hazmat/primitives/test_ec.py Show resolved Hide resolved

facutuesca added 2 commits February 26, 2024 19:31

Use the subtests fixture correctly

45b10e2

Actually use the subtests fixture correctly

94c92df

reaperhulk approved these changes Feb 26, 2024

View reviewed changes

reaperhulk enabled auto-merge (squash) February 26, 2024 19:00

reaperhulk merged commit 0a1098f into pyca:main Feb 26, 2024
56 checks passed

facutuesca deleted the deterministic-nonce branch February 26, 2024 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for ECDSA deterministic signing (RFC 6979) #10369

Support for ECDSA deterministic signing (RFC 6979) #10369

facutuesca commented Feb 8, 2024 •

edited

Loading

alex commented Feb 19, 2024

facutuesca commented Feb 20, 2024

facutuesca commented Feb 20, 2024 •

edited

Loading

alex Feb 21, 2024

facutuesca Feb 21, 2024

alex Feb 26, 2024

facutuesca Feb 26, 2024

alex Feb 21, 2024

facutuesca Feb 21, 2024

alex Feb 21, 2024

facutuesca Feb 21, 2024

alex Feb 21, 2024

facutuesca Feb 21, 2024

alex Feb 21, 2024

facutuesca Feb 21, 2024

Tarnum-tst commented Feb 25, 2024

alex Feb 26, 2024

facutuesca Feb 26, 2024

alex Feb 26, 2024

alex Feb 26, 2024

facutuesca Feb 26, 2024

alex Feb 26, 2024

facutuesca Feb 26, 2024

alex Feb 26, 2024

facutuesca Feb 26, 2024

facutuesca commented Feb 26, 2024

Tarnum-tst commented Feb 26, 2024

reaperhulk Feb 26, 2024 •

edited

Loading

facutuesca Feb 26, 2024

		let hash_function_name = _algo.getattr(pyo3::intern!(py, "name"))?.extract::<&str>()?;
		let hash_function = openssl::md::Md::fetch(None, hash_function_name, None)?;

	def test_unsupported_deterministic_nonce(self, backend, subtests):
	def test_unsupported_deterministic_nonce(self, backend):

Support for ECDSA deterministic signing (RFC 6979) #10369

Support for ECDSA deterministic signing (RFC 6979) #10369

Conversation

facutuesca commented Feb 8, 2024 • edited Loading

alex commented Feb 19, 2024

facutuesca commented Feb 20, 2024

facutuesca commented Feb 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tarnum-tst commented Feb 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facutuesca commented Feb 26, 2024

Tarnum-tst commented Feb 26, 2024

reaperhulk Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facutuesca commented Feb 8, 2024 •

edited

Loading

facutuesca commented Feb 20, 2024 •

edited

Loading

reaperhulk Feb 26, 2024 •

edited

Loading