Skip to content

Commit

Permalink
Remove ftp functionality (#2064)
Browse files Browse the repository at this point in the history
* Remove ftp functionality

https://eaflood.atlassian.net/browse/IWTF-4278

Remove ftp functionality as package we use in fulfilment and pocl jobs (ssh2-sftp-client) has critical vulnerability

* remove ssh2 sftp client

* remove ssh2 sftp client and fix lint

* remove reference

* fix tests

* fix tests and lint

* update tests

* refactor

* refactor

* update tests

* refactor tests

* s3spec

* undefined token

* rename file and undo removal of stores3

* rename file and undo removal of stores3

* add missing mock

* Remove ssh2 mock

* remove functionality from config

* remove ftp reference

---------
  • Loading branch information
ScottDormand96 authored Nov 25, 2024
1 parent 621875e commit 42f1d84
Show file tree
Hide file tree
Showing 21 changed files with 111 additions and 1,363 deletions.
385 changes: 8 additions & 377 deletions packages/fulfilment-job/package-lock.json

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions packages/fulfilment-job/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
"merge2": "^1.4.1",
"moment": "^2.29.1",
"openpgp": "^5.0.0-1",
"pluralize": "^8.0.0",
"ssh2-sftp-client": "^6.0.1"
"pluralize": "^8.0.0"
}
}
9 changes: 0 additions & 9 deletions packages/fulfilment-job/src/__mocks__/ssh2-sftp-client.js

This file was deleted.

