StableLib / stablelib

A stable library of useful TypeScript/JavaScript code
https://www.stablelib.com
Other
173 stars 35 forks source link

`clean`/`reset` is incorrectly? implemented in blake2b and inconsistent with the sha512 implementation #31

Closed otonashixav closed 3 years ago

otonashixav commented 3 years ago

I was attempting to modify ed25519 to use blake2b instead of sha512, but wasn't able to get the sign function to produce a correct signature without modifying how the blake2b object was used.

It seems that calling reset after clean on a blake2b object does not reinitialize the state, hence it cannot be reused after calling clean. The expected behaviour should be that calling reset allows the object to be reused with the same initial state?

Comparison of BLAKE2b and SHA512:

BLAKE2b:

Welcome to Node.js v14.16.0.
Type ".help" for more information.
> const BLAKE2b = require('@stablelib/blake2b').BLAKE2b
undefined
> const hs = new BLAKE2b()
undefined
> hs
BLAKE2b {
  ...,
  _state: Int32Array(16) [
     -222443192,  1779033703,
    -2067093701, -1150833019,
      -23791573,  1013904242,
     1595750129, -1521486534,
    -1377402159,  1359893119,
      725511199, -1694144372,
      -79577749,   528734635,
      327033209,  1541459225
  ],
  ...,
  _initialState: Uint32Array(16) [
    4072524104, 1779033703,
    2227873595, 3144134277,
    4271175723, 1013904242,
    1595750129, 2773480762,
    2917565137, 1359893119,
     725511199, 2600822924,
    4215389547,  528734635,
     327033209, 1541459225
  ]
}
> hs.clean()
undefined
> hs
BLAKE2b {
  ...,
  _state: Int32Array(16) [
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0
  ],
 ...,
  _initialState: Uint32Array(16) [
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0
  ]
}
> hs.reset()
BLAKE2b {
  ...,
  _state: Int32Array(16) [
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0
  ],
  ...,
  _initialState: Uint32Array(16) [
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0, 0, 0,
    0, 0, 0, 0
  ]
}

SHA512:

Welcome to Node.js v14.16.0.
Type ".help" for more information.
> const SHA512 = require('@stablelib/sha512').SHA512
undefined
> const hs = new SHA512()
undefined
> hs
SHA512 {
  ...,
  _stateHi: Int32Array(8) [
    1779033703,
    -1150833019,
    1013904242,
    -1521486534,
    1359893119,
    -1694144372,
    528734635,
    1541459225
  ],
  _stateLo: Int32Array(8) [
     -205731576, -2067093701,
      -23791573,  1595750129,
    -1377402159,   725511199,
      -79577749,   327033209
  ],
  ...
}
> hs.clean()
undefined
> hs
SHA512 {
  ...,
  _stateHi: Int32Array(8) [
    1779033703,
    -1150833019,
    1013904242,
    -1521486534,
    1359893119,
    -1694144372,
    528734635,
    1541459225
  ],
  _stateLo: Int32Array(8) [
     -205731576, -2067093701,
      -23791573,  1595750129,
    -1377402159,   725511199,
      -79577749,   327033209
  ],
  ...
}
> hs.reset()
SHA512 {
  ...,
  _stateHi: Int32Array(8) [
    1779033703,
    -1150833019,
    1013904242,
    -1521486534,
    1359893119,
    -1694144372,
    528734635,
    1541459225
  ],
  _stateLo: Int32Array(8) [
     -205731576, -2067093701,
      -23791573,  1595750129,
    -1377402159,   725511199,
      -79577749,   327033209
  ],
  ...
}
dchest commented 3 years ago

Reset can't be used after clean, indeed — where it can, it initializes the state only by accident, not by design. BLAKE2b works like this because its initial state depends on user configuration and key or salt, so that state is stored and when you call reset it is restored, allowing you to run hash with the initial parameters that you set. Clean throws everything away, including the configuration parameters, the key, and the salt. SHA on the other hand, doesn't have configurable features, so it just puts the IV constants back.

I should document that clean is intended as a final wipe for safety so that fewer secret bytes remain in memory. After you call clean, you shouldn't do anything else with the object.