Repository navigation
Conversation
…ch the specification
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For Ascon-80pq,
AsconEngineloads key bytes 0 to 3 withK0 = Pack.bigEndianToInt(key, 0);(core/src/main/java/.../engines/AsconEngine.java:220), which sign-extends whenkey[0]has its high bit set. At finalisation:194and:195useK1 >> 32andK2 >> 32, carrying the sign bits ofkey[4]andkey[12]into the upper half. The ascon-c v1.2.8 referencecrypto_aead/ascon80pqv12/ref/aead.cuses zero-extendeduint64_tand logical shifts. So for seven keys in eight the tag differs from the specification, and withkey[0]set the ciphertext too.LWC_AEAD_KAT_160_128.txt, read bytestVectorsEngine_ascon80pq, has the one key000102...13, which sets none of those bits.Reproduced on released bcprov-jdk18on-1.86.jar and the 1.87-SNAPSHOT beta (1.87.0.20737), which ship the same
AsconEngine.class;main(c314b9c) has the same source. Of ten vectors, the three with none of the bits set match and the other seven fail: ciphertext and tag for Wycheproof tcId 2, 6 and 45 (key[0]set), the tag only for the rest, e.g. tcId 1 gives tag9c8ca536..., nota781b81a.... Decrypting the expectedct || tagof each of the seven throwsInvalidCipherTextException"Ascon-80pq AEAD mac does not match". Ascon-128, Ascon-128a andAsconAEAD128are unaffected.This change:
:220with& 0xffffffffLand uses>>>at:194and:195, doing the same in the jdk1.4 copy (core/src/main/jdk1.4/.../AsconEngine.java:443,:372,:373). They are no-ops when none of those bits is set; andtestVectorsEngine_ascon80pq_highBitKeystoAsconTest, called fromperformTest(), encrypting and decrypting ten inline vectors: eight from Wycheproofascon80pq_test.jsonand Counts 1 and 537 of the ascon-c KAT, sources and licences in a comment.The new test fails without the change on both jars and passes with it. Other
RegressionTest.testsentries give the same result either way (21 need absent bc-test-data). Given the elevencrypto/asconfiles of bc-test-data, the wholeAsconTestprintsAscon: Okaywith the change and on the beta fails only in the new method.I compiled the jdk1.4 copy only with
--release 8. With the change it passes the seven new rows with a 0, 1 or 16 byte plaintext; tcId 3, 5 and 6 (2, 4 and 5 bytes) fail before and after from a separate defect left for its own change: itsProcessFinalDecryptandProcessFinalEncrypt(:308to:353) handle a final partial word withPack.littleEndianToLong_HighandlongToLittleEndian_High, where base uses big-endian. The jdk1.4RegressionTestdoes not registerAsconTest, so that build never runs it.Ciphertext an earlier release made under an affected key will no longer decrypt; it never did with a conformant implementation.
No jar has a
META-INF/versionscopy ofAsconEngine; nomodule-infoor OSGi change is needed.:core:checkstyleMainis clean. A release-note entry is included, happy to move it to another block.