feat(server): add encryption method option to the access key creation API (#1002)

src/shadowbox/integration_test/test.sh

@@ -194,6 +194,20 @@ function cleanup() {
     fi
   }
 
+  function test_encryption_for_new_keys() {
+    # Verify that we can create news keys with custom encryption.
+    client_curl --insecure -X POST -H "Content-Type: application/json" -d '{"method":"aes-256-gcm"}' "${SB_API_URL}/access-keys" \
+    || fail "Couldn't create a new access key with a custom method"
+
+    local ACCESS_KEY_JSON
+    ACCESS_KEY_JSON="$(client_curl --insecure -X GET "${SB_API_URL}/access-keys" \
+      || fail "Couldn't get a new access key after changing hostname")"
+
+    if [[ "${ACCESS_KEY_JSON}" != *'"method":"aes-256-gcm"'* ]]; then
+      fail "Custom encryption key not taken by new access key: ${ACCESS_KEY_JSON}"
+    fi
+  }
+
   function test_default_data_limit() {
     # Verify that we can create default data limits
     client_curl --insecure -X PUT -H 'Content-Type: application/json' -d '{"limit": {"bytes": 1000}}' \
@@ -231,6 +245,7 @@ function cleanup() {
   test_networking
   test_port_for_new_keys
   test_hostname_for_new_keys
+  test_encryption_for_new_keys
   test_default_data_limit
   test_per_key_data_limits
 

src/shadowbox/model/access_key.ts

@@ -50,7 +50,7 @@ export interface AccessKey {
 
 export interface AccessKeyRepository {
   // Creates a new access key. Parameters are chosen automatically.
-  createNewAccessKey(): Promise<AccessKey>;
+  createNewAccessKey(encryptionMethod?: string): Promise<AccessKey>;
   // Removes the access key given its id. Throws on failure.
   removeAccessKey(id: AccessKeyId);
   // Lists all existing access keys

src/shadowbox/model/errors.ts

@@ -39,3 +39,9 @@ export class AccessKeyNotFound extends OutlineError {
     super(`Access key "${accessKeyId}" not found`);
   }
 }
+
+export class InvalidCipher extends OutlineError {
+  constructor(public cipher: string) {
+    super(`cipher "${cipher}" is not valid`);
+  }
+}

src/shadowbox/server/api.yml

@@ -139,6 +139,18 @@ paths:
       description: Creates a new access key
       tags:
         - Access Key
+      requestBody:
+        required: false
+        content:
+          application/json:
+            schema:
+              type: object
+              properties:
+                method:
+                  type: string
+            examples:
+              '0':
+                value: '{"method":"aes-192-gcm"}'
       responses:
         '201':
           description: The newly created access key

src/shadowbox/server/manager_service.spec.ts

@@ -18,7 +18,6 @@ import * as restify from 'restify';
 
 import {InMemoryConfig, JsonConfig} from '../infrastructure/json_config';
 import {AccessKey, AccessKeyRepository, DataLimit} from '../model/access_key';
-
 import {ManagerMetrics} from './manager_metrics';
 import {bindService, ShadowsocksManagerService} from './manager_service';
 import {FakePrometheusClient, FakeShadowsocksServer} from './mocks/mocks';
@@ -279,7 +278,7 @@ describe('ShadowsocksManagerService', () => {
   });
 
   describe('createNewAccessKey', () => {
-    it('creates keys', (done) => {
+    it('verify default method', (done) => {
       const repo = getAccessKeyRepository();
       const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
 
@@ -288,18 +287,56 @@ describe('ShadowsocksManagerService', () => {
         send: (httpCode, data) => {
           expect(httpCode).toEqual(201);
           expect(Object.keys(data).sort()).toEqual(EXPECTED_ACCESS_KEY_PROPERTIES);
+          expect(data.method).toEqual('chacha20-ietf-poly1305');
           responseProcessed = true; // required for afterEach to pass.
         },
       };
       service.createNewAccessKey({params: {}}, res, done);
     });
+    it('non-default method gets set', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      // Verify that response returns a key with the expected properties.
+      const res = {
+        send: (httpCode, data) => {
+          expect(httpCode).toEqual(201);
+          expect(Object.keys(data).sort()).toEqual(EXPECTED_ACCESS_KEY_PROPERTIES);
+          expect(data.method).toEqual('aes-256-gcm');
+          responseProcessed = true; // required for afterEach to pass.
+        },
+      };
+      service.createNewAccessKey({params: {method: 'aes-256-gcm'}}, res, done);
+    });
+    it('method must be of type string', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {send: (_httpCode, _data) => {}};
+      service.createNewAccessKey({params: {method: Number('9876')}}, res, (error) => {
+        expect(error.statusCode).toEqual(400);
+        responseProcessed = true; // required for afterEach to pass.
+        done();
+      });
+    });
+    it('method must be valid', (done) => {
+      const repo = getAccessKeyRepository();
+      const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
+
+      const res = {send: (_httpCode, _data) => {}};
+      service.createNewAccessKey({params: {method: 'abcdef'}}, res, (error) => {
+        expect(error.statusCode).toEqual(400);
+        responseProcessed = true; // required for afterEach to pass.
+        done();
+      });
+    });
     it('Create returns a 500 when the repository throws an exception', (done) => {
       const repo = getAccessKeyRepository();
       spyOn(repo, 'createNewAccessKey').and.throwError('cannot write to disk');
       const service = new ShadowsocksManagerServiceBuilder().accessKeys(repo).build();
 
-      const res = {send: SEND_NOTHING};
-      service.createNewAccessKey({params: {}}, res, (error) => {
+      const res = {send: (_httpCode, _data) => {}};
+      service.createNewAccessKey({params: {method: 'aes-192-gcm'}}, res, (error) => {
         expect(error.statusCode).toEqual(500);
         responseProcessed = true; // required for afterEach to pass.
         done();

src/shadowbox/server/manager_service.ts

@@ -61,6 +61,7 @@ interface RequestParams {
   //   limit: DataLimit
   //   port: number
   //   hours: number
+  //   method: string
   [param: string]: unknown;
 }
 // Simplified request and response type interfaces containing only the
@@ -289,17 +290,28 @@ export class ShadowsocksManagerService {
   }
 
   // Creates a new access key
-  public createNewAccessKey(req: RequestType, res: ResponseType, next: restify.Next): void {
+  public async createNewAccessKey(req: RequestType, res: ResponseType, next: restify.Next): Promise<void> {
     try {
       logging.debug(`createNewAccessKey request ${JSON.stringify(req.params)}`);
-      this.accessKeys.createNewAccessKey().then((accessKey) => {
-        const accessKeyJson = accessKeyToApiJson(accessKey);
-        res.send(201, accessKeyJson);
-        logging.debug(`createNewAccessKey response ${JSON.stringify(accessKeyJson)}`);
-        return next();
-      });
-    } catch (error) {
+      let encryptionMethod = req.params.method;
+      if (!encryptionMethod) {
+        encryptionMethod = '';
+      }
+      if (typeof encryptionMethod !== 'string') {
+        return next(new restifyErrors.InvalidArgumentError(
+            {statusCode: 400},
+            `Expected a string encryptionMethod, instead got ${encryptionMethod} of type ${
+                typeof encryptionMethod}`));
+      }
+      const accessKeyJson = accessKeyToApiJson(await this.accessKeys.createNewAccessKey(encryptionMethod));
+      res.send(201, accessKeyJson);
+      logging.debug(`createNewAccessKey response ${JSON.stringify(accessKeyJson)}`);
+      return next();
+    } catch(error) {
       logging.error(error);
+      if (error instanceof errors.InvalidCipher) {
+        return next(new restifyErrors.InvalidArgumentError({statusCode: 400}, error.message));
+      }
       return next(new restifyErrors.InternalServerError());
     }
   }

src/shadowbox/server/server_access_key.spec.ts

@@ -38,6 +38,24 @@ describe('ServerAccessKeyRepository', () => {
     });
   });
 
+  it('New access keys have the correct default encryption method', (done) => {
+    const repo = new RepoBuilder().build();
+    repo.createNewAccessKey().then((accessKey) => {
+      expect(accessKey).toBeDefined();
+      expect(accessKey.proxyParams.encryptionMethod).toEqual('chacha20-ietf-poly1305');
+      done();
+    });
+  });
+
+  it('New access keys sees the encryption method correctly', (done) => {
+    const repo = new RepoBuilder().build();
+    repo.createNewAccessKey('aes-256-gcm').then((accessKey) => {
+      expect(accessKey).toBeDefined();
+      expect(accessKey.proxyParams.encryptionMethod).toEqual('aes-256-gcm');
+      done();
+    });
+  });
+
   it('Creates access keys under limit', async (done) => {
     const repo = new RepoBuilder().build();
     const accessKey = await repo.createNewAccessKey();

src/shadowbox/server/server_access_key.ts

@@ -96,6 +96,13 @@ function accessKeyToStorageJson(accessKey: AccessKey): AccessKeyStorageJson {
   };
 }
 
+function isValidCipher(cipher: string): boolean {
+    if (["aes-256-gcm", "aes-192-gcm", "aes-128-gcm", "chacha20-ietf-poly1305"].indexOf(cipher) === -1) {
+      return false;
+    }
+    return true;
+}
+
 // AccessKeyRepository that keeps its state in a config file and uses ShadowsocksServer
 // to start and stop per-access-key Shadowsocks instances.  Requires external validation
 // that portForNewAccessKeys is valid.
@@ -159,15 +166,20 @@ export class ServerAccessKeyRepository implements AccessKeyRepository {
     this.portForNewAccessKeys = port;
   }
 
-  async createNewAccessKey(): Promise<AccessKey> {
+  async createNewAccessKey(encryptionMethod?: string): Promise<AccessKey> {
     const id = this.keyConfig.data().nextId.toString();
     this.keyConfig.data().nextId += 1;
     const metricsId = uuidv4();
     const password = generatePassword();
+    encryptionMethod = encryptionMethod || this.NEW_USER_ENCRYPTION_METHOD;
+    // Validate encryption method.
+    if (!isValidCipher(encryptionMethod)) {
+      throw new errors.InvalidCipher(encryptionMethod);
+    }
     const proxyParams = {
       hostname: this.proxyHostname,
       portNumber: this.portForNewAccessKeys,
-      encryptionMethod: this.NEW_USER_ENCRYPTION_METHOD,
+      encryptionMethod,
       password,
     };
     const accessKey = new ServerAccessKey(id, '', metricsId, proxyParams);