Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Security fix #290

Merged
merged 2 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ exports.header = function (uri, method, options) {
artifacts.hash = Crypto.calculatePayloadHash(options.payload, credentials.algorithm, options.contentType);
}

const mac = Crypto.calculateMac('header', credentials, artifacts);
const mac = Crypto.generateRequestMac('header', credentials, artifacts);

// Construct header

Expand Down Expand Up @@ -187,7 +187,7 @@ exports.authenticate = function (res, credentials, artifacts, options) {
artifacts.ext = serverAuthAttributes.ext;
artifacts.hash = serverAuthAttributes.hash;

const mac = Crypto.calculateMac('response', credentials, artifacts);
const mac = Crypto.generateRequestMac('response', credentials, artifacts);
if (mac !== serverAuthAttributes.mac) {
throw new Boom.Boom('Bad response mac', { decorate: result });
}
Expand Down Expand Up @@ -276,7 +276,7 @@ exports.getBewit = function (uri, options) {
// Calculate signature

const exp = Math.floor(now / 1000) + options.ttlSec;
const mac = Crypto.calculateMac('bewit', credentials, {
const mac = Crypto.generateRequestMac('bewit', credentials, {
ts: exp,
nonce: '',
method: 'GET',
Expand Down Expand Up @@ -367,11 +367,8 @@ exports.message = function (host, port, message, options) {
ts: artifacts.ts,
nonce: artifacts.nonce,
hash: artifacts.hash,
mac: Crypto.calculateMac('message', credentials, artifacts)
mac: Crypto.generateRequestMac('message', credentials, artifacts)
};

return result;
};



53 changes: 51 additions & 2 deletions lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports.headerVersion = '1'; // Prevent comparison of mac
exports.algorithms = ['sha1', 'sha256'];


// Calculate the request MAC
// Generates the request MAC

/*
type: 'header', // 'header', 'bewit', 'response'
Expand All @@ -41,7 +41,49 @@ exports.algorithms = ['sha1', 'sha256'];
}
*/

exports.calculateMac = function (type, credentials, options) {
exports.generateRequestMac = function (type, credentials, options) {

const normalized = exports.generateNormalizedString(type, options);

const hmac = Crypto.createHmac(credentials.algorithm, credentials.key).update(normalized);
const digest = hmac.digest('base64');
return digest;
};


// Calculate the request MAC for verification

/*
type: 'header', // 'header', 'bewit', 'response'
credentials: {
key: 'aoijedoaijsdlaksjdl',
algorithm: 'sha256' // 'sha1', 'sha256'
},
options: {
method: 'GET',
resource: '/resource?a=1&b=2',
host: 'example.com',
port: 8080,
ts: 1357718381034,
nonce: 'd3d345f',
hash: 'U4MKKSmiVxk37JCCrAVIjV/OhB3y+NdwoCr6RShbVkE=',
ext: 'app-specific-data',
app: 'hf48hd83qwkj', // Application id (Oz)
dlg: 'd8djwekds9cj' // Delegated by application id (Oz), requires options.app
}
*/

exports.calculateServerMac = function (type, credentials, options, payload, contentType) {

if (options.hash) {
if (!payload) {
console.warn(`Security Warning: calculateServerMac was called trusting the client payload hash which provides no integrity checking and is insecure`);
}
else {
// never trust client provided hash, always calculate server side
options.hash = exports.calculatePayloadHash(payload, credentials.algorithm, contentType);
}
}

const normalized = exports.generateNormalizedString(type, options);

Expand All @@ -51,6 +93,13 @@ exports.calculateMac = function (type, credentials, options) {
};


exports.calculateMac = function (type, credentials, options) {

console.warn(`Deprecation Warning: calculateMac() is replaced by either calculateServerMac() or generateRequestMac()`);
return exports.generateRequestMac(type, credentials, options);
};


exports.generateNormalizedString = function (type, options) {

let resource = options.resource || '';
Expand Down
20 changes: 10 additions & 10 deletions lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,6 @@ exports.authenticate = async function (req, credentialsFunc, options) {
throw new Boom.Boom('Unknown algorithm', { decorate: result });
}

// Calculate MAC

const mac = Crypto.calculateMac('header', credentials, artifacts);
if (!Cryptiles.fixedTimeComparison(mac, attributes.mac)) {
throw Object.assign(Utils.unauthorized('Bad mac'), result);
}

// Check payload hash

if (options.payload ||
Expand All @@ -175,6 +168,13 @@ exports.authenticate = async function (req, credentialsFunc, options) {
}
}

// Calculate MAC

const mac = Crypto.calculateServerMac('header', credentials, artifacts, options.payload, request.contentType);
if (!Cryptiles.fixedTimeComparison(mac, attributes.mac)) {
throw Object.assign(Utils.unauthorized('Bad mac'), result);
}

// Check nonce

if (options.nonceFunc) {
Expand Down Expand Up @@ -288,7 +288,7 @@ exports.header = function (credentials, artifacts, options) {
artifacts.hash = Crypto.calculatePayloadHash(options.payload, credentials.algorithm, options.contentType);
}

const mac = Crypto.calculateMac('response', credentials, artifacts);
const mac = Crypto.calculateServerMac('response', credentials, artifacts, options.payload, options.contentType);

// Construct header

Expand Down Expand Up @@ -429,7 +429,7 @@ exports.authenticateBewit = async function (req, credentialsFunc, options) {

// Calculate MAC

const mac = Crypto.calculateMac('bewit', credentials, {
const mac = Crypto.generateRequestMac('bewit', credentials, {
ts: bewit.exp,
nonce: '',
method: 'GET',
Expand Down Expand Up @@ -508,7 +508,7 @@ exports.authenticateMessage = async function (host, port, message, authorization

// Calculate MAC

const mac = Crypto.calculateMac('message', credentials, artifacts);
const mac = Crypto.generateRequestMac('message', credentials, artifacts);
if (!Cryptiles.fixedTimeComparison(mac, authorization.mac)) {
throw Object.assign(Utils.unauthorized('Bad mac'), result);
}
Expand Down
36 changes: 36 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

return {
id,
key: 'werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn',

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn" is used as
key
.
algorithm: (id === '1' ? 'sha1' : 'sha256'),
user: 'steve'
};
Expand Down Expand Up @@ -307,4 +307,40 @@
const err = await expect(Hawk.server.authenticate(req, credentialsFunc)).to.reject();
expect(err.credentials).to.exist();
});

it('generates a header then fails to parse it (payload tampering)', async () => {

const req = {
method: 'POST',
url: '/resource/4?filter=a',
headers: {
host: 'example.com:8080',
'content-type': 'text/plain;x=y'
}
};

const payload = 'some not so random text';

const credentials1 = credentialsFunc('123456');

const reqHeader = Hawk.client.header('http://example.com:8080/resource/4?filter=a', req.method, { credentials: credentials1, ext: 'some-app-data', payload, contentType: req.headers['content-type'] });
req.headers.authorization = reqHeader.header;

const { credentials: credentials2, artifacts } = await Hawk.server.authenticate(req, credentialsFunc);
expect(credentials2.user).to.equal('steve');
expect(artifacts.ext).to.equal('some-app-data');
expect(() => Hawk.server.authenticatePayload('tampered text', credentials2, artifacts, req.headers['content-type'])).to.throw('Bad payload hash');

const res = {
headers: {
'content-type': 'text/plain'
}
};

res.headers['server-authorization'] = Hawk.server.header(credentials2, artifacts, { payload: 'some reply', contentType: 'text/plain', ext: 'response-specific' });
expect(res.headers['server-authorization']).to.exist();

expect(() => Hawk.client.authenticate(res, credentials2, artifacts, { payload: 'some reply' })).to.not.throw();
});

});
46 changes: 45 additions & 1 deletion test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

return {
id,
key: 'werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn',

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn" is used as
key
.
The hard-coded value "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn" is used as
key
.
algorithm: (id === '1' ? 'sha1' : 'sha256'),
user: 'steve'
};
Expand Down Expand Up @@ -1122,6 +1122,51 @@
creds.algorithm = 'blah';
expect(() => Hawk.client.message('example.com', 8080, 'some message', { credentials: creds })).to.throw('Unknown algorithm');
});

it('coverage for calculateMac arguments to calculatePayloadHash', async () => {

const credentials = credentialsFunc('123456');
const payload = 'some not so random text';
const req = {
method: 'POST',
url: '/resource/4?filter=a',
host: 'example.com',
port: 8080,
headers: {
host: 'example.com:8080',
'content-type': 'text/plain'
}
};

const exp = Math.floor(Hawk.utils.now() / 1000) + 60;
const ext = 'some-app-data';

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "some-app-data" is used as
authorization header
.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit test directory, the code scanner needs tuning

const nonce = '1AwuJD';

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "1AwuJD" is used as
authorization header
.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unit test directory, the code scanner needs tuning

const hash = Hawk.crypto.calculatePayloadHash(payload, 'sha256', req.headers['content-type']);
const opts = {
ts: exp,
nonce,
method: req.method,
resource: req.url,
host: req.host,
port: req.port,
hash,
ext
};
const mac = Hawk.crypto.calculateServerMac('header', credentials, opts, payload, req.headers['content-type']);
const header = 'Hawk id="' + credentials.id +
'", ts="' + exp +
'", nonce="' + nonce +
'", hash="' + hash +
'", ext="' + ext +
'", mac="' + mac + '"';

req.headers.authorization = header;
// missing contentType
Hawk.crypto.calculateServerMac('header', credentials, opts, payload);
Hawk.crypto.calculateMac('header', credentials, opts);
await expect(Hawk.server.authenticate(req, credentialsFunc)).to.not.reject();
});

});

describe('authenticatePayloadHash()', () => {
Expand All @@ -1133,4 +1178,3 @@
});
});
});

3 changes: 1 addition & 2 deletions test/uri.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

return {
id,
key: 'werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn',

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "werxhqb98rpaxn39848xrunpaw3489ruxnpa98w4rxn" is used as
key
.
algorithm: (id === '1' ? 'sha1' : 'sha256'),
user: 'steve'
};
Expand Down Expand Up @@ -149,7 +149,7 @@

const exp = Math.floor(Hawk.utils.now() / 1000) + 60;
const ext = 'some-app-data';
const mac = Hawk.crypto.calculateMac('bewit', credentials, {
const mac = Hawk.crypto.generateRequestMac('bewit', credentials, {
ts: exp,
nonce: '',
method: req.method,
Expand Down Expand Up @@ -395,7 +395,7 @@

const bewit = Hawk.uri.getBewit('https://example.com:8080/somewhere/over/the/rainbow', { credentials, ttlSec: 300, localtimeOffsetMsec: 1356420407232 - Hawk.utils.now(), ext: 'xandyandz' });
expect(bewit).to.equal('MTIzNDU2XDEzNTY0MjA3MDdcaFpiSjNQMmNLRW80a3kwQzhqa1pBa1J5Q1p1ZWc0V1NOYnhWN3ZxM3hIVT1ceGFuZHlhbmR6');
});

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.

it('returns a valid bewit value (null ext)', () => {

Expand All @@ -407,7 +407,7 @@

const bewit = Hawk.uri.getBewit('https://example.com/somewhere/over/the/rainbow', { credentials, ttlSec: 300, localtimeOffsetMsec: 1356420407232 - Hawk.utils.now(), ext: null });
expect(bewit).to.equal('MTIzNDU2XDEzNTY0MjA3MDdcSUdZbUxnSXFMckNlOEN4dktQczRKbFdJQStValdKSm91d2dBUmlWaENBZz1c');
});

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.

it('returns a valid bewit value (parsed uri)', () => {

Expand All @@ -419,7 +419,7 @@

const bewit = Hawk.uri.getBewit(Url.parse('https://example.com/somewhere/over/the/rainbow'), { credentials, ttlSec: 300, localtimeOffsetMsec: 1356420407232 - Hawk.utils.now(), ext: 'xandyandz' });
expect(bewit).to.equal('MTIzNDU2XDEzNTY0MjA3MDdca3NjeHdOUjJ0SnBQMVQxekRMTlBiQjVVaUtJVTl0T1NKWFRVZEc3WDloOD1ceGFuZHlhbmR6');
});

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.

it('errors on invalid options', () => {

Expand All @@ -431,7 +431,7 @@
expect(() => Hawk.uri.getBewit('https://example.com/somewhere/over/the/rainbow', {})).to.throw('Invalid inputs');
});

it('errors on missing uri', () => {

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.

const credentials = {
id: '123456',
Expand All @@ -453,7 +453,7 @@
expect(() => Hawk.uri.getBewit(5, { credentials, ttlSec: 300, localtimeOffsetMsec: 1356420407232 - Hawk.utils.now(), ext: 'xandyandz' })).to.throw('Invalid inputs');
});

it('errors on invalid credentials (id)', () => {

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.

const credentials = {
key: '2983d45yun89q',
Expand All @@ -464,7 +464,7 @@
});

it('errors on invalid credentials (id)', () => {

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.
const credentials = {
key: '2983d45yun89q',
algorithm: 'sha256'
Expand All @@ -474,7 +474,7 @@
});

it('errors on invalid credentials (algorithm)', () => {

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.
const credentials = {
key: '2983d45yun89q',
id: '123'
Expand All @@ -484,7 +484,7 @@
});

it('errors on missing credentials', () => {

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.
expect(() => Hawk.uri.getBewit('https://example.com/somewhere/over/the/rainbow', { ttlSec: 3000, ext: 'xandyandz' })).to.throw('Invalid credentials');
});

Expand All @@ -494,7 +494,7 @@
id: '123456',
algorithm: 'sha256'
};

Check failure

Code scanning / CodeQL

Hard-coded credentials Critical test

The hard-coded value "2983d45yun89q" is used as
key
.
expect(() => Hawk.uri.getBewit('https://example.com/somewhere/over/the/rainbow', { credentials, ttlSec: 3000, ext: 'xandyandz' })).to.throw('Invalid credentials');
});

Expand All @@ -515,4 +515,3 @@
});
});
});

Loading