Skip to content
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

Enhance S3 to support EC2 instance roles #19790

Closed
pcolmer opened this issue Mar 5, 2020 · 13 comments · Fixed by #20891 or #22188
Closed

Enhance S3 to support EC2 instance roles #19790

pcolmer opened this issue Mar 5, 2020 · 13 comments · Fixed by #20891 or #22188
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: object storage

Comments

@pcolmer
Copy link

pcolmer commented Mar 5, 2020

Is your feature request related to a problem? Please describe.
At the moment, when you configure NC to support S3 as primary object storage, you need to specify the access key ID and secret access key. This, in turn, requires the creation of a user on IAM. The drawback to this is that the keys get stale over time which becomes a security vulnerability.

Describe the solution you'd like
If NC is running on an AWS EC2 instance, it should be possible to use the instance role to gain the appropriate credentials. The code could try to use this mechanism automatically if config.php is missing the keys.

https://docs.aws.amazon.com/aws-sdk-php/v2/guide/credentials.html#instance-profile-credentials

Describe alternatives you've considered
Creating a user and then manually rotating the keys regularly but this requires the config file to get updated and NC to be restarted.

@pcolmer pcolmer added 0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement labels Mar 5, 2020
@kesselb
Copy link
Contributor

kesselb commented Mar 5, 2020

<?php

require_once '3rdparty/autoload.php';

use Aws\Credentials\CredentialProvider;
use Aws\Credentials\CredentialsInterface;

/** @var callable $credentialsProvider */
$credentialsProvider = CredentialProvider::defaultProvider();
/** @var CredentialsInterface $credentials */
$credentials = $credentialsProvider()->wait();

$CONFIG = [
	'objectstore' => array(
		'class' => \OC\Files\ObjectStore\S3::class,
		'arguments' => array(
			'bucket' => 'nextcloud',
			'autocreate' => true,
			'key' => $credentials->getAccessKeyId(),
			'secret' => $credentials->getSecretKey(),
			'hostname' => 'localhost',
			'port' => 9600,
			'use_ssl' => false,
			'region' => 'us-east-1',
			// required for some non Amazon S3 implementations
			'use_path_style' => true
		),
	),
];

Using above file as config/objectstore.config.php should work. It's a bit hacky because we need to load the autoloader for external libraries but then we can fetch the credentials from the identity provider. I have not tested it with a ec2 instance but the code is called at the right time ;)

@kesselb
Copy link
Contributor

kesselb commented Mar 5, 2020

But a flag like 'useCredentialProvider' => true, might be easier.

@pcolmer
Copy link
Author

pcolmer commented Mar 5, 2020

@kesselb thank you very much for providing that config snippet.

Apart from putting that file into the config directory, do I need to do anything to tell NC to use it? I've commented out the objectstore block in config/config.php but now NC gives me errors when I try to access the main page:

