Add rfc6979 k generation. by werwurm · Pull Request #80 · aurora-opensource/libnat20

werwurm · 2026-02-26T01:22:22Z

This implementation of rfc6979 nonce generation is based on the nat20 crypto context interface and is therefore independent of the bssl reference implementation.

Move common C headers into nat20/types.h so that necessary target platform customizations can be made in one place. Also add nat20/endian.h and nat20/limits.h and replace the posix type ssize_t with the C type ptrdiff_t.

github-actions · 2026-02-26T01:23:58Z

LCOV of commit `597bec8` during lcov-test-coverage-report #62

Summary coverage rate:
  lines......: 95.2% (2526 of 2654 lines)
  functions..: 99.0% (200 of 202 functions)
  branches...: 85.8% (1421 of 1656 branches)

Files changed coverage rate: n/a

Copilot

Pull request overview

This PR adds RFC 6979 deterministic nonce (k) generation for ECDSA signatures to the nat20 crypto library. The implementation is built on the nat20 crypto context interface, making it independent of the BoringSSL reference implementation. RFC 6979 provides a deterministic method to generate the nonce k from the private key and message, eliminating the security risks associated with weak random number generators in ECDSA signing.

Changes:

Adds new n20_rfc6979_k_generation function implementing RFC 6979 deterministic k generation for P-256 and P-384 curves
Implements big number utility functions for arithmetic operations (shift, subtract, compare, convert)
Adds comprehensive test coverage with RFC 6979 test vectors from Appendix A2.5 (P-256) and A2.6 (P-384), plus additional edge cases

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File	Description
include/nat20/crypto/nat20/rfc6979.h	Public API header defining the RFC 6979 k generation function and big number types with comprehensive documentation
src/crypto/nat20/rfc6979.c	Core implementation of RFC 6979 algorithm including HMAC-DRBG-based k generation and big number utility functions
src/crypto/test/crypto_nat20.cpp	Test suite with RFC 6979 standard test vectors for P-256 and P-384 curves, testing both first and second nonce candidates
CMakeLists.txt	Build configuration updates to include the new source and header files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/crypto/nat20/rfc6979.c

src/crypto/test/crypto_nat20.cpp

src/crypto/nat20/rfc6979.c

src/crypto/test/crypto_nat20.cpp

src/crypto/nat20/rfc6979.c

src/crypto/test/crypto_nat20.cpp

src/crypto/nat20/rfc6979.c

seidelrj · 2026-02-26T16:39:27Z

src/crypto/nat20/rfc6979.c

