Merge pull request #3165 from keks/fix-hmac_drbg

fix HMAC_DRBG edge case
oasisprotocol · Aug 11, 2020 · 481bbd9 · 481bbd9
2 parents 5083907 + 383608c
commit 481bbd9
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 4 deletions.
diff --git a/.changelog/3165.bugfix.md 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/.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'$')
 

diff --git 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
@@ -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")