x.crypto.chacha20: fix internal counter handling

[blackshirt]

This small patch fix internal counter handling after rewrites the internal of x.crypto.chacha20. The counter increments phase not be adjusted to align with the changes and leads to the bug in 25318.
In my test for firebird by @einar-hjortdal, it pass successfully,

---- Testing... ------------------------------------------------------------------------------------------------------
 OK    [1/5] C:   645.2 ms, R:     2.047 ms /workspaces/firebird/protocol_test.v
 OK    [2/5] C:   613.6 ms, R:     2.126 ms /workspaces/firebird/protocol_utils_test.v
 OK    [3/5] C:   606.7 ms, R:    69.396 ms /workspaces/firebird/secure_remote_password_test.v
 OK    [4/5] C:   746.3 ms, R:   603.540 ms /workspaces/firebird/statement_test.v
 OK    [5/5] C:   595.4 ms, R:     2.202 ms /workspaces/firebird/time_test.v

Besides of counter adjusment, this pr also contains some bits of clean up. It also maintain relatives stable performance with previous one, see the benchmark at here

cc @einar-hjortdal please give this pr a try, i hope this fix the bug

[huly-for-github]

Connected to Huly®: V_0.6-25830

[tankf33der]

@blackshirt I easily found a code combination where tests can still fail.

import rand
import x.crypto.chacha20

fn main() {
        mut ctx := chacha20.new_cipher(rand.bytes(32)!, rand.bytes(8)!)!
        // unsafe { ctx.reset() }               // PANIC
        ctx.set_counter(rand.u64())

        mut dst := []u8{len:1}

        ctx.encrypt(mut dst, dst)
        println(dst)
}

[blackshirt]

import rand
import x.crypto.chacha20

fn main() {
mut ctx := chacha20.new_cipher(rand.bytes(32)!, rand.bytes(8)!)!
// unsafe { ctx.reset() } // PANIC
ctx.set_counter(rand.u64())

    mut dst := []u8{len:1}

    ctx.encrypt(mut dst, dst)
    println(dst)

}

i have no panic with this patch. can you give a try ?

import rand
import x.crypto.chacha20

fn main() {
	mut ctx := chacha20.new_cipher(rand.bytes(32)!, rand.bytes(8)!)!
	for i in 0 .. 50 {
		ctx.set_counter(rand.u64())
		mut dst := []u8{len: rand.intn(6)!}
		ctx.encrypt(mut dst, dst)
		println(dst)
	}
}

output:

[112, 89, 106, 10, 103]
[71, 80, 51, 149]
[35, 215, 91, 204, 15]
[168]
[149, 88, 106, 152, 84]
[119]
[84]
[]
[206, 177, 61]
[57, 150, 90]
[151, 236]
[198, 128, 29, 36, 24]
[]
[]
[10, 98]
[21]
[217, 193]
[31, 28, 132, 238]
[202]
[]
[215, 140]
[57, 112, 151, 29]
[240, 91, 11, 134, 248]
[184, 139, 38, 145]
[124]
[]
[160, 212, 144]
[144, 22, 237]
[28, 243]
[108, 90]
[207, 164, 21, 88]
[145, 81, 45, 127, 75]
[]
[30]
[]
[32]
[]
[225, 54]
[95]
[25, 26]
[188, 7]
[61, 233]
[104, 253, 63, 132, 48]
[]
[73, 31]
[]
[182, 179, 19, 181, 206]
[]
[98]
[73, 255, 239]

[tankf33der]

@blackshirt if I uncomment line unsafe { ctx.reset() } i do not expect panic.

[blackshirt]

@blackshirt if I uncomment line unsafe { ctx.reset() } i do not expect panic.

Its should not, in current .reset its also reset mode flag, so, its would not match with your test.. you would need to rekey.
i dont think this .reset exposable to user was a good thing. I fill fix this on next iteration

[blackshirt]

@blackshirt if I uncomment line unsafe { ctx.reset() } i do not expect panic.

i fix it in later patch

[tankf33der]

@blackshirt
You broke x.crypto. :)
Now the encrypt() function can return an Error. I’ll have an additional comment later.

[blackshirt]

@blackshirt You broke x.crypto. :) Now the encrypt() function can return an Error. I’ll have an additional comment later.

i'm try to adjust it

[tankf33der]

@blackshirt

Here's my question-comment I mentioned earlier: I noticed that if I use this pseudocode, everything works fine:

mut ctx := chacha20.new_cipher(key, nonce)!
unsafe { ctx.reset() }
ctx.rekey(key, nonce)!
ctx.set_counter(counter)

but if I use that code, it produces an incorrect result, and my tests against Monocypher fail:

mut ctx := chacha20.new_cipher(key, nonce)!
unsafe { ctx.reset() }
ctx.set_counter(counter)
ctx.rekey(key, nonce)!

This is some kind of super trap for the user that should at least be mentioned in the documentation or fixed.

[blackshirt]

@blackshirt

Here's my question-comment I mentioned earlier: I noticed that if I use this pseudocode, everything works fine:

mut ctx := chacha20.new_cipher(key, nonce)!
unsafe { ctx.reset() }
ctx.rekey(key, nonce)!
ctx.set_counter(counter)

but if I use that code, it produces an incorrect result, and my tests against Monocypher fail:

mut ctx := chacha20.new_cipher(key, nonce)!
unsafe { ctx.reset() }
ctx.set_counter(counter)
ctx.rekey(key, nonce)!

This is some kind of super trap for the user that should at least be mentioned in the documentation or fixed.

its another culprits that should be cleans up, i think we should push this chacha20 as internal package like the go does .
but, unfortunately, i think we dont support this internal package like the go one.

[spytheman]

Thank you @blackshirt and @tankf33der .

[blackshirt]

@spytheman thank you ..i would dig into 64-bit counter issues when i have a free time on it . I think the short current goal to improve this cipher was already reached, minimally on the Benchmark part