FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix Blake2bpDigest.update buffer limit for input split across calls by Arpan0995 · Pull Request #2488 · bcgit/bc-java · GitHub

Fix Blake2bpDigest.update buffer limit for input split across calls - #2488

Open
Arpan0995 wants to merge 1 commit into
bcgit:mainfrom
Arpan0995:blake2bp-update-buffer-limit
Open

Arpan0995 wants to merge 1 commit into
bcgit:mainfrom
Arpan0995:blake2bp-update-buffer-limit

Conversation

Copy link
Copy Markdown
Contributor

Blake2bpDigest buffers one 128-byte block per leaf: PARALLELISM_DEGREE is 4 (Blake2bpDigest.java:32) and buffer = new byte[512] (:39). update(byte[], int, int) measures the room left as 8*BLAKE2B_BLOCKBYTES - left (:66), 1024 less what is buffered. Once input is buffered, a call that does not fit goes wrong:

  • at 1024 - left bytes or more, the flush copy at :70 ends past the buffer and throws ArrayIndexOutOfBoundsException;
  • under 512 bytes, the flush is skipped and the tail copy at :101 throws the same way;
  • in between, the leaf loop at :82-94 hashes the first 512 bytes of the new input ahead of the buffered bytes, a wrong digest with no exception.

update(byte) calls update(byte[], int, int) (:59), so a byte at a time fails on the 513th byte. One-shot hashing and messages of 512 bytes or fewer are unaffected.

Blake2spDigest has the same expression (Blake2spDigest.java:67) over eight 64-byte blocks (:31, :39), so it is consistent; there the buffer was short (#1363, fixed in 1.73). Here the limit is wrong: the BLAKE2 reference fills sizeof( S->buf ) - left of a 4 * BLAKE2B_BLOCKBYTES buffer (ref/blake2bp-ref.c:140, ref/blake2.h:84), and a 1024-byte buffer would still give wrong digests.

Reproduced on the released bcprov-jdk18on-1.86.jar with the new test's 1100-byte message (bytes 00 01 .. ff repeating): of the chunk sizes 1 to 1100, 946 threw ArrayIndexOutOfBoundsException (arraycopy: last destination index 513 out of bounds for byte[512] at size 1) and 76 (sizes 513 to 588) gave a wrong digest. Two calls of 10 and 600 bytes return the digest of M[10..522) || M[0..10) || M[522..610) rather than of M, and an org.bouncycastle.crypto.io.DigestOutputStream given write(buf, 0, 16) then write(buf, 16, 4096) throws with last destination index 1024. Current main and the 1.87-SNAPSHOT beta (1.87.0.20730) have the same Blake2bpDigest.java, and the beta behaves the same.

This change:

  • measures the room left as PARALLELISM_DEGREE * BLAKE2B_BLOCKBYTES - left, the 512 bytes the buffer holds, as the leaf loop at :88 does;
  • adds testMultiPartUpdate to Blake2bpDigestTest: an 1100-byte message hashed in one call against a fixed value, then in chunks of every size from 1 to 1100 and at every two-call split, unkeyed and keyed, and through update(byte), unkeyed; and
  • registers Blake2bpDigestTest in crypto.test.AllTests. No suite referenced it and the Gradle test task runs only AllTest* classes (build.gradle:250), so it never ran. Blake2spDigestTest is in the same position (it passes) and is left out here.

The new test fails without the change (ArrayIndexOutOfBoundsException on the 513th one-byte update) and passes with it; the class's other tests and the other digest tests in crypto.test give the same results either way.

Base tree only: no META-INF/versions copy, module-info or OSGi change. A release-note entry is included, happy to move it to another block.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant


Back | FazBrowse Home | New Git URL