33 changes: 1 addition & 32 deletions packages/fulfilment-job/src/__tests__/config.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ const clearEnvVars = () => {

const envVars = Object.freeze({
FULFILMENT_FILE_SIZE: 1234,
FULFILMENT_FTP_HOST: 'test-host',
FULFILMENT_FTP_PORT: 2222,
FULFILMENT_FTP_PATH: '/remote/share',
FULFILMENT_FTP_USERNAME: 'test-user',
FULFILMENT_FTP_KEY_SECRET_ID: 'test-secret-id',
FULFILMENT_S3_BUCKET: 'test-bucket',
FULFILMENT_PGP_PUBLIC_KEY_SECRET_ID: 'pgp-key-secret-id',
FULFILMENT_SEND_UNENCRYPTED_FILE: 'false'
Expand All @@ -44,32 +39,6 @@ describe('config', () => {
})
})

describe('ftp', () => {
it('provides properties relating the use of SFTP', async () => {
expect(config.ftp).toEqual(
expect.objectContaining({
host: 'test-host',
port: '2222',
path: '/remote/share',
username: 'test-user',
privateKey: 'test-ssh-key',
algorithms: { cipher: expect.any(Array), kex: expect.any(Array) },
// Wait up to 60 seconds for the SSH handshake
readyTimeout: expect.any(Number),
// Retry 5 times over a minute
retries: expect.any(Number),
retry_minTimeout: expect.any(Number),
debug: expect.any(Function)
})
)
})
it('defaults the sftp port to 22 if the environment variable is not configured', async () => {
delete process.env.FULFILMENT_FTP_PORT
await config.initialise()
expect(config.ftp.port).toEqual('22')
})
})

describe('s3', () => {
it('provides properties relating the use of Amazon S3', async () => {
expect(config.s3.bucket).toEqual('test-bucket')
Expand All @@ -79,7 +48,7 @@ describe('config', () => {

describe('pgp config', () => {
const init = async (samplePublicKey = 'sample-pgp-key') => {
AwsMock.SecretsManager.__setNextResponses('getSecretValue', { SecretString: 'test-ssh-key' }, { SecretString: samplePublicKey })
AwsMock.SecretsManager.__setNextResponses('getSecretValue', { SecretString: samplePublicKey })
await config.initialise()
}
beforeAll(setEnvVars)
Expand Down
68 changes: 1 addition & 67 deletions packages/fulfilment-job/src/config.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,6 @@
import { AWS } from '@defra-fish/connectors-lib'
import db from 'debug'
const { secretsManager } = AWS()

/**
* Key exchange algorithms for public key authentication - in descending order of priority
* @type {string[]}
*/
export const SFTP_KEY_EXCHANGE_ALGORITHMS = [
'[email protected]',
'curve25519-sha256',
'ecdh-sha2-nistp521',
'ecdh-sha2-nistp384',
'ecdh-sha2-nistp256',
'diffie-hellman-group-exchange-sha256',
'diffie-hellman-group14-sha256',
'diffie-hellman-group16-sha512',
'diffie-hellman-group18-sha512',
'diffie-hellman-group14-sha1',
'diffie-hellman-group-exchange-sha1',
'diffie-hellman-group1-sha1'
]
/**
* Ciphers for SFTP support - in descending order of priority
* @type {string[]}
*/
export const SFTP_CIPHERS = [
// http://tools.ietf.org/html/rfc4344#section-4
'aes256-ctr',
'aes192-ctr',
'aes128-ctr',
'aes256-gcm',
'[email protected]',
'aes128-gcm',
'[email protected]',
'aes256-cbc',
'aes192-cbc',
'aes128-cbc',
'blowfish-cbc',
'3des-cbc',
'cast128-cbc'
]
const { secretsManager } = AWS()
const falseRegEx = /(false|0)/i
const trueRegEx = /(true|1)/i
const toBoolean = val => {
Expand All @@ -54,7 +15,6 @@ const toBoolean = val => {

class Config {
_file
_ftp
_s3
_pgp

Expand All @@ -68,20 +28,6 @@ class Config {
*/
partFileSize: Math.min(Number.parseInt(process.env.FULFILMENT_FILE_SIZE), 999)
}
this.ftp = {
host: process.env.FULFILMENT_FTP_HOST,
port: process.env.FULFILMENT_FTP_PORT || '22',
path: process.env.FULFILMENT_FTP_PATH,
username: process.env.FULFILMENT_FTP_USERNAME,
privateKey: (await secretsManager.getSecretValue({ SecretId: process.env.FULFILMENT_FTP_KEY_SECRET_ID }).promise()).SecretString,
algorithms: { cipher: SFTP_CIPHERS, kex: SFTP_KEY_EXCHANGE_ALGORITHMS },
// Wait up to 60 seconds for the SSH handshake
readyTimeout: 60000,
// Retry 5 times over a minute
retries: 5,
retry_minTimeout: 12000,
debug: db('fulfilment:ftp')
}
this.s3 = {
bucket: process.env.FULFILMENT_S3_BUCKET
}
Expand All @@ -104,18 +50,6 @@ class Config {
this._file = cfg
}

/**
* FTP configuration settings
* @type {object}
*/
get ftp () {
return this._ftp
}

set ftp (cfg) {
this._ftp = cfg
}

/**
* S3 configuration settings
* @type {object}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Readable, PassThrough, Writable } from 'stream'
import { deliverFulfilmentFiles } from '../deliver-fulfilment-files.js'
import { createS3WriteStream, readS3PartFiles } from '../../transport/s3.js'
import { createFtpWriteStream } from '../../transport/ftp.js'
import { FULFILMENT_FILE_STATUS_OPTIONSET, getOptionSetEntry } from '../staging-common.js'
import { FulfilmentRequestFile, executeQuery, persist } from '@defra-fish/dynamics-lib'
import openpgp from 'openpgp'
Expand All @@ -10,7 +9,6 @@ import streamHelper from '../streamHelper.js'
import merge2 from 'merge2'

jest.mock('../../transport/s3.js')
jest.mock('../../transport/ftp.js')
jest.mock('openpgp', () => ({
readKey: jest.fn(() => ({})),
encrypt: jest.fn(({ message: readableStream }) => readableStream),
Expand Down Expand Up @@ -46,20 +44,10 @@ describe('deliverFulfilmentFiles', () => {
executeQuery.mockResolvedValue([{ entity: mockFulfilmentRequestFile2 }, { entity: mockFulfilmentRequestFile1 }])

// Streams for file1
const {
s3DataStreamFile: s3DataStreamFile1,
ftpDataStreamFile: ftpDataStreamFile1,
s3HashStreamFile: s3HashStreamFile1,
ftpHashStreamFile: ftpHashStreamFile1
} = createMockFileStreams()
const { s3DataStreamFile: s3DataStreamFile1, s3HashStreamFile: s3HashStreamFile1 } = createMockFileStreams()

// Streams for file2
const {
s3DataStreamFile: s3DataStreamFile2,
ftpDataStreamFile: ftpDataStreamFile2,
s3HashStreamFile: s3HashStreamFile2,
ftpHashStreamFile: ftpHashStreamFile2
} = createMockFileStreams()
const { s3DataStreamFile: s3DataStreamFile2, s3HashStreamFile: s3HashStreamFile2 } = createMockFileStreams()

// Run the delivery
await expect(deliverFulfilmentFiles()).resolves.toBeUndefined()
Expand All @@ -69,22 +57,14 @@ describe('deliverFulfilmentFiles', () => {
// File 1 expectations
expect(createS3WriteStream).toHaveBeenNthCalledWith(1, 'EAFF202006180001.json')
expect(createS3WriteStream).toHaveBeenNthCalledWith(3, 'EAFF202006180001.json.sha256')
expect(createFtpWriteStream).toHaveBeenNthCalledWith(1, 'EAFF202006180001.json')
expect(createFtpWriteStream).toHaveBeenNthCalledWith(3, 'EAFF202006180001.json.sha256')
expect(JSON.parse(s3DataStreamFile1.dataProcessed)).toEqual({ licences: [{ part: 0 }, { part: 1 }] })
expect(JSON.parse(ftpDataStreamFile1.dataProcessed)).toEqual({ licences: [{ part: 0 }, { part: 1 }] })
expect(s3HashStreamFile1.dataProcessed).toEqual(fileShaHash) // validated
expect(ftpHashStreamFile1.dataProcessed).toEqual(fileShaHash) // validated

// File 2 expectations
expect(createS3WriteStream).toHaveBeenNthCalledWith(4, 'EAFF202006180002.json')
expect(createS3WriteStream).toHaveBeenNthCalledWith(6, 'EAFF202006180002.json.sha256')
expect(createFtpWriteStream).toHaveBeenNthCalledWith(4, 'EAFF202006180002.json')
expect(createFtpWriteStream).toHaveBeenNthCalledWith(6, 'EAFF202006180002.json.sha256')
expect(JSON.parse(s3DataStreamFile2.dataProcessed)).toEqual({ licences: [{ part: 0 }, { part: 1 }] })
expect(JSON.parse(ftpDataStreamFile2.dataProcessed)).toEqual({ licences: [{ part: 0 }, { part: 1 }] })
expect(s3HashStreamFile2.dataProcessed).toEqual(fileShaHash) // validated
expect(ftpHashStreamFile2.dataProcessed).toEqual(fileShaHash) // validated

// Persist to dynamics for file 1
expect(persist).toHaveBeenNthCalledWith(1, [
Expand Down Expand Up @@ -197,10 +177,7 @@ describe('deliverFulfilmentFiles', () => {
const s3 = createTestableStream()
streamHelper.pipelinePromise.mockResolvedValue()
openpgp.encrypt.mockResolvedValue(s2)
merge2
.mockReturnValueOnce(s1)
.mockReturnValueOnce(s2)
.mockReturnValueOnce(s3)
merge2.mockReturnValueOnce(s1).mockReturnValueOnce(s2).mockReturnValueOnce(s3)
await mockExecuteQuery()
createMockFileStreams()

Expand All @@ -227,27 +204,18 @@ const createMockFulfilmentRequestFile = async (fileName, date) =>

const createMockFileStreams = () => {
const s3DataStreamFile = createTestableStream()
const ftpDataStreamFile = createTestableStream()
createS3WriteStream.mockReturnValueOnce({ s3WriteStream: s3DataStreamFile, managedUpload: Promise.resolve() })
createFtpWriteStream.mockReturnValueOnce({ ftpWriteStream: ftpDataStreamFile, managedUpload: Promise.resolve() })

const s3EncryptedDataStreamFile = createTestableStream()
const ftpEncryptedDataStreamFile = createTestableStream()
createS3WriteStream.mockReturnValueOnce({ s3WriteStream: s3EncryptedDataStreamFile, managedUpload: Promise.resolve() })
createFtpWriteStream.mockReturnValueOnce({ ftpWriteStream: ftpEncryptedDataStreamFile, managedUpload: Promise.resolve() })

const s3HashStreamFile = createTestableStream()
const ftpHashStreamFile = createTestableStream()
createS3WriteStream.mockReturnValueOnce({ s3WriteStream: s3HashStreamFile, managedUpload: Promise.resolve() })
createFtpWriteStream.mockReturnValueOnce({ ftpWriteStream: ftpHashStreamFile, managedUpload: Promise.resolve() })

return {
s3DataStreamFile,
ftpDataStreamFile,
s3EncryptedDataStreamFile,
ftpEncryptedDataStreamFile,
s3HashStreamFile,
ftpHashStreamFile
s3HashStreamFile
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import merge2 from 'merge2'
import moment from 'moment'
import { executeQuery, persist, findFulfilmentFiles } from '@defra-fish/dynamics-lib'
import { createS3WriteStream, readS3PartFiles } from '../transport/s3.js'
import { createFtpWriteStream } from '../transport/ftp.js'
import { FULFILMENT_FILE_STATUS_OPTIONSET, getOptionSetEntry } from './staging-common.js'
import db from 'debug'
import openpgp from 'openpgp'
Expand Down Expand Up @@ -69,11 +68,6 @@ const createEncryptedDataReadStream = async file => {
*/
const deliver = async (targetFileName, readableStream, ...transforms) => {
const { s3WriteStream: s3DataStream, managedUpload: s3DataManagedUpload } = createS3WriteStream(targetFileName)
const { ftpWriteStream: ftpDataStream, managedUpload: ftpDataManagedUpload } = createFtpWriteStream(targetFileName)

await Promise.all([
streamHelper.pipelinePromise([readableStream, ...transforms, s3DataStream, ftpDataStream]),
s3DataManagedUpload,
ftpDataManagedUpload
])
await Promise.all([streamHelper.pipelinePromise([readableStream, ...transforms, s3DataStream]), s3DataManagedUpload])
}
63 changes: 0 additions & 63 deletions packages/fulfilment-job/src/transport/__tests__/ftp.spec.js

This file was deleted.

28 changes: 0 additions & 28 deletions packages/fulfilment-job/src/transport/ftp.js

This file was deleted.

Loading

0 comments on commit 42f1d84

Please sign in to comment.