CERBERUS/outline-server
1
0
Fork 0
mirror of https://github.com/OutlineFoundation/outline-server.git synced 2026-08-04 06:35:06 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fix(server): block access when traffic limit is set to 0 bytes

Browse source 
* Fix isOverDataLimit logic to stop allowing 0 traffic byte

issue #1515 

This if statement is just wrong and doesn't block access when traffic limit is set to 0 byte. This one character fixes it.

Now if we create a new access key, set the limit to 0 before first usage, the client no longer will be able to connect.

* Rename `isOverDataLimit` to `reachedDataLimit`.

* Add a test case to prevent regression.

* Rename old variable.

* Update comment now that variable has changed meaning slightly.

Co-authored-by: Vinicius Fortuna <fortuna@users.noreply.github.com>

---------

Co-authored-by: sbruens <sbruens@users.noreply.github.com>
Co-authored-by: Vinicius Fortuna <fortuna@users.noreply.github.com>
This commit is contained in:
cornzzy 2024-07-24 22:51:29 +03:30 • committed by GitHub
parent b5dbe650f5
commit ee039ca596
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 59 additions and 44 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/shadowbox/model/access_key.ts
View file

@ -39,8 +39,8 @@ export interface AccessKey {
readonly name: string;
// Parameters to access the proxy
readonly proxyParams: ProxyParams;
// Whether the access key has exceeded the data transfer limit.
readonly isOverDataLimit: boolean;
// Whether the access key has reached the data transfer limit.
readonly reachedDataLimit: boolean;
// The key's current data limit. If it exists, it overrides the server default data limit.
readonly dataLimit?: DataLimit;
}

89
src/shadowbox/server/server_access_key.spec.ts
View file

