From 31da29d6869989dd0b049596df77c0c8aef37e8f Mon Sep 17 00:00:00 2001 From: Peter Dettman Date: Thu, 27 Apr 2023 15:07:36 +0700 Subject: Fix Ascon decryption buffering bug - add test coverage for all buffer splits --- crypto/Readme.html | 18 ++++++++ crypto/src/crypto/engines/AsconEngine.cs | 14 ++++--- crypto/test/src/crypto/test/AsconTest.cs | 70 ++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/crypto/Readme.html b/crypto/Readme.html index d71cf63da..a7c903371 100644 --- a/crypto/Readme.html +++ b/crypto/Readme.html @@ -31,6 +31,8 @@
  • Notes:
      +
    1. + Release 2.3.0
    2. Release 2.2.1
    3. @@ -327,6 +329,22 @@

      Notes:

      +

      Release 2.3.0, TBD

      +
      Defects Fixed
      +
        +
      • AsconEngine: Fixed a buffering bug when decrypting across multiple ProcessBytes calls (ascon128a unaffected).
        • +
      +
      Additional Features and Functionality
      +
        +
      +
      Additional Notes
      +
        +
      • + See the (cumulative) list of GitHub pull requests that we have accepted at + bcgit/bc-csharp. +
        • +
      +

      Release 2.2.1, Friday April 21, 2023

      Defects Fixed
        diff --git a/crypto/src/crypto/engines/AsconEngine.cs b/crypto/src/crypto/engines/AsconEngine.cs index 8fd49b03e..0f5ff49ce 100644 --- a/crypto/src/crypto/engines/AsconEngine.cs +++ b/crypto/src/crypto/engines/AsconEngine.cs @@ -345,12 +345,13 @@ namespace Org.BouncyCastle.Crypto.Engines return 0; } - if (m_bufPos >= ASCON_AEAD_RATE) + // NOTE: Need 'while' here because ASCON_AEAD_RATE < CRYPTO_ABYTES in some parameter sets + while (m_bufPos >= ASCON_AEAD_RATE) { - ProcessBufferDecrypt(m_buf, 0, outBytes, outOff); + ProcessBufferDecrypt(m_buf, 0, outBytes, outOff + resultLength); m_bufPos -= ASCON_AEAD_RATE; Array.Copy(m_buf, ASCON_AEAD_RATE, m_buf, 0, m_bufPos); - resultLength = ASCON_AEAD_RATE; + resultLength += ASCON_AEAD_RATE; available += ASCON_AEAD_RATE; if (len < available) @@ -429,12 +430,13 @@ namespace Org.BouncyCastle.Crypto.Engines return 0; } - if (m_bufPos >= ASCON_AEAD_RATE) + // NOTE: Need 'while' here because ASCON_AEAD_RATE < CRYPTO_ABYTES in some parameter sets + while (m_bufPos >= ASCON_AEAD_RATE) { - ProcessBufferDecrypt(m_buf, output); + ProcessBufferDecrypt(m_buf, output[resultLength..]); m_bufPos -= ASCON_AEAD_RATE; m_buf.AsSpan(0, m_bufPos).CopyFrom(m_buf.AsSpan(ASCON_AEAD_RATE)); - resultLength = ASCON_AEAD_RATE; + resultLength += ASCON_AEAD_RATE; available += ASCON_AEAD_RATE; if (input.Length < available) diff --git a/crypto/test/src/crypto/test/AsconTest.cs b/crypto/test/src/crypto/test/AsconTest.cs index eabf7e043..400767df0 100644 --- a/crypto/test/src/crypto/test/AsconTest.cs +++ b/crypto/test/src/crypto/test/AsconTest.cs @@ -94,6 +94,24 @@ namespace Org.BouncyCastle.Crypto.Tests ImplBenchXof(AsconXof.AsconParameters.AsconXofA); } + [Test] + public void TestBufferingEngine_ascon128() + { + ImplTestBufferingEngine(AsconEngine.AsconParameters.ascon128); + } + + [Test] + public void TestBufferingEngine_ascon128a() + { + ImplTestBufferingEngine(AsconEngine.AsconParameters.ascon128a); + } + + [Test] + public void TestBufferingEngine_ascon80() + { + ImplTestBufferingEngine(AsconEngine.AsconParameters.ascon80pq); + } + [Test] public void TestExceptionsDigest_AsconHash() { @@ -327,6 +345,58 @@ namespace Org.BouncyCastle.Crypto.Tests } } + private static void ImplTestBufferingEngine(AsconEngine.AsconParameters asconParameters) + { + Random random = new Random(); + + int plaintextLength = 256; + byte[] plaintext = new byte[plaintextLength]; + random.NextBytes(plaintext); + + var ascon0 = CreateEngine(asconParameters); + InitEngine(ascon0, true); + + byte[] ciphertext = new byte[ascon0.GetOutputSize(plaintextLength)]; + random.NextBytes(ciphertext); + + int ciphertextLength = ascon0.ProcessBytes(plaintext, 0, plaintextLength, ciphertext, 0); + ciphertextLength += ascon0.DoFinal(ciphertext, ciphertextLength); + + byte[] output = new byte[ciphertextLength]; + + // Encryption + for (int split = 1; split < plaintextLength; ++split) + { + var ascon = CreateEngine(asconParameters); + InitEngine(ascon, true); + + random.NextBytes(output); + + int length = ascon.ProcessBytes(plaintext, 0, split, output, 0); + length += ascon.ProcessBytes(plaintext, split, plaintextLength - split, output, length); + length += ascon.DoFinal(output, length); + + Assert.IsTrue(Arrays.AreEqual(ciphertext, 0, ciphertextLength, output, 0, length), + "encryption failed with split: " + split); + } + + // Decryption + for (int split = 1; split < ciphertextLength; ++split) + { + var ascon = CreateEngine(asconParameters); + InitEngine(ascon, false); + + random.NextBytes(output); + + int length = ascon.ProcessBytes(ciphertext, 0, split, output, 0); + length += ascon.ProcessBytes(ciphertext, split, ciphertextLength - split, output, length); + length += ascon.DoFinal(output, length); + + Assert.IsTrue(Arrays.AreEqual(plaintext, 0, plaintextLength, output, 0, length), + "decryption failed with split: " + split); + } + } + private static void ImplTestExceptionsDigest(AsconDigest.AsconParameters asconParameters) { var ascon = new AsconDigest(asconParameters); -- cgit 1.4.1