Skip to content

Commit

Permalink
Use s3 for temporary storage
Browse files Browse the repository at this point in the history 
We were using the local file system for storing files temporarily as
they were being uploaded to the SFTP service.  We knew this was not
a long term solution for several reasons, including the security risk of
a denial of service by filling up the disk space.

We're moving to the long term solution of using S3 directly for that
upload.  We can't use the existing upload-service and permanent API for
this because, at least for now, it requires us to know the size of the
file AND does not allow us to upload over multiple parts.

For this reason we're integrating with S3 directly.

Issue #145 A user could create a DOS by uploading files that are too large
  • Loading branch information
@slifty
slifty committed May 25, 2023
1 parent 520015e commit a71b99f
Show file tree
Hide file tree
Showing 9 changed files with 9,898 additions and 6,456 deletions.
17 changes: 17 additions & 0 deletions .env.example
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@ SSH_HOST=${SSH_HOSTNAME}
# e.g. ./keys/host.key
SSH_HOST_KEY_PATH=${SSH_HOST_KEY_PATH}

# The location of an instance of upload-service
# e.g. 'localhost:3000'
UPLOAD_SERVICE_API_BASE_PATH=

# The S3 bucket to use for temporary file storage
# e.g. 'permanent-local'
TEMPORARY_FILE_S3_BUCKET=
TEMPORARY_FILE_S3_BUCKET_REGION=

# Any sub-path within the `local` bucket relevant to your environment
# e.g. '_YourNameHere'
TEMPORARY_FILE_S3_SUBDIRECTORY=

# S3 Credentials
AWS_ACCESS_KEY_ID=
AWS_ACCESS_SECRET=

# This is for local dev
export NODE_TLS_REJECT_UNAUTHORIZED=0