{
  "reqId": "yEwiJelsN5HFBiHo57rf",
  "level": 3,
  "time": "2020-03-05T13:44:57+00:00",
  "remoteAddr": "81.128.185.34",
  "user": "ocadmin",
  "app": "core",
  "method": "GET",
  "url": "/nextcloud/index.php/apps/files/",
  "message": {
    "Exception": "OCP\\Files\\NotPermittedException",
    "Message": "Could not create path",
    "Code": 0,
    "Trace": [
      {
        "file": "/var/www/nextcloud/lib/private/Files/SimpleFS/SimpleFolder.php",
        "line": 84,
        "function": "newFile",
        "class": "OC\\Files\\Node\\Folder",
        "type": "->",
        "args": [
          "merged-template-prepend.js"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/JSCombiner.php",
        "line": 179,
        "function": "newFile",
        "class": "OC\\Files\\SimpleFS\\SimpleFolder",
        "type": "->",
        "args": [
          "merged-template-prepend.js"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/JSCombiner.php",
        "line": 105,
        "function": "cache",
        "class": "OC\\Template\\JSCombiner",
        "type": "->",
        "args": [
          "/var/www/nextcloud/core/js",
          "merged-template-prepend.js",
          {
            "__class__": "OC\\Files\\SimpleFS\\SimpleFolder"
          }
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/JSResourceLocator.php",
        "line": 115,
        "function": "process",
        "class": "OC\\Template\\JSCombiner",
        "type": "->",
        "args": [
          "/var/www/nextcloud",
          "core/js/merged-template-prepend.json",
          "core"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/JSResourceLocator.php",
        "line": 71,
        "function": "cacheAndAppendCombineJsonIfExist",
        "class": "OC\\Template\\JSResourceLocator",
        "type": "->",
        "args": [
          "/var/www/nextcloud",
          "core/js/merged-template-prepend.json"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/ResourceLocator.php",
        "line": 78,
        "function": "doFind",
        "class": "OC\\Template\\JSResourceLocator",
        "type": "->",
        "args": [
          "js/merged-template-prepend"
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/TemplateLayout.php",
        "line": 346,
        "function": "find",
        "class": "OC\\Template\\ResourceLocator",
        "type": "->",
        "args": [
          [
            "core/js/dist/main",
            "js/merged-template-prepend",
            "search/js/search",
            "core/l10n/en",
            "js/backgroundjobs",
            "files_pdfviewer/l10n/en",
            "files_pdfviewer/js/previewplugin",
            "files_sharing/l10n/en",
            "files_sharing/js/dist/main",
            "files_videoplayer/l10n/en",
            "files_videoplayer/js/main",
            "notifications/l10n/en",
            "notifications/js/notifications",
            "search/l10n/en",
            "search/js/searchprovider",
            "js/files/fileinfo",
            "js/files/client",
            "files/l10n/en",
            "files/js/merged-index",
            "files_sharing/js/dist/collaboration",
            "spreed/l10n/en",
            "spreed/js/collections",
            "text/l10n/en",
            "text/js/files",
            "federatedfilesharing/l10n/en",
            "federatedfilesharing/js/external",
            "files_rightclick/l10n/en",
            "files_rightclick/js/script",
            "files_rightclick/js/files",
            "onlyoffice/l10n/en",
            "onlyoffice/js/desktop",
            "onlyoffice/js/main",
            "recommendations/l10n/en",
            "recommendations/js/main",
            "js/dist/systemtags",
            "systemtags/l10n/en",
            "systemtags/js/systemtags",
            "comments/l10n/en",
            "comments/js/comments",
            "files_versions/l10n/en",
            "files_versions/js/files_versions",
            "files_sharing/js/dist/files_sharing",
            "files_sharing/js/dist/additionalScripts",
            "viewer/l10n/en",
            "viewer/js/viewer",
            "files/js/dist/sidebar",
            "files/js/fileinfomodel",
            "activity/l10n/en",
            "activity/js/activity-sidebar",
            "files_sharing/js/dist/files_sharing_tab",
            "spreed/js/talk-files-sidebar",
            "spreed/js/talk-files-sidebar-loader",
            "files_trashbin/l10n/en",
            "files_trashbin/js/files_trashbin",
            "firstrunwizard/l10n/en",
            "firstrunwizard/js/about"
          ]
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/TemplateLayout.php",
        "line": 174,
        "function": "findJavascriptFiles",
        "class": "OC\\TemplateLayout",
        "type": "::",
        "args": [
          [
            "core/js/dist/main",
            "js/merged-template-prepend",
            "search/js/search",
            "core/l10n/en",
            "js/backgroundjobs",
            "files_pdfviewer/l10n/en",
            "files_pdfviewer/js/previewplugin",
            "files_sharing/l10n/en",
            "files_sharing/js/dist/main",
            "files_videoplayer/l10n/en",
            "files_videoplayer/js/main",
            "notifications/l10n/en",
            "notifications/js/notifications",
            "search/l10n/en",
            "search/js/searchprovider",
            "js/files/fileinfo",
            "js/files/client",
            "files/l10n/en",
            "files/js/merged-index",
            "files_sharing/js/dist/collaboration",
            "spreed/l10n/en",
            "spreed/js/collections",
            "text/l10n/en",
            "text/js/files",
            "federatedfilesharing/l10n/en",
            "federatedfilesharing/js/external",
            "files_rightclick/l10n/en",
            "files_rightclick/js/script",
            "files_rightclick/js/files",
            "onlyoffice/l10n/en",
            "onlyoffice/js/desktop",
            "onlyoffice/js/main",
            "recommendations/l10n/en",
            "recommendations/js/main",
            "js/dist/systemtags",
            "systemtags/l10n/en",
            "systemtags/js/systemtags",
            "comments/l10n/en",
            "comments/js/comments",
            "files_versions/l10n/en",
            "files_versions/js/files_versions",
            "files_sharing/js/dist/files_sharing",
            "files_sharing/js/dist/additionalScripts",
            "viewer/l10n/en",
            "viewer/js/viewer",
            "files/js/dist/sidebar",
            "files/js/fileinfomodel",
            "activity/l10n/en",
            "activity/js/activity-sidebar",
            "files_sharing/js/dist/files_sharing_tab",
            "spreed/js/talk-files-sidebar",
            "spreed/js/talk-files-sidebar-loader",
            "files_trashbin/l10n/en",
            "files_trashbin/js/files_trashbin",
            "firstrunwizard/l10n/en",
            "firstrunwizard/js/about"
          ]
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/legacy/template.php",
        "line": 184,
        "function": "__construct",
        "class": "OC\\TemplateLayout",
        "type": "->",
        "args": [
          "error",
          ""
        ]
      },
      {
        "file": "/var/www/nextcloud/lib/private/Template/Base.php",
        "line": 132,
        "function": "fetchPage",
        "class": "OC_Template",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/nextcloud/lib/private/legacy/template.php",
        "line": 333,
        "function": "printPage",
        "class": "OC\\Template\\Base",
        "type": "->",
        "args": []
      },
      {
        "file": "/var/www/nextcloud/index.php",
        "line": 65,
        "function": "printExceptionErrorPage",
        "class": "OC_Template",
        "type": "::",
        "args": [
          {
            "__class__": "OCP\\Files\\NotPermittedException"
          },
          500
        ]
      }
    ],
    "File": "/var/www/nextcloud/lib/private/Files/Node/Folder.php",
    "Line": 185,
    "CustomMessage": "--"
  },
  "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/82.0.4063.0 Safari/537.36 Edg/82.0.439.1",
  "version": "18.0.1.3"
}

@pcolmer
Copy link
Author

pcolmer commented Mar 5, 2020

Hmm ... it looks like there may be a problem with the suggested code. If I uncomment the objectstore block with the hardcoded access details, I still have a broken NC service until I rename the new file out of the way. That suggests that NC is trying to use that file but not getting what it needs.

I've installed the AWS CLI on the server and confirmed that the implicit instance role is granting the permissions I need, so the instance role itself is working.

@kesselb
Copy link
Contributor

kesselb commented Mar 5, 2020

Hmm :( You adjusted the arguments to your needs? I just copied my local version (that was configured for minio).

@pcolmer
Copy link
Author

pcolmer commented Mar 5, 2020

Yes, I did but thanks for checking.

I changed the bucket name and removed hostname, port, use_ssl and use_path_style.

Is there any debugging I can add to try and see what is happening/going wrong?

@c-reiter

This comment has been minimized.

@cinghaman

This comment has been minimized.

@c-reiter

This comment has been minimized.

@kesselb
Copy link
Contributor

kesselb commented Apr 11, 2020

I think I have a similar problem as @pcolmer. After building a working setup with Nextcloud 17 and Minio as primary storage a few months ago

@c-reiter You don't have a similar problem. That enhancement request is about reading the credentials for S3 from a EC2 instance profile. It's not about B2 or Minio. If you found a reproducible bug feel free to create a new issue or use https://help.nextcloud.com for questions in general but do not spam unrelated issues with your problems.

@c-reiter
Copy link

@kesselb I'm sorry to have caused any inconvenience, I thought that the problems were related because of the "OCP\Files\NotPermittedException" from Nextcloud. It was never my intention to spam. Thank's for telling me the mistake

@cuppett
Copy link
Contributor

cuppett commented May 9, 2020

<?php

require_once '3rdparty/autoload.php';

use Aws\Credentials\CredentialProvider;
use Aws\Credentials\CredentialsInterface;

/** @var callable $credentialsProvider */
$credentialsProvider = CredentialProvider::defaultProvider();
/** @var CredentialsInterface $credentials */
$credentials = $credentialsProvider()->wait();

$CONFIG = [
	'objectstore' => array(
		'class' => \OC\Files\ObjectStore\S3::class,
		'arguments' => array(
			'bucket' => 'nextcloud',
			'autocreate' => true,
			'key' => $credentials->getAccessKeyId(),
			'secret' => $credentials->getSecretKey(),
			'hostname' => 'localhost',
			'port' => 9600,
			'use_ssl' => false,
			'region' => 'us-east-1',
			// required for some non Amazon S3 implementations
			'use_path_style' => true
		),
	),
];

Using above file as config/objectstore.config.php should work. It's a bit hacky because we need to load the autoloader for external libraries but then we can fetch the credentials from the identity provider. I have not tested it with a ec2 instance but the code is called at the right time ;)

This won't work as it doesn't use the session token. Also unclear how long these params are used (will have to take into account the expiration also).

$> curl 169.254.169.254/latest/meta-data/identity-credentials/ec2/securit-credentials/ec2-instance/

  "Code" : "Success",
  "LastUpdated" : "2020-05-09T11:51:11Z",
  "Type" : "AWS-HMAC",
  "AccessKeyId" : "ASIA...NWN",
  "SecretAccessKey" : "66MOR...oO00L",
  "Token" : "IQoJb3JpZ2luX2...G1w4=",
  "Expiration" : "2020-05-09T17:59:44Z"
}```

@cuppett
Copy link
Contributor

cuppett commented May 9, 2020

Sorry for the commit noise, but I have a PR up which adds this seamlessly. Let's you omit the key/secret in the parameter file altogether and will pick it up (fallback) as either environment variables or instance profile.

This is the important part:

CredentialProvider::chain(
	$this->paramCredentialProvider(),
	CredentialProvider::env(),
	CredentialProvider::instanceProfile()
)

Should be helpful both on EC2 or ECS using IAM roles or also when using environment variables from Secrets in k8s.

@kesselb kesselb linked a pull request May 9, 2020 that will close this issue
@MorrisJobke MorrisJobke mentioned this issue Aug 11, 2020
57 tasks
rullzer added a commit that referenced this issue Aug 20, 2020
Resolves #19790, Provides Support for IAM Credentials
kesselb pushed a commit that referenced this issue Aug 31, 2020
Includes support for either leveraging environment variables
passed to the PHP runtime or IAM instance profile present
on the host being used. The default and first choice is
still the parameter file as documented.

See also: https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/guide_credentials_provider.html#chaining-providers

Signed-off-by: Stephen Cuppett <steve@cuppett.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap enhancement feature: object storage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants