Skip to content

Commit 8595398

Browse files
linuxwolflinuxwolf
committed
Fix: HMAC minimum length checks should be enforced
1 parent d1b8d88 commit 8595398

4 files changed

Lines changed: 87 additions & 46 deletions

File tree

‎lib/algorithms/aes-cbc-hmac-sha2.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ function cbcHmacEncryptFN(size) {
232232

233233
var promise;
234234
promise = HMAC["HS" + (size * 2)].sign(iKey, mdata, {
235-
loose: true
235+
length: size
236236
});
237237
promise = promise.then(function(result) {
238238
// TODO: move slice to hmac.js
@@ -279,7 +279,7 @@ function cbcHmacDecryptFN(size) {
279279
helpers.int64ToBuffer(adata.length * 8)
280280
]);
281281
promise = HMAC["HS" + (size * 2)].verify(iKey, mdata, tag, {
282-
loose: true
282+
length: size
283283
});
284284
promise = promise.then(function() {
285285
return cdata;
@@ -402,7 +402,7 @@ function concatKdfCbcHmacEncryptFN(size, alg) {
402402
]);
403403
return Promise.all([
404404
Promise.resolve(cdata),
405-
HMAC["HS" + (size * 2)].sign(cik, mdata, { loose: false })
405+
HMAC["HS" + (size * 2)].sign(cik, mdata, { length: size })
406406
]);
407407
});
408408
promise = promise.then(function(result){

‎lib/algorithms/hkdf.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function hkdfDeriveFn(name) {
4747
T = new Buffer(0);
4848
}
4949
T = Buffer.concat([T, info, new Buffer([idx])]);
50-
T = op(key, T, { loose: true });
50+
T = op(key, T);
5151
T = T.then(function(result) {
5252
T = result.mac;
5353
okm.push(T);
@@ -58,7 +58,7 @@ function hkdfDeriveFn(name) {
5858
}
5959

6060
// Step 1: Extract
61-
promise = op(salt, key, { loose: true });
61+
promise = op(salt, key, { length: salt.length * 8 });
6262
promise = promise.then(function(result) {
6363
// Step 2: Expand
6464
return expand(result.mac);

‎lib/algorithms/hmac.js‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -14,30 +14,37 @@ function hmacSignFN(name) {
1414
var md = name.replace("HS", "SHA").toLowerCase(),
1515
hash = name.replace("HS", "SHA-");
1616

17-
// ### Fallback Implementation -- uses forge
18-
var fallback = function(key, pdata, props) {
19-
props = props || {};
20-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
17+
function checkKeyLength(len, key) {
18+
len = (len || CONSTANTS.HASHLENGTH[hash]) / 8;
19+
if (len > key.length) {
2120
return Promise.reject(new Error("invalid key length"));
2221
}
2322

24-
var sig = forge.hmac.create();
25-
sig.start(md, key.toString("binary"));
26-
sig.update(pdata);
27-
sig = sig.digest().native();
23+
return Promise.resolve(key);
24+
}
2825

29-
return Promise.resolve({
30-
data: pdata,
31-
mac: sig
26+
// ### Fallback Implementation -- uses forge
27+
var fallback = function(key, pdata, props) {
28+
props = props || {};
29+
var promise;
30+
promise = checkKeyLength(props.length, key);
31+
promise = p.then(function() {
32+
var sig = forge.hmac.create();
33+
sig.start(md, key.toString("binary"));
34+
sig.update(pdata);
35+
sig = sig.digest().native();
36+
37+
return {
38+
data: pdata,
39+
mac: sig
40+
}
3241
});
42+
return p;
3343
};
3444

3545
// ### WebCryptoAPI Implementation
3646
var webcrypto = function(key, pdata, props) {
3747
props = props || {};
38-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
39-
return Promise.reject(new Error("invalid key length"));
40-
}
4148

4249
var alg = {
4350
name: "HMAC",
@@ -46,7 +53,10 @@ function hmacSignFN(name) {
4653
}
4754
};
4855
var promise;
49-
promise = helpers.subtleCrypto.importKey("raw", key, alg, true, ["sign"]);
56+
promise = checkKeyLength(props.length, key);
57+
promise = promise.then(function() {
58+
return helpers.subtleCrypto.importKey("raw", key, alg, true, ["sign"]);
59+
});
5060
promise = promise.then(function(key) {
5161
return helpers.subtleCrypto.sign(alg, key, pdata);
5262
});
@@ -64,18 +74,20 @@ function hmacSignFN(name) {
6474
// ### NodeJS implementation
6575
var nodejs = function(key, pdata, props) {
6676
props = props || {};
67-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
68-
return Promise.reject(new Error("invalid key length"));
69-
}
7077

71-
var hmac = helpers.nodeCrypto.createHmac(md, key);
72-
hmac.update(pdata);
78+
var promise;
79+
promise = checkKeyLength(props.length, key);
80+
promise = promise.then(function() {
81+
var hmac = helpers.nodeCrypto.createHmac(md, key);
82+
hmac.update(pdata);
7383

74-
var sig = hmac.digest();
75-
return {
76-
data: pdata,
77-
mac: sig
78-
};
84+
var sig = hmac.digest();
85+
return {
86+
data: pdata,
87+
mac: sig
88+
};
89+
});
90+
return promise;
7991
};
8092

8193
return helpers.setupFallback(nodejs, webcrypto, fallback);
@@ -85,9 +97,9 @@ function hmacVerifyFN(name) {
8597
var md = name.replace("HS", "SHA").toLowerCase(),
8698
hash = name.replace("HS", "SHA-");
8799

88-
function compare(loose, expected, actual) {
89-
var len = loose ? expected.length : CONSTANTS.HASHLENGTH[hash] / 8,
90-
valid = true;
100+
function compare(len, expected, actual) {
101+
len = (len || CONSTANTS.HASHLENGTH[hash]) / 8;
102+
var valid = true;
91103
for (var idx = 0; len > idx; idx++) {
92104
valid = valid && (expected[idx] === actual[idx]);
93105
}
@@ -97,16 +109,13 @@ function hmacVerifyFN(name) {
97109
// ### Fallback Implementation -- uses forge
98110
var fallback = function(key, pdata, mac, props) {
99111
props = props || {};
100-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
101-
return Promise.reject(new Error("invalid key length"));
102-
}
103112

104113
var vrfy = forge.hmac.create();
105114
vrfy.start(md, new DataBuffer(key));
106115
vrfy.update(pdata);
107116
vrfy = vrfy.digest().native();
108117

109-
if (compare(props.loose, mac, vrfy)) {
118+
if (compare(props.length, mac, vrfy)) {
110119
return Promise.resolve({
111120
data: pdata,
112121
mac: mac,
@@ -119,9 +128,6 @@ function hmacVerifyFN(name) {
119128

120129
var webcrypto = function(key, pdata, mac, props) {
121130
props = props || {};
122-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
123-
return Promise.reject(new Error("invalid key length"));
124-
}
125131

126132
var alg = {
127133
name: "HMAC",
@@ -130,14 +136,14 @@ function hmacVerifyFN(name) {
130136
}
131137
};
132138
var promise;
133-
if (props.loose) {
139+
if (props.length) {
134140
promise = helpers.subtleCrypto.importKey("raw", key, alg, true, ["sign"]);
135141
promise = promise.then(function(key) {
136142
return helpers.subtleCrypto.sign(alg, key, pdata);
137143
});
138144
promise = promise.then(function(result) {
139145
var sig = new Buffer(result);
140-
return compare(true, mac, sig);
146+
return compare(props.length, mac, sig);
141147
});
142148
} else {
143149
promise = helpers.subtleCrypto.importKey("raw", key, alg, true, ["verify"]);
@@ -162,15 +168,12 @@ function hmacVerifyFN(name) {
162168

163169
var nodejs = function(key, pdata, mac, props) {
164170
props = props || {};
165-
if (!props.loose && CONSTANTS.HASHLENGTH[hash] > (key.length << 3)) {
166-
return Promise.reject(new Error("invalid key length"));
167-
}
168171

169172
var hmac = helpers.nodeCrypto.createHmac(md, key);
170173
hmac.update(pdata);
171174

172175
var sig = hmac.digest();
173-
if (!compare(props.loose, mac, sig)) {
176+
if (!compare(props.length, mac, sig)) {
174177
throw new Error("verification failed");
175178
}
176179
return {

‎test/algorithms/aes-cbc-hmac-sha2-test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,43 @@ describe("algorithms/aes-cbc-hmac-sha2", function() {
6969

7070
it("performs " + v.alg + " (" + v.desc + ") encryption", encrunner);
7171
it("performs " + v.alg + " (" + v.desc + ") decryption", decrunner);
72+
73+
it('should not pass verification (truncated tag)', function() {
74+
var tamperedTag = v.tag.substring(0, 6);
75+
var key = new Buffer(v.key, "hex"),
76+
pdata = new Buffer(v.plaintext, "hex"),
77+
cdata = new Buffer(v.ciphertext, "hex"),
78+
props = {
79+
iv: new Buffer(v.iv, "hex"),
80+
aad: new Buffer(v.aad, "hex"),
81+
tag: new Buffer(tamperedTag, "hex")
82+
};
83+
var promise = algorithms.decrypt(v.alg, key, cdata, props);
84+
promise = promise.then(function(result) {
85+
assert(false, "unexpected success");
86+
});
87+
promise = promise.catch(function(err) {
88+
assert(err);
89+
});
90+
})
91+
it('should not pass verification, (empty tag)', function() {
92+
var tamperedTag = "";
93+
var key = new Buffer(v.key, "hex"),
94+
pdata = new Buffer(v.plaintext, "hex"),
95+
cdata = new Buffer(v.ciphertext, "hex"),
96+
props = {
97+
iv: new Buffer(v.iv, "hex"),
98+
aad: new Buffer(v.aad, "hex"),
99+
tag: new Buffer(tamperedTag, "hex")
100+
};
101+
var promise = algorithms.decrypt(v.alg, key, cdata, props);
102+
promise = promise.then(function(result) {
103+
assert(false, "unexpected success");
104+
});
105+
promise = promise.catch(function(err) {
106+
assert(err);
107+
});
108+
return promise;
109+
});
72110
});
73111
});

0 commit comments

Comments
 (0)