From b4f65c1abf4c107800325d2767c9f02d4d0557fe Mon Sep 17 00:00:00 2001 From: keks Date: Tue, 4 Aug 2020 12:39:33 +0200 Subject: [PATCH 1/2] Fix HMAC_DRBG edge case 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 "" 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 --- .changelog/3165.bugfix.md | 10 ++++++ go/common/crypto/drbg/hmac_drbg.go | 4 +-- go/common/crypto/drbg/hmac_drbg_test.go | 46 ++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 .changelog/3165.bugfix.md diff --git a/.changelog/3165.bugfix.md b/.changelog/3165.bugfix.md new file mode 100644 index 00000000000..498377c58fe --- /dev/null +++ b/.changelog/3165.bugfix.md @@ -0,0 +1,10 @@ +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. + +Oasis code never uses HMAC_DRNG with a non-nil empty `additional_data` +parameter, so nothing changes for Oasis users. diff --git a/go/common/crypto/drbg/hmac_drbg.go b/go/common/crypto/drbg/hmac_drbg.go index 747708f4c40..cddc8247ae3 100644 --- a/go/common/crypto/drbg/hmac_drbg.go +++ b/go/common/crypto/drbg/hmac_drbg.go @@ -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) } @@ -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 } diff --git a/go/common/crypto/drbg/hmac_drbg_test.go b/go/common/crypto/drbg/hmac_drbg_test.go index 2d9231608ba..0e7408b777f 100644 --- a/go/common/crypto/drbg/hmac_drbg_test.go +++ b/go/common/crypto/drbg/hmac_drbg_test.go @@ -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") From 383608c3c5e35e7389604587d4f3d72cde25b35f Mon Sep 17 00:00:00 2001 From: keks Date: Tue, 11 Aug 2020 23:07:26 +0200 Subject: [PATCH 2/2] .gitlint: Also ignore URLs with hash fragments --- .gitlint-rules/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlint-rules/urls.py b/.gitlint-rules/urls.py index b36270e917b..381a4f25468 100644 --- a/.gitlint-rules/urls.py +++ b/.gitlint-rules/urls.py @@ -7,7 +7,7 @@ gitlint.rule_finder.assert_valid_rule_class = lambda *_: True # URL regular expression. -URL_RE_RAW = r'http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+' +URL_RE_RAW = r'http[s]?://(?:[a-zA-Z]|[0-9]|[#-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+' URL_RE = re.compile(URL_RE_RAW, re.IGNORECASE) LINE_MATCH_RE = re.compile(r'.*' + URL_RE_RAW + r'$')