Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add strict object name validation #1265

Merged
merged 3 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.

### [6.19.0](https://github.com/ali-sdk/ali-oss/compare/v6.18.1...v6.19.0) (2023-12-14)

### Features

* add strict mode for verifying object names when signing URLs and enabled by default ([#1265](https://github.com/ali-sdk/ali-oss/issues/1265)) ([ff03bbb](https://github.com/ali-sdk/ali-oss/pull/1265/commits/ff03bbb62b041dd34d30ca2503745ba5d7c0a216))

### [6.18.1](https://github.com/ali-sdk/ali-oss/compare/v6.18.0...v6.18.1) (2023-09-08)

### Features
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ options:
- [region] {String} the bucket data region location, please see [Data Regions](#data-regions),
default is `oss-cn-hangzhou`.
- [internal] {Boolean} access OSS with aliyun internal network or not, default is `false`.
If your servers are running on aliyun too, you can set `true` to save lot of money.
If your servers are running on aliyun too, you can set `true` to save a lot of money.
- [secure] {Boolean} instruct OSS client to use HTTPS (secure: true) or HTTP (secure: false) protocol.
- [timeout] {String|Number} instance level timeout for all operations, default is `60s`.
- [cname] {Boolean}, default false, access oss with custom domain name. if true, you can fill `endpoint` field with your custom domain name,
Expand All @@ -355,6 +355,7 @@ options:
- [proxy] {String | Object}, proxy agent uri or options, default is null.
- [retryMax] {Number}, used by auto retry send request count when request error is net error or timeout. **_NOTE:_** Not support `put` with stream, `putStream`, `append` with stream because the stream can only be consumed once
- [maxSockets] {Number} Maximum number of sockets to allow per host. Default is infinity
- [verifyObjectStrict] {Boolean} Strictly verify object names when signing URLs, default is true.
YunZZY marked this conversation as resolved.
Show resolved Hide resolved

example:

Expand Down
10 changes: 10 additions & 0 deletions lib/browser/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,13 @@ proto.requestError = async function requestError(result) {
this.debug('generate error %j', err, 'error');
return err;
};

/**
* set the flag of verifying object name strictly
* @param {boolean} enable
* @api private
*/
proto.setVerifyObjectNameStrictEnabled = function setVerifyObjectNameStrictEnabled(enable) {
this.options.verifyObjectStrict = !!enable;
return this;
};
10 changes: 10 additions & 0 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -392,3 +392,13 @@ proto.setSLDEnabled = function setSLDEnabled(enable) {
this.options.sldEnable = !!enable;
return this;
};

/**
* set the flag of verifying object name strictly
* @param {boolean} enable
* @api private
*/
proto.setVerifyObjectNameStrictEnabled = function setVerifyObjectNameStrictEnabled(enable) {
this.options.verifyObjectStrict = !!enable;
return this;
};
3 changes: 2 additions & 1 deletion lib/common/client/initOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ module.exports = function (options) {
headerEncoding: 'utf-8',
refreshSTSToken: null,
refreshSTSTokenInterval: 60000 * 5,
retryMax: 0
retryMax: 0,
verifyObjectStrict: true
},
options
);
Expand Down
5 changes: 5 additions & 0 deletions lib/common/object/asyncSignatureUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ proto.asyncSignatureUrl = async function asyncSignatureUrl(name, options) {
if (isIP(this.options.endpoint.hostname)) {
throw new Error('can not get the object URL when endpoint is IP');
}

if (this.options.verifyObjectStrict && /^\?/.test(name)) {
throw new Error(`Invalid object name ${name}`);
}

options = options || {};
name = this._objectName(name);
options.method = options.method || 'GET';
Expand Down
5 changes: 5 additions & 0 deletions lib/common/object/signatureUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ proto.signatureUrl = function signatureUrl(name, options) {
if (isIP(this.options.endpoint.hostname)) {
throw new Error('can not get the object URL when endpoint is IP');
}

if (this.options.verifyObjectStrict && /^\?/.test(name)) {
throw new Error(`Invalid object name ${name}`);
}

options = options || {};
name = this._objectName(name);
options.method = options.method || 'GET';
Expand Down
75 changes: 74 additions & 1 deletion test/browser/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,7 @@ describe('browser', () => {
let store;
let name;
let needEscapeName;
const testSignatureObjectName = `?{测}\r\n[试];,/?:@&=+$<中>-_.!~*'(文)"¥#%!(字)^ \`符|\\${prefix}test.txt`;
before(async () => {
store = oss(ossConfig);
name = `${prefix}ali-sdk/oss/signatureUrl.js`;
Expand All @@ -1006,6 +1007,9 @@ describe('browser', () => {
});
assert.equal(object.res.status, 200);
// assert.equal(typeof object.res.headers['x-oss-request-id'], 'string');

const testSignatureObject = await store.put(testSignatureObjectName, Buffer.from('Hello World!', 'utf8'));
assert.equal(typeof testSignatureObject.res.headers['x-oss-request-id'], 'string');
});

it('should signature url get object ok', async () => {
Expand Down Expand Up @@ -1057,7 +1061,7 @@ describe('browser', () => {
assert.equal(urlRes.data.toString(), result.content.toString());
});

it('should signature url with reponse limitation', async () => {
it('should signature url with response limitation', async () => {
const response = {
'content-type': 'xml',
'content-language': 'zh-cn'
Expand Down Expand Up @@ -1104,6 +1108,75 @@ describe('browser', () => {
await store.asyncSignatureUrl('test.txt');
assert(flag);
});

it('should verify object name strictly by default', () => {
assert.throws(() => {
try {
store.signatureUrl(testSignatureObjectName);
} catch (err) {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
throw err;
}
}, Error);

store
.asyncSignatureUrl(testSignatureObjectName)
.then(() => {
assert.fail('Expected asyncSignatureUrl to throw an error');
})
.catch(err => {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
});
});

it('should verify object name loosely', async () => {
store = oss(
Object.assign({}, ossConfig, {
verifyObjectStrict: false
})
);

const testSignatureObjectFromGet = await store.get(testSignatureObjectName);
const testSignatureObjectUrl = store.signatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
assert.equal(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());

const testSignatureObjectUrlAsync = await store.asyncSignatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
assert.equal(testSignatureObjectFromUrlAsync.data.toString(), testSignatureObjectFromGet.content.toString());
});

it('should verify object name via manual configuration', async () => {
store = oss(ossConfig);
store.setVerifyObjectNameStrictEnabled(false);

const testSignatureObjectFromGet = await store.get(testSignatureObjectName);
const testSignatureObjectUrl = store.signatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
assert.equal(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());

const testSignatureObjectUrlAsync = await store.asyncSignatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
assert.equal(testSignatureObjectFromUrlAsync.data.toString(), testSignatureObjectFromGet.content.toString());

store.setVerifyObjectNameStrictEnabled(true);

assert.throws(() => {
try {
store.signatureUrl(testSignatureObjectName);
} catch (err) {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
throw err;
}
}, Error);

try {
await store.asyncSignatureUrl(testSignatureObjectName);
assert.fail('Expected asyncSignatureUrl to throw an error');
} catch (err) {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
}
});
});

describe('multipart', () => {
Expand Down
68 changes: 68 additions & 0 deletions test/node/object.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,7 @@ describe('test/object.test.js', () => {
describe('signatureUrl()', () => {
let name;
let needEscapeName;
const testSignatureObjectName = `?{测}\r\n[试];,/?:@&=+$<中>-_.!~*'(文)"¥#%!(字)^ \`符|\\${prefix}test.txt`;
before(async () => {
name = `${prefix}ali-sdk/oss/signatureUrl.js`;
let object = await store.put(name, __filename, {
Expand All @@ -1045,6 +1046,9 @@ describe('test/object.test.js', () => {
}
});
assert.equal(typeof object.res.headers['x-oss-request-id'], 'string');

const testSignatureObject = await store.put(testSignatureObjectName, Buffer.from('Hello World!', 'utf8'));
assert.equal(testSignatureObject.res.status, 200);
});

it('should signature url get object ok', async () => {
Expand Down Expand Up @@ -1224,6 +1228,70 @@ describe('test/object.test.js', () => {
assert(error.message === 'can not get the object URL when endpoint is IP', error.message);
}
});

it('should verify object name strictly by default', () => {
assert.throws(() => {
try {
store.signatureUrl(testSignatureObjectName);
} catch (err) {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
throw err;
}
}, Error);

assert.rejects(store.asyncSignatureUrl(testSignatureObjectName), err => {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);

return true;
});
});

it('should verify object name loosely', async () => {
const conf = {};
copy(config).to(conf);
conf.bucket = bucket;
conf.verifyObjectStrict = false;
const verifyObjectNameOss = oss(conf);

const testSignatureObjectFromGet = await verifyObjectNameOss.get(testSignatureObjectName);
const testSignatureObjectUrl = verifyObjectNameOss.signatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
assert.equal(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());

const testSignatureObjectUrlAsync = await verifyObjectNameOss.asyncSignatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
assert.equal(testSignatureObjectFromUrlAsync.data.toString(), testSignatureObjectFromGet.content.toString());
});

it('should verify object name via manual configuration', async () => {
store.setVerifyObjectNameStrictEnabled(false);

const testSignatureObjectFromGet = await store.get(testSignatureObjectName);
const testSignatureObjectUrl = store.signatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrl = await urllib.request(testSignatureObjectUrl);
assert.equal(testSignatureObjectFromUrl.data.toString(), testSignatureObjectFromGet.content.toString());

const testSignatureObjectUrlAsync = await store.asyncSignatureUrl(testSignatureObjectName);
const testSignatureObjectFromUrlAsync = await urllib.request(testSignatureObjectUrlAsync);
assert.equal(testSignatureObjectFromUrlAsync.data.toString(), testSignatureObjectFromGet.content.toString());

store.setVerifyObjectNameStrictEnabled(true);

assert.throws(() => {
try {
store.signatureUrl(testSignatureObjectName);
} catch (err) {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);
throw err;
}
}, Error);

await assert.rejects(store.asyncSignatureUrl(testSignatureObjectName), err => {
assert.strictEqual(err.message, `Invalid object name ${testSignatureObjectName}`);

return true;
});
});
});

describe('getStream()', () => {
Expand Down
Loading