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

DDoS-ing the Metadata Server. #297

Closed
xvzf opened this issue Aug 3, 2020 · 8 comments
Closed

DDoS-ing the Metadata Server. #297

xvzf opened this issue Aug 3, 2020 · 8 comments
Assignees
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@xvzf
Copy link

xvzf commented Aug 3, 2020

Hello,

With high volume traffic, a check (on each interaction with Google APIs!!!) if the application is running on GCP by calling the metadata server is becoming quite a bottleneck response-time wise.

The specific code is located here:

public static function onGce(callable $httpHandler = null)

Environment details

  • OS: Linux
  • PHP version: 7.x
  • Package name and version: irrelevant
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Aug 4, 2020
@dwsupplee dwsupplee added status: investigating The issue is under investigation, which is determined to be non-trivial. type: question Request for information or clarification. Not an issue. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Aug 10, 2020
@xvzf
Copy link
Author

xvzf commented Aug 11, 2020

@dwsupplee since you tagged it as investigating, it's not just PHP 7.4 but all 7.x releases

@ericnorris
Copy link

ericnorris commented Aug 25, 2020

At Etsy we've encountered the same issue; during periods of high enough traffic (e.g. backfills) we've seen metadata server outages that result in the following backtrace:

Could not load the default credentials. Browse to https://developers.google.com/accounts/docs/application-default-credentials for more information
DomainException trace:
[Dev_Backtrace]:
#12 getCredentials() /GoogleCloudCredentials.php:70
...

...where the getCredentials is this call:

public static function getCredentials(

...which ends up calling the onGce method in the original comment.

We use the FetchAuthTokenCache to avoid hammering the metadata server with auth calls, but this onGce call went unnoticed until we experienced outages. Internally we are trying to resolve this by passing in a special $httpHandler to ApplicationDefaultCredentials::getCredentials, but ideally the library would offer a PSR-6 way of caching this, much like how the FetchAuthTokenCache works.

@bshaffer bshaffer self-assigned this Aug 28, 2020
@bshaffer
Copy link
Contributor

bshaffer commented Aug 29, 2020

If we added a new class, like @ericnorris suggested, we could have something like this:

// GCE cache implementing CacheItemPoolInterface
$cache = new FileCache();
// optional GCE cache config
$cacheConfig = [
    'lifetime' => 1500,
    'prefix' => '',
];
$gceCache = new GCECache($cache, $cacheConfig);

// fetch default credentials with the cache
$credentials = ApplicationDefaultCredentials::getCredentials($scope, null, null, null, null, $gceCache);
// check directly with the cache
$onGce = GCECredentials::getCredentials(null, $gceCache);

We can't use the existing $cache and $cacheConfig parameters passed to ApplicationDefaultCredentials::getCredentials because the cache config / cache interface may be different for the GCE metadata cache.

We could also add CacheItemPoolInterface and $cacheConfig as additional parameters and use GCECache under the hood. This would be messier, but at least it would be consistent:

// FetchAuthTokn cache implementing CacheItemPoolInterface
$authTokenCache = new FileCache();
// optional GCE cache config
$authTokenCacheConfig = [
    'lifetime' => 1500,
    'prefix' => '',
];

// GCE cache implementing CacheItemPoolInterface
$gceCache = new FileCache();
// optional GCE cache config
$gceCacheConfig = [
    'lifetime' => 1500,
    'prefix' => '',
];

// fetch default credentials with the cache
$credentials = ApplicationDefaultCredentials::getCredentials(
    $scope,
    null,
    null,
    $authTokenCache,
    $authTokenCacheConfig,
    $gceCache,
    $gceCacheConfig
);
// check directly with the cache
$onGce = GCECredentials::getCredentials(null, $gceCache, $gceCacheConfig);

Do these solutions sound sufficient, and do you have a preference for either of them?

@ericnorris
Copy link

@bshaffer, I actually proposed a different solution internally (props to @etsteeeve for implementing) that seems to work for us. We've instead written a wrapper class called LazyApplicationDefaultCredentials that only calls ApplicationDefaultCredentials::getCredentials when ::fetchAuthToken and ::getLastReceivedToken is called on it. Since this is passed into a FetchAuthTokenCache, this means the ::onGce call is only made once(ish) every ~25 minutes or so.

Perhaps this could be implemented in the library as a DefaultCredentialsFetcher?

@bshaffer bshaffer added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed type: question Request for information or clarification. Not an issue. status: investigating The issue is under investigation, which is determined to be non-trivial. labels Aug 31, 2020
@bshaffer
Copy link
Contributor

@ericnorris This is a really great idea! If you or @etsteeeve want to submit a PR please do! Otherwise I can take a look.

It would be nice to have caching for ::onGce independently, so we could implement both of these options.

@bshaffer
Copy link
Contributor

bshaffer commented Sep 1, 2020

I've coded up a potential option, please tell me what you think.

@bshaffer
Copy link
Contributor

bshaffer commented Sep 8, 2020

fixed in v1.12.0

Passing a cache into ApplicationDefaultCredentials::getCredentials will automatically cache calls to onGce:

// fetch default credentials with the cache
$credentials = ApplicationDefaultCredentials::getCredentials(
    $scope,
    null,
    null,
    $cacheConfig,
    $cache
);

@bshaffer bshaffer closed this as completed Sep 8, 2020
@Xfaider48
Copy link

@bshaffer I disagree with the provided soultion.

I think this code must be disabled by default. I'm sure that this is a big problem when without any credentials you get enabled this mechanism and it ruins any feature tests. E.g. when you use datastore/spanner (or something else with GRPC) in feature tests and you don't need any credentials (because you can use datastore/spanner emulators) you get each test slowed down by at least 1.5 sec (3 tries with 500ms timeout per GRPC class). And the default caching mechanism doesn't work because it's in-memory storage and a state of an app is cleared between test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants