Skip to content

Added AES encryption/decryption using native crypto #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 44 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
95b5a89
add native AES file encryption
cbaker6 Jun 30, 2020
7ae60e4
removed update of package.json
cbaker6 Jun 30, 2020
e507825
update package.json
cbaker6 Jun 30, 2020
dad1e7c
update node in travis
cbaker6 Jun 30, 2020
d01aa1f
update node in travis
cbaker6 Jun 30, 2020
e6c8f9b
trying different verion of key and iv hash
cbaker6 Jun 30, 2020
f9893de
downgrading node to match minimal parse-server
cbaker6 Jun 30, 2020
74c7c9e
removed commented code
cbaker6 Jun 30, 2020
9af50d3
add random iv for each file instead of using constant. Also removed e…
cbaker6 Jun 30, 2020
db8a1a8
remove unneccesary files
cbaker6 Jun 30, 2020
c9f9db8
Use AES 256 GCM to detect file tampering
cbaker6 Jun 30, 2020
11c6f3c
remove codecov from package.json
cbaker6 Jun 30, 2020
9f4fd11
add repo field to get rid of npm install warning
cbaker6 Jun 30, 2020
fb7517f
Fix options
cbaker6 Jun 30, 2020
92013a4
switch secretKey to fileKey
cbaker6 Jul 1, 2020
6f2752e
switch secretKey to fileKey
cbaker6 Jul 1, 2020
d2fce74
added the ability to rotate fileKeys
cbaker6 Jul 1, 2020
b42d683
add syntax highlighting to readme
cbaker6 Jul 1, 2020
0d25c2c
bump version
cbaker6 Jul 1, 2020
f633f70
attempt to fix coverage
cbaker6 Jul 1, 2020
e07ec58
update testcase title
cbaker6 Jul 1, 2020
7f1784c
clean up unused vars
cbaker6 Jul 2, 2020
8939921
add directions for multiple instances of parse-server
cbaker6 Jul 2, 2020
75d6b8a
update readme
cbaker6 Jul 2, 2020
9ac405f
update file names in readme
cbaker6 Jul 2, 2020
0c4bf0d
add testcase for rotating key from oldKey to noKey leaving all files …
cbaker6 Jul 2, 2020
47c87e7
Add notice about previous versions of parse-server
cbaker6 Jul 2, 2020
07061b7
Update README.md
cbaker6 Jul 2, 2020
bd19045
Update README.md
cbaker6 Jul 2, 2020
51c2b51
Update README.md
cbaker6 Jul 2, 2020
01fdf22
Update README.md
cbaker6 Jul 2, 2020
4517087
make createFile and getFile use streams instead of putting whole file…
cbaker6 Jul 4, 2020
b68681c
Merge branch 'master' of https://github.com/netreconlab/parse-server-…
cbaker6 Jul 4, 2020
9646246
don't read file into memory while deleting
cbaker6 Jul 4, 2020
39f96d2
clean up code
cbaker6 Jul 4, 2020
fbcabf4
make more consistant with GridFS adapter
cbaker6 Jul 4, 2020
1d3ca24
fixed formatting
cbaker6 Jul 4, 2020
4c344ee
Update .travis.yml
cbaker6 Jul 5, 2020
a20a7a2
Remove unnecessary testcase
cbaker6 Jul 5, 2020
b649dde
Update secureFiles.spec.js
cbaker6 Jul 5, 2020
2390e59
add directions for dev server to readme
cbaker6 Oct 21, 2020
6b175fb
Revert version
cbaker6 Oct 21, 2020
5beb618
Update package.json
cbaker6 Oct 21, 2020
490dc75
Update .travis.yml
cbaker6 Oct 21, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .nycrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"reporter": [
"lcov",
"text-summary"
],
"exclude": [
"**/spec/**"
]
}
8 changes: 6 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,9 @@ branches:
- master
language: node_js
node_js:
- "4.3"
after_success: ./node_modules/.bin/codecov
- 8
env:
global:
- COVERAGE_OPTION='./node_modules/.bin/nyc'
script: npm run coverage
after_success: bash <(curl -s https://codecov.io/bash)
66 changes: 57 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,92 @@
[![Build Status](https://travis-ci.org/parse-community/parse-server-fs-adapter.svg?branch=master)](https://travis-ci.org/parse-community/parse-server-fs-adapter)
[![codecov.io](https://codecov.io/github/parse-community/parse-server-fs-adapter/coverage.svg?branch=master)](https://codecov.io/github/parse-community/parse-server-fs-adapter?branch=master)

parse-server file system storage adapter
parse-server file system storage adapter.


# installation
# Multiple instances of parse-server
When using parse-server-fs-adapter across multiple parse-server instances it's important to establish "centralization" of your file storage (this is the same premise as the other file adapters, you are sending/recieving files through a dedicated link). You can accomplish this at the file storage level by Samba mounting (or any other type of mounting) your storage to each of your parse-server instances, e.g if you are using parse-server via docker (volume mount your SMB drive to `- /Volumes/SMB-Drive/MyParseApp1/files:/parse-server/files`). All parse-server instances need to be able to read and write to the same storage in order for parse-server-fs-adapter to work properly with parse-server. If the file storage isn't centralized, parse-server will have trouble locating files and you will get random behavior on client-side.

# Installation

`npm install --save @parse/fs-files-adapter`

# usage with parse-server
# Usage with parse-server

### using a config file
### Using a config file

```
```javascript
{
"appId": 'my_app_id',
"masterKey": 'my_master_key',
// other options
"filesAdapter": {
"module": "@parse/fs-files-adapter",
"options": {
"filesSubDirectory": "my/files/folder" // optional
"filesSubDirectory": "my/files/folder", // optional
"fileKey": "someKey" //optional, but mandatory if you want to encrypt files
}
}
}
```

### passing as an instance
### Passing as an instance
***Notice: If used with parse-server versions <= 4.2.0, DO NOT PASS in `PARSE_SERVER_FILE_KEY` or `fileKey` from parse-server. Instead pass your key directly to `FSFilesAdapter` using your own environment variable or hardcoding the string. parse-server versions > 4.2.0 can pass in `PARSE_SERVER_FILE_KEY` or `fileKey`.***

```javascript
var FSFilesAdapter = require('@parse/fs-files-adapter');

var fsAdapter = new FSFilesAdapter({
"filesSubDirectory": "my/files/folder", // optional
"fileKey": "someKey" //optional, but mandatory if you want to encrypt files
});

var api = new ParseServer({
appId: 'my_app',
masterKey: 'master_key',
filesAdapter: fsAdapter
})
```

### Rotating to a new fileKey
Periodically you may want to rotate your fileKey for security reasons. When this is the case. Initialize the file adapter with the new key and do the following in your `index.js`:

#### Files were previously unencrypted and you want to encrypt
```javascript
var FSFilesAdapter = require('@parse/fs-files-adapter');

var fsAdapter = new FSFilesAdapter({
"filesSubDirectory": "my/files/folder" // optional
});
"filesSubDirectory": "my/files/folder", // optional
"fileKey": "newKey" //Use the newKey
});

var api = new ParseServer({
appId: 'my_app',
masterKey: 'master_key',
filesAdapter: fsAdapter
})

//This can take awhile depending on how many files and how larger they are. It will attempt to rotate the key of all files in your filesSubDirectory
const {rotated, notRotated} = await api.filesAdapter.rotateFileKey();
console.log('Files rotated to newKey: ' + rotated);
console.log('Files that couldn't be rotated to newKey: ' + notRotated);
```


#### Files were previously encrypted with `oldKey` and you want to encrypt with `newKey`
The same process as above, but pass in your `oldKey` to `rotateFileKey()`.
```javascript
//This can take awhile depending on how many files and how larger they are. It will attempt to rotate the key of all files in your filesSubDirectory
const {rotated, notRotated} = await api.filesAdapter.rotateFileKey({oldKey: oldKey});
console.log('Files rotated to newKey: ' + rotated);
console.log('Files that couldn't be rotated to newKey: ' + notRotated);
```

#### Only rotate a select list of files that were previously encrypted with `oldKey` and you want to encrypt with `newKey`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you only rotate a subset of the files, how will the not rotated ones be served?

Copy link
Contributor Author

@cbaker6 cbaker6 Jul 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, they can't be served with the current key. This is additional functionality for people who have custom setups or if the server crashes/has issues during rotation and you need to rotate the rest of the files.

In common cases, admins should store files for respective apps in individual folders, and rotate all when needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that rotating the keys like this can generate a lot of problems in the case there is a large amount of files in the storage. I'd prefer to either have this rotate functionality on a separate module/project or think in some way to have both the old and the new key serving old and new files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand, how does encrypting keys with this module, but having the key rotation on a different module/project help? I think if you encrypt files, that same module should always have a way to decrypt them and leave unencrypted if wanted, which will probably be the main use of the rotate functionality.

Server admins can come up with their own creative ways of maintaining their key, like encrypting the fileKey with a separate key and then rotating the separate key instead of rotating the fileKey encrypting all of the files. They have to maintain two keys, but gets around the large amount of files issues (I think AWS does something similar to this). I'm sure others can come up with a ton of ways, but the ability to decrypt should always be apart of the same module IMO.

Parse-server admins determine when to rotate keys. Another way is they could create create a separate dev parse-server, rotate the keys of a copy of their files directory and then point their production server to the new folder.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this rotate function shouldn't be part of the scope of the adapter and that's why I suggested either a separate project or a separate script. You are suggesting people to use the rotate function in the Parse Server initialization which can generate a lot of problems in the case there is a large amount of files. Take the example below:

var FSFilesAdapter = require('@parse/fs-files-adapter');
var fsAdapter = new FSFilesAdapter({	var fsAdapter = new FSFilesAdapter({
      "filesSubDirectory": "my/files/folder" // optional	  "filesSubDirectory": "my/files/folder", // optional
    });	  "fileKey": "newKey" //Use the newKey
});
var api = new ParseServer({	var api = new ParseServer({
	appId: 'my_app',		appId: 'my_app',
	masterKey: 'master_key',		masterKey: 'master_key',
	filesAdapter: fsAdapter		filesAdapter: fsAdapter
})	})
//This can take awhile depending on how many files and how larger they are. It will attempt to rotate the key of all files in your filesSubDirectory
const {rotated, notRotated} =  await api.filesAdapter.rotateFileKey();
console.log('Files rotated to newKey: ' + rotated);
console.log('Files that couldn't be rotated to newKey: ' + notRotated);
  • Now Imagine that I am running Parse Server in multiple containers. I will have multiple processed running the rotation at the same time.
  • The rotation will consume cpu and memory in these processes which can cause poor performance while it is running.
  • The files not yet rotated will not be correctly served

I still believe that the rotate function is useful though, but we should either solve all these problems (which will be hard to be done) or not suggest it to be used this way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dplewis @mtrezza can you both weigh in on this?

The difference in opinion between @davimacedo and I are expressed on the thread and I think there are pros/cons to both approaches. I don’t believe the differences are a showstopper for this PR and I think improvements can be made by others (if I see an improvement, I will make it as well).

On a related note, @davimacedo @dplewis @mtrezza I know there has been recent/past discussions about the parse-server allowing access to all files if you guess the link (which isn’t necessarily easy, but doable). I’m going off memory, but I believe the server has something like getHostFile (I’ll look this up when I get to my computer). I don’t know entirely how this works, but I assume it gets the file whenever someone requests the link and calls the fileAdapters underlying methods to retrieve? If this is the case (or something similar), is possible to use the fileKey to either serve an encrypted or unencrypted file based on the user access (I’m assuming some file always to be present at the link to stay inline with older versions which is why I mention the encrypted file)? I definitely haven’t investigated if this is the easiest and best way, but I wanted to start the convo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davimacedo I know it’s been awhile since we discussed this. If I recall, you didn’t believe the key rotation should be in the adapter itself. I approached from the direction if we provide a way to encrypt, we should provide a way to decrypt (with this adapter as well as the grid adapter on parse-server). Are we able to come to some type of agreement on this? This PR along with the parse-server have been around for a few months and I’m sure my JS skills are not as good as they were when I made the PR’s.

I’m not attempting to force these in, but my assumption is people enable file encryption here will understand what they are doing and take the necessary precautions when rotating keys. People that use the S3 adapter have this benefit because AWS handles it for them https://docs.aws.amazon.com/kms/latest/developerguide/rotate-keys.html

I want to also state that file encryption isn’t enabled by default, so I’m sure many will just be using the refreshed code

Copy link
Member

@davimacedo davimacedo Oct 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I told you before, you did a great job here, but the only thing I don't like in this PR is when you suggest something like this:

var FSFilesAdapter = require('@parse/fs-files-adapter');

var fsAdapter = new FSFilesAdapter({
  "filesSubDirectory": "my/files/folder", // optional
  "fileKey": "newKey" //Use the newKey
});

var api = new ParseServer({
	appId: 'my_app',
	masterKey: 'master_key',
	filesAdapter: fsAdapter
})

//This can take awhile depending on how many files and how larger they are. It will attempt to rotate the key of all files in your filesSubDirectory
const {rotated, notRotated} =  await api.filesAdapter.rotateFileKey();
console.log('Files rotated to newKey: ' + rotated);
console.log('Files that couldn't be rotated to newKey: ' + notRotated);

In my vision, the developers should never use the same process they are using for running Parse Server to also rotate the files. That's what this example is suggesting and I don't see it ending well unless the app has just few files.

I believe we can even maintain the key rotation here but it would be more clear and actually more convenient for the developers if it worked via a script that they could run something like:

parse-server-fs-adapter appId masterKey path oldKey newKey

If it is too much effort to be done, I believe we should at least move the rotate method from the adapter to another class in which you would pass appId, masterKey, path, oldKey, newKey (and not an instance of Parse Server) and make it clear in the readme that this operation is not intended to be executed in the same process of Parse Server.

The other maintainers may have a different vision though. @dplewis @mtrezza thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davimacedo I guess my thinking was similar to your script example, with the belief that the code you mentioned (key rotation code) above would be executed on a development server (not the live server). So the development server is spun up with the newKey, then keys are rotated. If there are additional tests to be done on dev parse-server they can be done as well. After completion, the live server will need to be restarted with the newKey.

Would it help to change my examples to recommend doing key rotation on a dev server? Then mention the part about restarting the live server?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I believe it helps. Anything that makes clear to the developers that they should not run the rotate key in the same process they are running parse server.

This is useful if for some reason there errors and some of the files werent rotated and returned in `notRotated`. The same process as above, but pass in your `oldKey` along with the array of `fileNames` to `rotateFileKey()`.
```javascript
//This can take awhile depending on how many files and how larger they are. It will attempt to rotate the key of all files in your filesSubDirectory
const {rotated, notRotated} = await api.filesAdapter.rotateFileKey({oldKey: oldKey, fileNames: ["fileName1.png","fileName2.png"]});
console.log('Files rotated to newKey: ' + rotated);
console.log('Files that couldn't be rotated to newKey: ' + notRotated);
```
150 changes: 130 additions & 20 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,16 @@
var fs = require('fs');
var path = require('path');
var pathSep = require('path').sep;
const crypto = require("crypto");
const algorithm = 'aes-256-gcm';

function FileSystemAdapter(options) {
options = options || {};
this._fileKey = null;

if (options.fileKey !== undefined){
this._fileKey = crypto.createHash('sha256').update(String(options.fileKey)).digest('base64').substr(0, 32);
}
let filesSubDirectory = options.filesSubDirectory || '';
this._filesDir = filesSubDirectory;
this._mkdir(this._getApplicationDir());
Expand All @@ -19,44 +26,147 @@ function FileSystemAdapter(options) {
}

FileSystemAdapter.prototype.createFile = function(filename, data) {
let filepath = this._getLocalFilePath(filename);
const stream = fs.createWriteStream(filepath);
return new Promise((resolve, reject) => {
let filepath = this._getLocalFilePath(filename);
fs.writeFile(filepath, data, (err) => {
if(err !== null) {
return reject(err);
}
resolve(data);
});
try{
if(this._fileKey !== null){
const iv = crypto.randomBytes(16);
const cipher = crypto.createCipheriv(
algorithm,
this._fileKey,
iv
);
const encryptedResult = Buffer.concat([
cipher.update(data),
cipher.final(),
iv,
cipher.getAuthTag(),
]);
stream.write(encryptedResult);
stream.end();
stream.on('finish', function() {
resolve(data);
});
}else{
stream.write(data);
stream.end();
stream.on('finish', function() {
resolve(data);
});
}
}catch(err){
return reject(err);
}
});
}

FileSystemAdapter.prototype.deleteFile = function(filename) {
let filepath = this._getLocalFilePath(filename);
const chunks = [];
const stream = fs.createReadStream(filepath);
return new Promise((resolve, reject) => {
let filepath = this._getLocalFilePath(filename);
fs.readFile( filepath , function (err, data) {
if(err !== null) {
return reject(err);
}
fs.unlink(filepath, (unlinkErr) => {
if(err !== null) {
return reject(unlinkErr);
stream.read();
stream.on('data', (data) => {
chunks.push(data);
});
stream.on('end', () => {
const data = Buffer.concat(chunks);
fs.unlink(filepath, (err) => {
if(err !== null) {
return reject(err);
}
resolve(data);
});
});

stream.on('error', (err) => {
reject(err);
});
});
}

FileSystemAdapter.prototype.getFileData = function(filename) {
let filepath = this._getLocalFilePath(filename);
const stream = fs.createReadStream(filepath);
stream.read();
return new Promise((resolve, reject) => {
let filepath = this._getLocalFilePath(filename);
fs.readFile( filepath , function (err, data) {
if(err !== null) {
return reject(err);
const chunks = [];
stream.on('data', (data) => {
chunks.push(data);
});
stream.on('end', () => {
const data = Buffer.concat(chunks);
if(this._fileKey !== null){
const authTagLocation = data.length - 16;
const ivLocation = data.length - 32;
const authTag = data.slice(authTagLocation);
const iv = data.slice(ivLocation,authTagLocation);
const encrypted = data.slice(0,ivLocation);
try{
const decipher = crypto.createDecipheriv(algorithm, this._fileKey, iv);
decipher.setAuthTag(authTag);
const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]);
return resolve(decrypted);
}catch(err){
return reject(err);
}
}
resolve(data);
});
stream.on('error', (err) => {
reject(err);
});
});
}

FileSystemAdapter.prototype.rotateFileKey = function(options = {}) {
const applicationDir = this._getApplicationDir();
var fileNames = [];
var oldKeyFileAdapter = {};
if (options.oldKey !== undefined) {
oldKeyFileAdapter = new FileSystemAdapter({filesSubDirectory: this._filesDir, fileKey: options.oldKey});
}else{
oldKeyFileAdapter = new FileSystemAdapter({filesSubDirectory: this._filesDir});
}
if (options.fileNames !== undefined){
fileNames = options.fileNames;
}else{
fileNames = fs.readdirSync(applicationDir);
fileNames = fileNames.filter(fileName => fileName.indexOf('.') !== 0);
}
return new Promise((resolve, _reject) => {
var fileNamesNotRotated = fileNames;
var fileNamesRotated = [];
var fileNameTotal = fileNames.length;
var fileNameIndex = 0;
fileNames.forEach(fileName => {
oldKeyFileAdapter
.getFileData(fileName)
.then(plainTextData => {
//Overwrite file with data encrypted with new key
this.createFile(fileName, plainTextData)
.then(() => {
fileNamesRotated.push(fileName);
fileNamesNotRotated = fileNamesNotRotated.filter(function(value){ return value !== fileName;})
fileNameIndex += 1;
if (fileNameIndex == fileNameTotal){
resolve({rotated: fileNamesRotated, notRotated: fileNamesNotRotated});
}
})
.catch(() => {
fileNameIndex += 1;
if (fileNameIndex == fileNameTotal){
resolve({rotated: fileNamesRotated, notRotated: fileNamesNotRotated});
}
})
})
.catch(() => {
fileNameIndex += 1;
if (fileNameIndex == fileNameTotal){
resolve({rotated: fileNamesRotated, notRotated: fileNamesNotRotated});
}
});
});
});
}

Expand Down
14 changes: 9 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
{
"name": "@parse/fs-files-adapter",
"version": "1.0.1",
"version": "1.0.2",
"description": "File system adapter for parse-server",
"main": "index.js",
"repository": {
"type": "git",
"url": "https://github.com/parse-community/parse-server-fs-adapter"
},
"scripts": {
"test": "istanbul cover -x **/spec/** jasmine --captureExceptions"
"test": "jasmine",
"coverage": "nyc jasmine"
},
"keywords": [
"parse-server",
Expand All @@ -15,9 +20,8 @@
"author": "Parse",
"license": "MIT",
"devDependencies": {
"codecov": "^1.0.1",
"istanbul": "^0.4.2",
"jasmine": "^2.4.1",
"nyc": "^15.1.0",
"jasmine": "^3.5.0",
"parse-server-conformance-tests": "^1.0.0"
}
}
Loading