Skip to content

Commit

Permalink
Fix HMAC_DRBG edge case
Browse files Browse the repository at this point in the history
The NIST SP 800-90A[1] spec of HMAC_DRBG uses "!= Null" checks,
but "Null" is never defined in the document. In the current
implementation, this was interpreteted as nil.
However, the NIST examples document[2] uses "<empty>" values,
so []byte{} should be passed in the test cases. If that happens,
the nil checks do not work as expected. Therefore, instead of checking
equality with nil, it should be checked whether the length of
additional_info is zero.

As additional evidence I cite the openssl implementation[3], which checks
for zero length instead of NULL pointer equality.

[1]: https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-90Ar1.pdf
[2]: https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/HMAC_DRBG.pdf
[3]: https://github.com/openssl/openssl/pull/6779/files#diff-c44c9681c4fc49b1d7e9e74578085212R77-R79
  • Loading branch information
keks committed Aug 4, 2020
1 parent 3a0b2f3 commit 383efd5
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 3 deletions.
9 changes: 9 additions & 0 deletions .changelog/3165.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
go/common/crypto/drbg: Also consider empty slices as Null values

The wrong handling of an edge case in the HMAC_DRBG implementation has been
corrected. An update with empty `additional_data` now behaves the same as an
update with nil additional data. While the spec is not 100% clear around how
this is to be treated, supplemental documentation suggests that this is the
correct way to handle it.

TODO: Did Oasis ever do this call? Does this break something?
4 changes: 2 additions & 2 deletions go/common/crypto/drbg/hmac_drbg.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (r *Drbg) generate(requestedNumberOfBytes int, additionalInput []byte) ([]b

// 2. If additional_input != Null, then (Key, V) = HMAC_DRBG_Update
// (additional_input, Key, V).
if additionalInput != nil {
if len(additionalInput) != 0 {
r.update(additionalInput)
}

Expand Down Expand Up @@ -130,7 +130,7 @@ func (r *Drbg) update(providedData []byte) {
v := updateV(r.hash, k, r.v)

// 3. If (provided_data = Null), then return K and V.
if providedData == nil {
if len(providedData) == 0 {
r.v, r.k = v, k
return
}
Expand Down
46 changes: 45 additions & 1 deletion go/common/crypto/drbg/hmac_drbg_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,51 @@ func toHexUpper(b []byte) string {
return strings.ToUpper(hex.EncodeToString(b))
}

func TestDrbgNISTExample1(t *testing.T) {
func TestDrbgNISTExampleZeroLength(t *testing.T) {
const readSz = 1024 / 8

entropyInput, _ := hex.DecodeString("000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E")
nonce, _ := hex.DecodeString("202122232425262728292A2B2C2D2E2F")

// HMAC_DRBG_Instantiate_algorithm
drbg, err := New(crypto.SHA512, entropyInput, nonce, nil)
require.NoError(t, err, "HMAC_DRBG_Instantiate_algorithm (ctor)")
require.Equal(
t,
"A7E118A531DEF956DCFF94BB3D801F775DC68F91696A434CC25E270E639044E1A7240266D3AA202D46C1054B610247535007DF12CFC8DA45982A587FC81C47D5",
toHexUpper(drbg.k),
"HMAC_DRBG_Instantiate_algorithm (K)",
)
require.Equal(
t,
"110793EAA60DC9DBCD4208104088A23DAECC1226EAF1D03BBA9D83A69599916571907346B15A0439362B9C8EE330E52DEACC639B98E8030A95780CD7C24B04D5",
toHexUpper(drbg.v),
"HMAC_DRBG_Instantiate_algorithm (V)",
)

// HMAC_DRBG_Generate
v, err := drbg.generate(readSz, []byte{})
require.NoError(t, err, "HMAC_DRBG_Generate")
require.Equal(
t,
"A463395AA79F237A22E5BD24462BD303E1BE5103BA37299BED170E10713EE9CDA62FABD5171231E1F6D82629BC521D41178D002D92918F397824E449004E9AE1851F7BFA11CD616EF519A9E2A05951D9108AB38959CA7E9E80B18ADFCC622389495795CBFB7D39AF6C8571DDCE035CA6890C7A1AF80861F0629EF1B6952BA206",
toHexUpper(v),
"HMAC_DRBG_Generate",
)

// HMAC_DRBG_Generate, io.Reader edition.
n, err := drbg.Read(v)
require.NoError(t, err, "HMAC_DRBG_Generate (io.Reader)")
require.Equal(t, readSz, n, "HMAC_DRBG_Generate (io.Reader")
require.Equal(
t,
"FB5BD98D2CB25EC4955CD15204D68C497281CA0CE2201DACA5E412DDFDEBAF98D724D21662E45ABA9AE200D941C4CF76039808F29A8000346A6CC97D44417737A89F90472AC6088B45C666C561686F191745228F11ED556A519DA9AA1646D15B901382D87726D17DC5139FDEE1E8BDB0F328D4B105865BD1D815641E6B1DBA23",
toHexUpper(v),
"HMAC_DRBG_Generate (io.Reader)",
)
}

func TestDrbgNISTExampleNil(t *testing.T) {
const readSz = 1024 / 8

entropyInput, _ := hex.DecodeString("000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F202122232425262728292A2B2C2D2E2F303132333435363738393A3B3C3D3E3F404142434445464748494A4B4C4D4E4F505152535455565758595A5B5C5D5E5F606162636465666768696A6B6C6D6E")
Expand Down

0 comments on commit 383efd5

Please sign in to comment.