From dd5f27b77d4680e564c391f3db938bdece4b6b15 Mon Sep 17 00:00:00 2001 From: Leon Li Date: Thu, 4 Mar 2021 20:26:52 -0500 Subject: [PATCH 1/5] change dummy string used to generate continuation tokens to avoid confusion with access keys --- lib/controllers/bucket.js | 4 ++-- lib/middleware/authentication.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/controllers/bucket.js b/lib/controllers/bucket.js index de5a8d5f..8491935b 100644 --- a/lib/controllers/bucket.js +++ b/lib/controllers/bucket.js @@ -11,7 +11,7 @@ const { const { utf8BodyParser } = require('../utils'); function generateContinuationToken(bucket, keyName, region) { - const key = Buffer.alloc(8, 'S3RVER', 'utf8'); + const key = Buffer.alloc(8, 'S3RVER_CONTINUATION_TOKEN_SECRET', 'utf8'); const iv = crypto.randomBytes(8); // ensure the first byte of IV lies between [212, 216) iv[0] = (iv[0] & 0b00000011) | 0b11010100; @@ -28,7 +28,7 @@ function generateContinuationToken(bucket, keyName, region) { function decipherContinuationToken(token) { const buf = Buffer.from(token, 'base64'); if (buf.length < 8) return ''; - const key = Buffer.alloc(8, 'S3RVER', 'utf8'); + const key = Buffer.alloc(8, 'S3RVER_CONTINUATION_TOKEN_SECRET', 'utf8'); const iv = buf.slice(0, 8); const decipher = crypto.createDecipheriv('des', key, iv); const ciphertext = buf.slice(8); diff --git a/lib/middleware/authentication.js b/lib/middleware/authentication.js index 9d08f6ae..c8f04e7b 100644 --- a/lib/middleware/authentication.js +++ b/lib/middleware/authentication.js @@ -198,7 +198,7 @@ module.exports = () => } else if (signature.version === 4) { stringToSign = getStringToSignV4(canonicalRequest, signature); - const secretKey = account.accessKeys.get(signature.accessKeyId); + const secretKey = account.getSecretAccessKey(signature.accessKeyId); const calculatedSignature = calculateSignatureV4( stringToSign, secretKey, From ffcb17e4846486180aba32e37552921b3437bccc Mon Sep 17 00:00:00 2001 From: Leon Li Date: Thu, 4 Mar 2021 20:26:55 -0500 Subject: [PATCH 2/5] rewrite account & key pair handling logic to allow adding default user based on S3rver configuration --- lib/controllers/bucket.js | 5 +-- lib/controllers/object.js | 5 +-- lib/controllers/service.js | 6 +-- lib/middleware/authentication.js | 5 +-- lib/models/account.js | 66 ++++++++++++++++++++++++-------- lib/s3rver.js | 18 +++++++++ 6 files changed, 76 insertions(+), 29 deletions(-) diff --git a/lib/controllers/bucket.js b/lib/controllers/bucket.js index 8491935b..cdcdb7b0 100644 --- a/lib/controllers/bucket.js +++ b/lib/controllers/bucket.js @@ -2,7 +2,6 @@ const crypto = require('crypto'); -const { DUMMY_ACCOUNT } = require('../models/account'); const S3Error = require('../models/error'); const { S3CorsConfiguration, @@ -270,8 +269,8 @@ exports.getBucket = async function getBucket(ctx) { Size: object.size, Owner: options.fetchOwner ? { - ID: DUMMY_ACCOUNT.id, - DisplayName: DUMMY_ACCOUNT.displayName, + ID: ctx.state.account.id, + DisplayName: ctx.state.account.displayName, } : undefined, StorageClass: 'STANDARD', diff --git a/lib/controllers/object.js b/lib/controllers/object.js index dfbf42bf..5310fcdf 100644 --- a/lib/controllers/object.js +++ b/lib/controllers/object.js @@ -6,7 +6,6 @@ const xmlParser = require('fast-xml-parser'); const he = require('he'); const { URL } = require('url'); -const { DUMMY_ACCOUNT } = require('../models/account'); const S3Error = require('../models/error'); const S3Event = require('../models/event'); const S3Object = require('../models/object'); @@ -209,8 +208,8 @@ exports.getObjectAcl = async function getObjectAcl(ctx) { AccessControlPolicy: { '@': { xmlns: 'http://doc.s3.amazonaws.com/2006-03-01/' }, Owner: { - ID: DUMMY_ACCOUNT.id, - DisplayName: DUMMY_ACCOUNT.displayName, + ID: ctx.state.account.id, + DisplayName: ctx.state.account.displayName, }, AccessControlList: { Grant: { diff --git a/lib/controllers/service.js b/lib/controllers/service.js index 223fae4d..54cf3c37 100644 --- a/lib/controllers/service.js +++ b/lib/controllers/service.js @@ -1,7 +1,5 @@ 'use strict'; -const { DUMMY_ACCOUNT } = require('../models/account'); - /* * Operations on the Service * The following methods correspond to operations you can perform on the Amazon S3 service. @@ -21,8 +19,8 @@ exports.getService = async function getService(ctx) { ListAllMyBucketsResult: { '@': { xmlns: 'http://doc.s3.amazonaws.com/2006-03-01/' }, Owner: { - ID: DUMMY_ACCOUNT.id, - DisplayName: DUMMY_ACCOUNT.displayName, + ID: ctx.state.account.id, + DisplayName: ctx.state.account.displayName, }, Buckets: { Bucket: buckets.map((bucket) => ({ diff --git a/lib/middleware/authentication.js b/lib/middleware/authentication.js index c8f04e7b..1593e9a2 100644 --- a/lib/middleware/authentication.js +++ b/lib/middleware/authentication.js @@ -3,7 +3,6 @@ const { createHmac, createHash } = require('crypto'); const { mapKeys, pickBy } = require('lodash'); -const AWSAccount = require('../models/account'); const S3Error = require('../models/error'); const { RESPONSE_HEADERS } = require('./response-header-override'); const { @@ -174,7 +173,7 @@ module.exports = () => } let stringToSign; - const account = AWSAccount.registry.get(signature.accessKeyId); + const account = ctx.app.accounts.getByAccessKeyId(signature.accessKeyId); if (!account) { throw new S3Error( 'InvalidAccessKeyId', @@ -186,7 +185,7 @@ module.exports = () => if (signature.version === 2) { stringToSign = getStringToSignV2(canonicalRequest); - const signingKey = account.accessKeys.get(signature.accessKeyId); + const signingKey = account.getSecretAccessKey(signature.accessKeyId); const calculatedSignature = calculateSignatureV2( stringToSign, signingKey, diff --git a/lib/models/account.js b/lib/models/account.js index 301aff15..32ac89e0 100644 --- a/lib/models/account.js +++ b/lib/models/account.js @@ -1,26 +1,60 @@ 'use strict'; -class AWSAccount { - constructor(accountId, displayName) { - this.id = accountId; - this.displayName = displayName; - this.accessKeys = new Map(); +// basic in-memory storage abstraction for accounts and AWS credential key pairs + +class Account { + constructor(id, displayName) { + Object.assign(this, { id, displayName }); + this.secretAccessKeys = new Map(); + } + + assignKeyPair(accessKeyId, secretAccessKey) { + this.secretAccessKeys.set(accessKeyId, secretAccessKey); } - createKeyPair(accessKeyId, secretAccessKey) { - AWSAccount.registry.set(accessKeyId, this); - this.accessKeys.set(accessKeyId, secretAccessKey); + getSecretAccessKey(accessKeyId) { + return this.secretAccessKeys.get(accessKeyId); } - revokeAccessKey(accessKeyId) { - AWSAccount.registry.delete(accessKeyId); - this.accessKeys.delete(accessKeyId); + deleteKeyPair(accessKeyId) { + this.secretAccessKeys.delete(accessKeyId); } } -AWSAccount.registry = new Map(); -exports = module.exports = AWSAccount; +class AccountStore { + constructor() { + // track accounts by both account id and access key id + this.accountsById = new Map(); + this.accountsByAccessKeyId = new Map(); + } + + addAccount(accountId, displayName) { + const account = new Account(accountId, displayName); + this.accountsById.set(accountId, account); + } + + addKeyPair(accountId, accessKeyId, secretAccessKey) { + const account = this.accountsById.get(accountId); + account.assignKeyPair(accessKeyId, secretAccessKey); + this.accountsByAccessKeyId.set(accessKeyId, account); + } + + revokeKeyPair(accessKeyId) { + const account = this.accountsByAccessKeyId.get(accessKeyId); + account.deleteKeyPair(accessKeyId); + this.accountsByAccessKeyId.delete(accessKeyId); + } + + getByAccessKeyId(accessKeyId) { + return this.accountsByAccessKeyId.get(accessKeyId); + } + + removeAccount(accountId) { + const account = this.accountsById.get(accountId); + const accessKeyIds = [...account.secretAccessKeys.keys()]; + accessKeyIds.forEach((accessKeyId) => this.revokeKeyPair(accessKeyId)); + this.accountsById.delete(accountId); + } +} -// Hardcoded dummy user used for authenticated requests -exports.DUMMY_ACCOUNT = new AWSAccount(123456789000, 'S3rver'); -exports.DUMMY_ACCOUNT.createKeyPair('S3RVER', 'S3RVER'); +module.exports = AccountStore; diff --git a/lib/s3rver.js b/lib/s3rver.js index f45c86aa..434ab805 100644 --- a/lib/s3rver.js +++ b/lib/s3rver.js @@ -15,9 +15,13 @@ const vhostMiddleware = require('./middleware/vhost'); const { getConfigModel } = require('./models/config'); const S3Error = require('./models/error'); const FilesystemStore = require('./stores/filesystem'); +const AccountStore = require('./models/account'); const router = require('./routes'); const { getXmlRootTag } = require('./utils'); +const defaultAccountId = 123456789000; +const defaultAccountDisplayName = 'S3rver'; + class S3rver extends Koa { constructor(options) { super(); @@ -30,6 +34,8 @@ class S3rver extends Koa { allowMismatchedSignatures, vhostBuckets, configureBuckets, + defaultAccessKeyId, + defaultSecretAccessKey, ...serverOptions } = defaults({}, options, S3rver.defaultOptions); this.serverOptions = serverOptions; @@ -39,6 +45,16 @@ class S3rver extends Koa { this.allowMismatchedSignatures = allowMismatchedSignatures; this.store = this.context.store = new FilesystemStore(directory); + this.accounts = new AccountStore(); + + // create a default account + this.accounts.addAccount(defaultAccountId, defaultAccountDisplayName); + this.accounts.addKeyPair( + defaultAccountId, + defaultAccessKeyId, + defaultSecretAccessKey, + ); + // Log all requests this.use(loggerMiddleware(this, silent)); @@ -216,6 +232,8 @@ S3rver.defaultOptions = { allowMismatchedSignatures: false, vhostBuckets: true, configureBuckets: [], + defaultAccessKeyId: 'S3RVER', + defaultSecretAccessKey: 'S3RVER', }; S3rver.prototype.getMiddleware = S3rver.prototype.callback; From e91f76ddb2deca928e03acb54de372f23928c0d9 Mon Sep 17 00:00:00 2001 From: Leon Li Date: Thu, 4 Mar 2021 20:26:57 -0500 Subject: [PATCH 3/5] authenticated user is unavailable in ListAllMyBuckets service request --- lib/controllers/service.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/controllers/service.js b/lib/controllers/service.js index 54cf3c37..573672a3 100644 --- a/lib/controllers/service.js +++ b/lib/controllers/service.js @@ -19,8 +19,8 @@ exports.getService = async function getService(ctx) { ListAllMyBucketsResult: { '@': { xmlns: 'http://doc.s3.amazonaws.com/2006-03-01/' }, Owner: { - ID: ctx.state.account.id, - DisplayName: ctx.state.account.displayName, + ID: 'BUCKET_OWNER_ID', + DisplayName: 'BUCKET_OWNER_DISPLAY_NAME', }, Buckets: { Bucket: buckets.map((bucket) => ({ From f0d087dcd0ff34dd06a8e9e18a099521b71d5776 Mon Sep 17 00:00:00 2001 From: Leon Li Date: Thu, 4 Mar 2021 20:26:58 -0500 Subject: [PATCH 4/5] add comments to indicate dummy nature of 'Owner' result fields --- lib/controllers/bucket.js | 1 + lib/controllers/object.js | 1 + lib/controllers/service.js | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/controllers/bucket.js b/lib/controllers/bucket.js index cdcdb7b0..1e7e33bc 100644 --- a/lib/controllers/bucket.js +++ b/lib/controllers/bucket.js @@ -269,6 +269,7 @@ exports.getBucket = async function getBucket(ctx) { Size: object.size, Owner: options.fetchOwner ? { + // we assume the objects are owned by the fetching user since we don't store owner metadata with objects ID: ctx.state.account.id, DisplayName: ctx.state.account.displayName, } diff --git a/lib/controllers/object.js b/lib/controllers/object.js index 5310fcdf..71eb3639 100644 --- a/lib/controllers/object.js +++ b/lib/controllers/object.js @@ -208,6 +208,7 @@ exports.getObjectAcl = async function getObjectAcl(ctx) { AccessControlPolicy: { '@': { xmlns: 'http://doc.s3.amazonaws.com/2006-03-01/' }, Owner: { + // we assume the objects are owned by the fetching user since we don't store owner metadata with objects ID: ctx.state.account.id, DisplayName: ctx.state.account.displayName, }, diff --git a/lib/controllers/service.js b/lib/controllers/service.js index 573672a3..58203be7 100644 --- a/lib/controllers/service.js +++ b/lib/controllers/service.js @@ -19,6 +19,7 @@ exports.getService = async function getService(ctx) { ListAllMyBucketsResult: { '@': { xmlns: 'http://doc.s3.amazonaws.com/2006-03-01/' }, Owner: { + // we provide dummy values here because we don't store any metadata with buckets ID: 'BUCKET_OWNER_ID', DisplayName: 'BUCKET_OWNER_DISPLAY_NAME', }, From a2c333776feb099692d18fd426ec6712fd640ad0 Mon Sep 17 00:00:00 2001 From: Leon Li Date: Thu, 4 Mar 2021 20:26:59 -0500 Subject: [PATCH 5/5] add cli documentation for defaultAccessKeyId and defaultSecretAccessKey --- lib/cli.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/cli.js b/lib/cli.js index 9f404300..9ea7e3d1 100644 --- a/lib/cli.js +++ b/lib/cli.js @@ -72,6 +72,16 @@ program 'Bucket name and configuration files for creating and configuring a bucket at startup', parseConfigureBucket, ) + .option( + '--default-access-key-id ', + 'Access Key ID used to authenticate with s3rver.', + S3rver.defaultOptions.defaultAccessKeyId, + ) + .option( + '--default-secret-access-key ', + 'Secret Access Key used to authenticate with s3rver.', + S3rver.defaultOptions.defaultSecretAccessKey, + ) .version(pkg.version, '-v, --version'); // NOTE: commander doesn't support repeated variadic options,