+ * are wider than @p res, a return value of true may also indicate
+ * data loss.
+ */
+bool n20_bn_sub_overflow(n20_bn_t const* a, n20_bn_t const* b, n20_bn_t* res) {


i think MISRA will ding you for using carry as both a bool and uint32_t.

seidelrj · 2026-02-26T16:57:25Z

src/crypto/nat20/rfc6979.c

+#define N20_SHA2_224_OCTETS 28
+#define N20_SHA2_256_OCTETS 32
+#define N20_SHA2_384_OCTETS 48
+#define N20_SHA2_512_OCTETS 64
+
+static size_t digest_enum_to_size(n20_crypto_digest_algorithm_t alg_in) {
+    switch (alg_in) {
+        case n20_crypto_digest_algorithm_sha2_224_e:
+            return N20_SHA2_224_OCTETS;
+        case n20_crypto_digest_algorithm_sha2_256_e:
+            return N20_SHA2_256_OCTETS;
+        case n20_crypto_digest_algorithm_sha2_384_e:
+            return N20_SHA2_384_OCTETS;
+        case n20_crypto_digest_algorithm_sha2_512_e:
+            return N20_SHA2_512_OCTETS;
+        default:
+            return 0;
+    }
+}


does it make since to add these to the crypto nat20 sha implementation in case we want to reuse it in the future?

We can always move this around. Because this implementation can be used with other implementations of SHA2, I'd rather keep it it self contained here. I don't have strong opinions about this but I also don't see a natural place where this should go.

src/crypto/nat20/rfc6979.c

seidelrj · 2026-02-26T17:11:45Z

src/crypto/nat20/rfc6979.c

+    if (m_octets != NULL) {
+        err = ctx->digest(ctx, digest_algorithm, m_octets, 1, h1_bytes, &h1_size);
+        if (err != n20_error_ok_e) {
+            return err;
+        }
+
+        h1_slice.size = h1_size < qlen_octets ? h1_size : qlen_octets;
+        n20_slice_to_bn(k, &h1_slice);
+
+        if (n20_bn_cmp(k, q) >= 0) {
+            if (n20_bn_sub_overflow(k, q, k)) {
+                return n20_error_crypto_implementation_specific_e;
+            }
+        }
+        h1_size = qlen_octets;
+        n20_bn_to_octets(h1_bytes, h1_size, k);
+        h1_slice.size = h1_size;
+    }


should probably add similar comments to what we added to the boringssl implementation.

src/crypto/nat20/rfc6979.c

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T19:24:28Z

src/crypto/nat20/rfc6979.c

+
+    return n20_error_crypto_implementation_specific_e;


This return statement is unreachable because the while loop on line 301 is infinite and only exits via return statements within the loop body. The function will never reach this line during normal execution. Consider removing this unreachable code to improve code clarity.

Suggested change

return n20_error_crypto_implementation_specific_e;

Copilot · 2026-02-27T19:24:29Z

src/crypto/nat20/rfc6979.c

+bool n20_bn_sub_overflow(n20_bn_t const* a, n20_bn_t const* b, n20_bn_t* res) {
+    uint32_t carry = 0;
+    size_t i = 0;
+    size_t max_words = a->word_count > b->word_count ? a->word_count : b->word_count;
+    max_words = max_words > res->word_count ? max_words : res->word_count;
+
+    for (; i < max_words; ++i) {
+        uint32_t res_ = 0;
+        if (i < a->word_count) {
+            res_ = a->words[i];
+        }
+        carry = __builtin_sub_overflow(res_, carry, &res_) ? 1 : 0;
+        if (i < b->word_count) {
+            carry |= __builtin_sub_overflow(res_, b->words[i], &res_) ? 1 : 0;
+        }
+
+        if (i < res->word_count) {
+            res->words[i] = res_;
+        } else if (res_ != 0) {
+            /* Result does not fit into output bn. */
+            return true;
+        }
+    }
+
+    return carry;
+}
+
+void n20_slice_to_bn(n20_bn_t* bn, n20_slice_t const* slice) {
+    uint32_t octet = slice->size;
+    for (size_t i = 0; i < bn->word_count; ++i) {
+        bn->words[i] = 0;
+        for (int j = 0; j < 4; ++j) {
+            bn->words[i] >>= 8;
+            if (octet) {
+                octet -= 1;
+                bn->words[i] |= ((uint32_t)slice->buffer[octet]) << 24;
+            }
+        }
+    }
+}
+
+void n20_bn_to_octets(uint8_t* octets, size_t octets_len, n20_bn_t const* bn) {
+    for (size_t i = 0; i < octets_len; ++i) {
+        size_t word_index = i >> 2;
+        size_t byte_index = i & 0x3;
+        if (word_index < bn->word_count) {
+            octets[octets_len - i - 1] =
+                (uint8_t)((bn->words[word_index] >> (8 * byte_index)) & 0xff);
+        } else {
+            octets[octets_len - i - 1] = 0;
+        }
+    }
+}
+
+bool n20_bn_is_zero(n20_bn_t* bn) {
+    for (size_t i = 0; i < bn->word_count; ++i) {
+        if (bn->words[i] != 0) return false;
+    }
+    return true;
+}
+
+int n20_bn_cmp(n20_bn_t* a, n20_bn_t* b) {
+    size_t max_words = a->word_count > b->word_count ? a->word_count : b->word_count;
+    int result = 0;
+    for (size_t i_ = max_words; i_ > 0; --i_) {
+        size_t i = i_ - 1;
+        uint32_t aw = i < a->word_count ? a->words[i] : 0;
+        uint32_t bw = i < b->word_count ? b->words[i] : 0;
+        if (result == 0 && aw < bw) result = -1;
+        if (result == 0 && aw > bw) result = 1;
+    }
+    return result;
+}


These helper functions are not declared in the public header file and should be marked as static to indicate they are internal implementation details. This follows the established convention in the codebase (see src/crypto/nat20/crypto.c, sha256.c, sha512.c) where internal helper functions are consistently marked static. Making these functions static improves encapsulation and prevents unintended external linkage.

werwurm added 16 commits February 12, 2026 14:42

cmake package

850caa1

Add license header and tidy

a29ea11

Add cmake.in files to license check.

a5378fb

fixup cmake packages alias and install include paths

a785dc1

tidy cmake install targets

bd872b0

Make C++ language discovery optional

2616735

Replace hard coded lib with CMAKE_INSTALL_LIBDIR variable

80d3ed8

Add target aliases.

b612d0a

Do not export test utils.

2573aa0

Add boringssl conditionaly to package config

9202b42

fixups

08397c7

Add version file and stop exporting nat20_crypto_boringssl

ac05c83

Do not install test utils library

0f2420f

Add PATH_VARS to configure_package_config_file stanza

7a0306f

Refactor C headers into nat20/types.h

b02b186

Move common C headers into nat20/types.h so that necessary target platform customizations can be made in one place. Also add nat20/endian.h and nat20/limits.h and replace the posix type ssize_t with the C type ptrdiff_t.

Add rfc6979 k generation independent of bssl.

8be841d

werwurm requested review from Copilot and seidelrj February 26, 2026 01:22

Copilot started reviewing on behalf of werwurm February 26, 2026 01:22 View session

Copilot AI reviewed Feb 26, 2026

View reviewed changes

seidelrj requested changes Feb 26, 2026

View reviewed changes

werwurm added 2 commits February 26, 2026 16:46

Address comments

8193ce7

fix formating

c55f02e

werwurm requested a review from seidelrj February 27, 2026 00:50

werwurm changed the base branch from werwurm/types to main February 27, 2026 17:56

Merge branch 'main' into werwurm/rfc6979

597bec8

werwurm requested a review from Copilot February 27, 2026 19:20

Copilot started reviewing on behalf of werwurm February 27, 2026 19:20 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

Conversation

werwurm commented Feb 26, 2026

Uh oh!

github-actions bot commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit 597bec8 during lcov-test-coverage-report #62

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

seidelrj Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

werwurm Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

seidelrj Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

werwurm Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

seidelrj Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 26, 2026 •

edited

Loading

LCOV of commit `597bec8` during lcov-test-coverage-report #62