Uploaded image for project: 'Commons Crypto'
  1. Commons Crypto
  2. CRYPTO-149

Intermittent Failure in GCMCipherTest#testGcmTamperedData()

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 1.0.0, 1.1.0
    • 1.1.0
    • Cipher
    • None

    Description

      Gary Gregory
      Aug 5, 2020, 10:53 PM (20 hours ago)
      to Commons

      Hi All:

      I am seeing what may be a random AEADBadTagException in GcmCipherTest?

      For example:

      [ERROR] testGcmTamperedData(org.apache.commons.crypto.cipher.GcmCipherTest)
      Time elapsed: 0.015 s <<< ERROR!
      881java.lang.Exception: Unexpected exception,
      expected<javax.crypto.AEADBadTagException> but
      was<java.lang.InternalError>
      882 at org.apache.commons.crypto.cipher.GcmCipherTest.testGcmTamperedData(GcmCipherTest.java:224)
      883
      884

      Any thoughts?

      The above is from
      https://travis-ci.org/github/apache/commons-crypto/jobs/715348986

      Gary

      Alex Remily
      8:10 AM (11 hours ago)
      to Commons

      That is an intermittent issue that I haven't been able to reliably reproduce. As I recall, the test that's failing is supposed to fail, but in a different way. I think it's supposed to fail because of a short buffer but occasionally fails because of an internal error, and when that happens this test fails. I don't know when it was introduced. We should probably document it in jira and or realese notes.

      Matt Sicker
      10:33 AM (8 hours ago)
      to Commons

      Now I hope we don't have unit tests depending on non-static state for
      its random number generator! I'd expect a crypto library's test
      suites to include several hard-coded known-good and known-bad
      ciphertexts with static keys/IVs similar to the test cases presented
      in their RFCs (especially since said tests are typically small enough
      to copy/paste the binary data fairly easily).

      Matt Sicker

      Rob Tompkins
      10:37 AM (8 hours ago)
      to Commons

      We actually do have a considerable number of those in our projects where we use probabilistic epsilons on the output. See commons-rng. Note, Gilles is quite good at writing such tests.

      -Rob

      Matt Sicker
      10:42 AM (8 hours ago)
      to Commons

      Well, for testing RNGs, I can understand using property testing, yes.
      It would also be useful for testing fuzzing scenarios like making sure
      the GCM tag is invalid for any random input data (giving a near zero
      probability of valid data) or that an elliptic curve implementation
      doesn't leak out information about points outside the curve or respond
      to invalid inputs improperly or things like that.

      Rob Tompkins
      10:50 AM (8 hours ago)
      to Commons

      +1 - the elliptic curve stuff I’ll have to defer to you on as I’m less a number theorist and more of a logician.

      -Rob

      Gary Gregory
      10:56 AM (8 hours ago)
      to Commons

      This is all fine and good but how would you fix the test such that it does
      not fail randomly. PR anyone?

      Gary

      Rob Tompkins
      10:57 AM (8 hours ago)
      to Commons

      Either static inputs for determinism, or putting a probabilistic boundary in which the solution can fall.

      -Rob

      Matt Sicker
      11:00 AM (8 hours ago)
      to Commons

      Choose a seed value for the `new Random()` constructor and the tests
      will be deterministic.

      Matt Sicker
      11:02 AM (8 hours ago)
      to Commons

      Or alternatively, if using random values each time, have it retry the
      test with a different value. It's typically better to use an actual
      property testing library for these types of tests anyways. One example
      library I found is https://jqwik.net/ (these types of testing
      libraries are more common in functional programming like in Scala).

      Rob Tompkins
      11:03 AM (8 hours ago)
      to Commons

      Precisely. That’s another technique we’ve used in rng.

      -Ropb

      Attachments

        Activity

          People

            Unassigned Unassigned
            aremily Alex Remily
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 0.5h
                0.5h