@ -183,7 +183,7 @@ describe('ServerAccessKeyRepository', () => {
it('Creates access keys under limit', async (done) => {
const repo = new RepoBuilder().build();
const accessKey = await repo.createNewAccessKey();
expect(accessKey.isOverDataLimit).toBeFalsy();
expect(accessKey.reachedDataLimit).toBeFalsy();
done();
});
@ -346,13 +346,13 @@ describe('ServerAccessKeyRepository', () => {
const key = await repo.createNewAccessKey();
await setKeyLimitAndEnforce(repo, key.id, {bytes: 0});
expect(key.isOverDataLimit).toBeTruthy();
expect(key.reachedDataLimit).toBeTruthy();
let serverKeys = server.getAccessKeys();
expect(serverKeys.length).toEqual(0);
await setKeyLimitAndEnforce(repo, key.id, {bytes: 1000});
expect(key.isOverDataLimit).toBeFalsy();
expect(key.reachedDataLimit).toBeFalsy();
serverKeys = server.getAccessKeys();
expect(serverKeys.length).toEqual(1);
expect(serverKeys[0].id).toEqual(key.id);
@ -371,13 +371,13 @@ describe('ServerAccessKeyRepository', () => {
const higherLimitThanDefault = await repo.createNewAccessKey();
await repo.setDefaultDataLimit({bytes: 1000});
expect(lowerLimitThanDefault.isOverDataLimit).toBeFalsy();
expect(lowerLimitThanDefault.reachedDataLimit).toBeFalsy();
await setKeyLimitAndEnforce(repo, lowerLimitThanDefault.id, {bytes: 500});
expect(lowerLimitThanDefault.isOverDataLimit).toBeTruthy();
expect(lowerLimitThanDefault.reachedDataLimit).toBeTruthy();
expect(higherLimitThanDefault.isOverDataLimit).toBeTruthy();
expect(higherLimitThanDefault.reachedDataLimit).toBeTruthy();
await setKeyLimitAndEnforce(repo, higherLimitThanDefault.id, {bytes: 1500});
expect(higherLimitThanDefault.isOverDataLimit).toBeFalsy();
expect(higherLimitThanDefault.reachedDataLimit).toBeFalsy();
done();
});
@ -404,10 +404,10 @@ describe('ServerAccessKeyRepository', () => {
await repo.start(new ManualClock());
await repo.setDefaultDataLimit({bytes: 0});
await setKeyLimitAndEnforce(repo, key.id, {bytes: 1000});
expect(key.isOverDataLimit).toBeFalsy();
expect(key.reachedDataLimit).toBeFalsy();
await removeKeyLimitAndEnforce(repo, key.id);
expect(key.isOverDataLimit).toBeTruthy();
expect(key.reachedDataLimit).toBeTruthy();
done();
});
@ -422,13 +422,13 @@ describe('ServerAccessKeyRepository', () => {
const key = await repo.createNewAccessKey();
await setKeyLimitAndEnforce(repo, key.id, {bytes: 0});
expect(key.isOverDataLimit).toBeTruthy();
expect(key.reachedDataLimit).toBeTruthy();
let serverKeys = server.getAccessKeys();
expect(serverKeys.length).toEqual(0);
await setKeyLimitAndEnforce(repo, key.id, {bytes: 1000});
expect(key.isOverDataLimit).toBeFalsy();
expect(key.reachedDataLimit).toBeFalsy();
serverKeys = server.getAccessKeys();
expect(serverKeys.length).toEqual(1);
expect(serverKeys[0].id).toEqual(key.id);
@ -447,13 +447,13 @@ describe('ServerAccessKeyRepository', () => {
const higherLimitThanDefault = await repo.createNewAccessKey();
await repo.setDefaultDataLimit({bytes: 1000});
expect(lowerLimitThanDefault.isOverDataLimit).toBeFalsy();
expect(lowerLimitThanDefault.reachedDataLimit).toBeFalsy();
await setKeyLimitAndEnforce(repo, lowerLimitThanDefault.id, {bytes: 500});
expect(lowerLimitThanDefault.isOverDataLimit).toBeTruthy();
expect(lowerLimitThanDefault.reachedDataLimit).toBeTruthy();
expect(higherLimitThanDefault.isOverDataLimit).toBeTruthy();
expect(higherLimitThanDefault.reachedDataLimit).toBeTruthy();
await setKeyLimitAndEnforce(repo, higherLimitThanDefault.id, {bytes: 1500});
expect(higherLimitThanDefault.isOverDataLimit).toBeFalsy();
expect(higherLimitThanDefault.reachedDataLimit).toBeFalsy();
done();
});
@ -487,10 +487,10 @@ describe('ServerAccessKeyRepository', () => {
await repo.start(new ManualClock());
await repo.setDefaultDataLimit({bytes: 0});
await setKeyLimitAndEnforce(repo, key.id, {bytes: 1000});
expect(key.isOverDataLimit).toBeFalsy();
expect(key.reachedDataLimit).toBeFalsy();
await removeKeyLimitAndEnforce(repo, key.id);
expect(key.isOverDataLimit).toBeTruthy();
expect(key.reachedDataLimit).toBeTruthy();
done();
});
@ -505,11 +505,11 @@ describe('ServerAccessKeyRepository', () => {
await repo.start(new ManualClock());
await setKeyLimitAndEnforce(repo, key.id, {bytes: 0});
expect(key.isOverDataLimit).toBeTruthy();
expect(key.reachedDataLimit).toBeTruthy();
expect(server.getAccessKeys().length).toEqual(0);
await removeKeyLimitAndEnforce(repo, key.id);
expect(key.isOverDataLimit).toBeFalsy();
expect(key.reachedDataLimit).toBeFalsy();
expect(server.getAccessKeys().length).toEqual(1);
done();
});
@ -537,8 +537,8 @@ describe('ServerAccessKeyRepository', () => {
// We enforce asynchronously, in setAccessKeyDataLimit, so explicitly call it here to make sure
// enforcement is done before we make assertions.
await repo.enforceAccessKeyDataLimits();
expect(accessKey1.isOverDataLimit).toBeTruthy();
expect(accessKey2.isOverDataLimit).toBeFalsy();
expect(accessKey1.reachedDataLimit).toBeTruthy();
expect(accessKey2.reachedDataLimit).toBeFalsy();
// We determine which access keys have been enabled/disabled by accessing them from
// the server's perspective, ensuring `server.update` has been called.
let serverAccessKeys = server.getAccessKeys();
@ -549,8 +549,8 @@ describe('ServerAccessKeyRepository', () => {
prometheusClient.bytesTransferredById = {'0': 500, '1': 1000};
repo.setDefaultDataLimit({bytes: 700});
await repo.enforceAccessKeyDataLimits();
expect(accessKey1.isOverDataLimit).toBeFalsy();
expect(accessKey2.isOverDataLimit).toBeTruthy();
expect(accessKey1.reachedDataLimit).toBeFalsy();
expect(accessKey2.reachedDataLimit).toBeTruthy();
serverAccessKeys = server.getAccessKeys();
expect(serverAccessKeys.length).toEqual(1);
expect(serverAccessKeys[0].id).toEqual(accessKey1.id);
@ -586,8 +586,8 @@ describe('ServerAccessKeyRepository', () => {
// enforcement is done before we make assertions.
await repo.enforceAccessKeyDataLimits();
expect(server.getAccessKeys().length).toEqual(2);
expect(accessKey1.isOverDataLimit).toBeFalsy();
expect(accessKey2.isOverDataLimit).toBeFalsy();
expect(accessKey1.reachedDataLimit).toBeFalsy();
expect(accessKey2.reachedDataLimit).toBeFalsy();
done();
});
@ -609,7 +609,7 @@ describe('ServerAccessKeyRepository', () => {
}
await repo.enforceAccessKeyDataLimits();
for (const key of repo.listAccessKeys()) {
expect(key.isOverDataLimit).toEqual(
expect(key.reachedDataLimit).toEqual(
prometheusClient.bytesTransferredById[key.id] > limit.bytes
);
}
@ -618,7 +618,7 @@ describe('ServerAccessKeyRepository', () => {
await repo.enforceAccessKeyDataLimits();
for (const key of repo.listAccessKeys()) {
expect(key.isOverDataLimit).toEqual(
expect(key.reachedDataLimit).toEqual(
prometheusClient.bytesTransferredById[key.id] > limit.bytes
);
}
@ -636,14 +636,14 @@ describe('ServerAccessKeyRepository', () => {
await setKeyLimitAndEnforce(repo, perKeyLimited.id, {bytes: 100});
await repo.enforceAccessKeyDataLimits();
expect(perKeyLimited.isOverDataLimit).toBeTruthy();
expect(defaultLimited.isOverDataLimit).toBeFalsy();
expect(perKeyLimited.reachedDataLimit).toBeTruthy();
expect(defaultLimited.reachedDataLimit).toBeFalsy();
prometheusClient.bytesTransferredById[perKeyLimited.id] = 50;
prometheusClient.bytesTransferredById[defaultLimited.id] = 600;
await repo.enforceAccessKeyDataLimits();
expect(perKeyLimited.isOverDataLimit).toBeFalsy();
expect(defaultLimited.isOverDataLimit).toBeTruthy();
expect(perKeyLimited.reachedDataLimit).toBeFalsy();
expect(defaultLimited.reachedDataLimit).toBeTruthy();
done();
});
@ -673,6 +673,21 @@ describe('ServerAccessKeyRepository', () => {
done();
});
it('enforceAccessKeyDataLimits disables on exact data limit', async (done) => {
const server = new FakeShadowsocksServer();
const prometheusClient = new FakePrometheusClient({'0': 0});
const repo = new RepoBuilder()
.prometheusClient(prometheusClient)
.shadowsocksServer(server)
.build();
await repo.createNewAccessKey({dataLimit: {bytes: 0}});
await repo.enforceAccessKeyDataLimits();
expect(server.getAccessKeys().length).toEqual(0);
done();
});
it('Repos created with an existing file restore access keys', async (done) => {
const config = new InMemoryConfig<AccessKeyConfigJson>({accessKeys: [], nextId: 0});
const repo1 = new RepoBuilder().keyConfig(config).build();
@ -741,9 +756,9 @@ describe('ServerAccessKeyRepository', () => {
await repo.start(clock);
await clock.runCallbacks();
expect(accessKey1.isOverDataLimit).toBeTruthy();
expect(accessKey2.isOverDataLimit).toBeFalsy();
expect(accessKey3.isOverDataLimit).toBeTruthy();
expect(accessKey1.reachedDataLimit).toBeTruthy();
expect(accessKey2.reachedDataLimit).toBeFalsy();
expect(accessKey3.reachedDataLimit).toBeTruthy();
let serverAccessKeys = await server.getAccessKeys();
expect(serverAccessKeys.length).toEqual(1);
expect(serverAccessKeys[0].id).toEqual(accessKey2.id);
@ -751,9 +766,9 @@ describe('ServerAccessKeyRepository', () => {
// Simulate a change in usage.
prometheusClient.bytesTransferredById = {'0': 100, '1': 200, '2': 1000};
await clock.runCallbacks();
expect(accessKey1.isOverDataLimit).toBeFalsy();
expect(accessKey2.isOverDataLimit).toBeFalsy();
expect(accessKey3.isOverDataLimit).toBeTruthy();
expect(accessKey1.reachedDataLimit).toBeFalsy();
expect(accessKey2.reachedDataLimit).toBeFalsy();
expect(accessKey3.reachedDataLimit).toBeTruthy();
serverAccessKeys = await server.getAccessKeys();
expect(serverAccessKeys.length).toEqual(2);
expect(serverAccessKeys[0].id).toEqual(accessKey1.id);

10
src/shadowbox/server/server_access_key.ts
View file

@ -50,7 +50,7 @@ export interface AccessKeyConfigJson {
// AccessKey implementation with write access enabled on properties that may change.
class ServerAccessKey implements AccessKey {
isOverDataLimit = false;
reachedDataLimit = false;
constructor(
readonly id: AccessKeyId,
public name: string,
@ -308,13 +308,13 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
let limitStatusChanged = false;
for (const accessKey of this.accessKeys) {
const usageBytes = bytesTransferredById[accessKey.id] ?? 0;
const wasOverDataLimit = accessKey.isOverDataLimit;
const oldReachedDataLimit = accessKey.reachedDataLimit;
let limitBytes = (accessKey.dataLimit ?? this._defaultDataLimit)?.bytes;
if (limitBytes === undefined) {
limitBytes = Number.POSITIVE_INFINITY;
}
accessKey.isOverDataLimit = usageBytes > limitBytes;
limitStatusChanged = accessKey.isOverDataLimit !== wasOverDataLimit || limitStatusChanged;
accessKey.reachedDataLimit = usageBytes >= limitBytes;
limitStatusChanged = accessKey.reachedDataLimit !== oldReachedDataLimit || limitStatusChanged;
}
if (limitStatusChanged) {
await this.updateServer();
@ -323,7 +323,7 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
private updateServer(): Promise<void> {
const serverAccessKeys = this.accessKeys
.filter((key) => !key.isOverDataLimit)
.filter((key) => !key.reachedDataLimit)
.map((key) => {
return {
id: key.id,