Add OBJECTSTORE_S3_SSE_C_KEY environment variable to use S3 SSE-C encryption (server side encryption)#2098
Add OBJECTSTORE_S3_SSE_C_KEY environment variable to use S3 SSE-C encryption (server side encryption)#2098jessebot wants to merge 3 commits intonextcloud:masterfrom
OBJECTSTORE_S3_SSE_C_KEY environment variable to use S3 SSE-C encryption (server side encryption)#2098Conversation
…FIG array Signed-off-by: JesseBot <jessebot@linux.com>
…README.md Signed-off-by: JesseBot <jessebot@linux.com>
|
Hi @jessebot - Thanks for including the matching README update. :-) This variable should get |
| - `OBJECTSTORE_S3_LEGACYAUTH` (default: `false`): Not required for AWS S3 | ||
| - `OBJECTSTORE_S3_OBJECT_PREFIX` (default: `urn:oid:`): Prefix to prepend to the fileid | ||
| - `OBJECTSTORE_S3_AUTOCREATE` (default: `true`): Create the container if it does not exist | ||
| - `OBJECTSTORE_S3_SSE_C_KEY`: Server side encryption key - must be a base64 encoded string with a maximum length of 32 bytes |
There was a problem hiding this comment.
| - `OBJECTSTORE_S3_SSE_C_KEY`: Server side encryption key - must be a base64 encoded string with a maximum length of 32 bytes | |
| - `OBJECTSTORE_S3_SSE_C_KEY`: _Optional_ Server side encryption key - must be a base64 encoded string with a maximum length of 32 bytes |
We might want to include the word optional, so users know it is not required.
There was a problem hiding this comment.
The existing wording in the README is (not set by default) - not _Optional_. Could you please add it to your PR?
No problem :) I can take a look the docker secrets now |
Signed-off-by: jessebot <jessebot@linux.com>
|
@joshtrichards I've added the |
|
do these jobs normally pass? @joshtrichards They look like they're failing on not having the hostname set. I took a quick peak at the github action workflows for these and couldn't find where we set any sort of env vars. Happy to update them if you can point me in the right direction. |
|
please let me know if there's anything I can do to help more this forward. |
pathob
left a comment
There was a problem hiding this comment.
Hi @jessebot , I'm not a maintainer here, but highly interested in this change, so I would like to help moving it forward. I've added two suggestions that might help get this merged. It would be great if you could make these two small adjustments. Thank you!
| if (getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE') && file_exists(getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE'))) { | ||
| $CONFIG['objectstore']['arguments']['sse_c_key'] = trim(file_get_contents(getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE')); | ||
| } elseif (getenv('OBJECTSTORE_S3_SSE_C_KEY')) { | ||
| $CONFIG['objectstore']['arguments']['sse_c_key'] = getenv('OBJECTSTORE_S3_SSE_C_KEY'); | ||
| } | ||
|
|
There was a problem hiding this comment.
IMHO this block should go to the end of the file since OBJECTSTORE_S3_KEY_FILE and OBJECTSTORE_S3_SECRET_FILE have much more relevance than this.
| - `OBJECTSTORE_S3_LEGACYAUTH` (default: `false`): Not required for AWS S3 | ||
| - `OBJECTSTORE_S3_OBJECT_PREFIX` (default: `urn:oid:`): Prefix to prepend to the fileid | ||
| - `OBJECTSTORE_S3_AUTOCREATE` (default: `true`): Create the container if it does not exist | ||
| - `OBJECTSTORE_S3_SSE_C_KEY`: Server side encryption key - must be a base64 encoded string with a maximum length of 32 bytes |
There was a problem hiding this comment.
The existing wording in the README is (not set by default) - not _Optional_. Could you please add it to your PR?
| ); | ||
|
|
||
| if (getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE') && file_exists(getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE'))) { | ||
| $CONFIG['objectstore']['arguments']['sse_c_key'] = trim(file_get_contents(getenv('OBJECTSTORE_S3_SSE_C_KEY_FILE')); |
There was a problem hiding this comment.
I've tried to run it... there is a third closing bracket ) missing at the end
|
closing in favour of #2151 |
Sorry for missing this, but thank you for carrying the torch to the finish line! 🙏 Glad to see the feature implemented :) |
Thanks to everyone who maintains this repo!
I recently learned I could pass in env vars for using S3 as the primary object store, but I was missing the encryption key.
This PR would add an environment variable called
OBJECTSTORE_S3_SSE_C_KEYto the docs and if set, it will be used to provide thesse_c_keyargument to the$CONFIGarray in s3.config.php. If the user doesn't provideOBJECTSTORE_S3_SSE_C_KEY, we don't setss3_c_key, because it has no default.It's mentioned in the docs here:
openssl rand 32 | base64