Expand Down
15,973 changes: 9,621 additions & 6,352 deletions package-lock.json
View file

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,9 @@
"typescript": "^5.0.4"
},
"dependencies": {
"@aws-sdk/client-s3": "^3.332.0",
"@fusionauth/typescript-client": "^1.45.0",
"@permanentorg/sdk": "^0.5.4",
"@permanentorg/sdk": "^0.6.0",
"@sentry/node": "^7.52.0",
"dotenv": "^16.0.3",
"logform": "^2.3.2",
Expand Down
14 changes: 6 additions & 8 deletions src/classes/PermanentFileSystem.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
getArchiveSlugFromPath,
getOriginalFileForArchiveRecord,
} from '../utils';
import type { Readable } from 'stream';
import type {
Archive,
ClientConfiguration,
Expand All @@ -42,6 +41,7 @@ import type {
Attributes,
FileEntry,
} from 'ssh2';
import type { TemporaryFile } from './TemporaryFile';

const isRootPath = (requestedPath: string): boolean => (
requestedPath === '/'
Expand Down Expand Up @@ -225,19 +225,17 @@ export class PermanentFileSystem {
}

public async createFile(
requestedPath: string,
dataStream: Readable,
size: number,
temporaryFile: TemporaryFile,
): Promise<void> {
const parentPath = path.dirname(requestedPath);
const archiveRecordName = path.basename(requestedPath);
const parentPath = path.dirname(temporaryFile.permanentFileSystemPath);
const archiveRecordName = path.basename(temporaryFile.permanentFileSystemPath);
const parentFolder = await this.loadFolder(parentPath);
await createArchiveRecord(
this.getClientConfiguration(),
dataStream,
temporaryFile.url,
{
contentType: 'application/octet-stream',
size,
size: temporaryFile.size,
},
{
displayName: archiveRecordName,
Expand Down
161 changes: 66 additions & 95 deletions src/classes/SftpSessionHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@ import path from 'path';
import fetch from 'node-fetch';
import { v4 as uuidv4 } from 'uuid';
import ssh2 from 'ssh2';
import tmp from 'tmp';
import { logger } from '../logger';
import { generateFileEntry } from '../utils';
import { TemporaryFile } from './TemporaryFile';
import type { AuthenticationSession } from './AuthenticationSession';
import type { PermanentFileSystem } from './PermanentFileSystem';
import type { PermanentFileSystemManager } from './PermanentFileSystemManager';
import type { FileResult } from 'tmp';
import type {
Attributes,
FileEntry,
Expand All @@ -23,10 +22,6 @@ const SFTP_STATUS_CODE = ssh2.utils.sftp.STATUS_CODE;

const generateHandle = (): string => uuidv4();

interface TemporaryFile extends FileResult {
virtualPath: string;
}

export class SftpSessionHandler {
private readonly sftpConnection: SFTPWrapper;

Expand All @@ -36,7 +31,7 @@ export class SftpSessionHandler {

private readonly openFilePaths = new Map<string, string>();

private readonly openTemporaryFiles = new Map<string, TemporaryFile>();
private readonly openWriteFiles = new Map<string, TemporaryFile>();

private readonly permanentFileSystemManager: PermanentFileSystemManager;

Expand Down Expand Up @@ -238,9 +233,9 @@ export class SftpSessionHandler {
'Request Data:',
{ reqId, data },
);
const temporaryFile = this.openTemporaryFiles.get(handle.toString());
if (!temporaryFile) {
logger.debug('There is no open temporary file associated with this handle', { reqId, handle });
const writeFile = this.openWriteFiles.get(handle.toString());
if (!writeFile) {
logger.debug('There is no open file associated with this handle', { reqId, handle });
logger.verbose(
'Response: Status (FAILURE)',
{
Expand All @@ -251,36 +246,31 @@ export class SftpSessionHandler {
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
return;
}
fs.write(
temporaryFile.fd,
data,
0,
(err, written, buffer) => {
if (err) {
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: temporaryFile.virtualPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
return;
}
logger.debug('Successful Write.', { reqId, handle, written });
logger.silly('Written Data:', { buffer });
writeFile.append(data)
.then(() => {
logger.debug('Successful Write.', { reqId, handle });
logger.silly('Written Data:', { data });
logger.verbose(
'Response: Status (OK)',
{
reqId,
code: SFTP_STATUS_CODE.OK,
path: temporaryFile.virtualPath,
path: writeFile.permanentFileSystemPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.OK);
},
);
})
.catch(() => {
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: writeFile.permanentFileSystemPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
});
}

/**
Expand Down Expand Up @@ -348,54 +338,37 @@ export class SftpSessionHandler {
handle: handle.toString(),
},
);
const temporaryFile = this.openTemporaryFiles.get(handle.toString());
if (temporaryFile) {
fs.stat(
temporaryFile.name,
(statError, stats) => {
if (statError) {
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: temporaryFile.virtualPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
return;
}
const { size } = stats;
this.getCurrentPermanentFileSystem().createFile(
temporaryFile.virtualPath,
fs.createReadStream(temporaryFile.name),
size,
const writeFile = this.openWriteFiles.get(handle.toString());
if (writeFile) {
writeFile.close()
.then(async () => {
await this.getCurrentPermanentFileSystem().createFile(
writeFile,
).then(() => {
temporaryFile.removeCallback();
this.openTemporaryFiles.delete(handle.toString());
this.openWriteFiles.delete(handle.toString());
logger.verbose(
'Response: Status (OK)',
{
reqId,
code: SFTP_STATUS_CODE.OK,
path: temporaryFile.virtualPath,
path: writeFile.permanentFileSystemPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.OK);
}).catch((err) => {
logger.verbose(err);
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: temporaryFile.virtualPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
});
},
);
})
.catch((e) => {
logger.debug(e);
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: writeFile.permanentFileSystemPath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
});
return;
}
this.openFiles.delete(handle.toString());
Expand Down Expand Up @@ -893,40 +866,38 @@ export class SftpSessionHandler {
case 'xa+': // append and read (file must not exist)
case 'a': // append
{
tmp.file((err, name, fd, removeCallback) => {
if (err) {
const writeFile = new TemporaryFile(filePath);
writeFile.open()
.then(() => {
this.openWriteFiles.set(
handle,
writeFile,
);
logger.verbose(
'Response: Handle',
{
reqId,
handle,
path: filePath,
},
);
this.sftpConnection.handle(
reqId,
Buffer.from(handle),
);
})
.catch((e) => {
logger.debug(e);
logger.verbose(
'Response: Status (FAILURE)',
{
reqId,
code: SFTP_STATUS_CODE.FAILURE,
path: filePath,
},
);
this.sftpConnection.status(reqId, SFTP_STATUS_CODE.FAILURE);
return;
}
const temporaryFile = {
name,
fd,
removeCallback,
};
this.openTemporaryFiles.set(handle, {
...temporaryFile,
virtualPath: filePath,
});
logger.verbose(
'Response: Handle',
{
reqId,
handle,
path: filePath,
},
);
this.sftpConnection.handle(
reqId,
Buffer.from(handle),
);
});
break;
}
case 'r+': // read and write (error if doesn't exist)
Expand Down
Loading

0 comments on commit a71b99f

Please sign in to comment.