[go: nahoru, domu]

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

Investigate UrlSigner support for ComputeCredential #2316

Closed
jskeet opened this issue Jul 9, 2018 · 30 comments
Closed

Investigate UrlSigner support for ComputeCredential #2316

jskeet opened this issue Jul 9, 2018 · 30 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@jskeet
Copy link
Collaborator
jskeet commented Jul 9, 2018

Branched off #1056, specifically for ComputeCredential, or possibly ServiceCredential.

[A] limitation makes it nearly impossible to use UrlSigner in conjunction with implicit service account that is associated with GCE instance. When running an app on GCE instance and calling to GoogleCredential.GetApplicationDefault() the ComputeCredential is returned. There is no way to get an instance of ServiceAccountCredential from an instance of ComputeCredential.
I would expect that since both are based on ServiceCredential type, they both can be used for signing URLs. But the library is very strict on the matter.

@jskeet jskeet added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: storage Issues related to the Cloud Storage API. status: investigating The issue is under investigation, which is determined to be non-trivial. labels Jul 9, 2018
@jskeet jskeet self-assigned this Jul 9, 2018
@jskeet
Copy link
Collaborator Author
jskeet commented Jul 9, 2018

Initial investigation: ComputeCredential doesn't hold any secrets. The only information it has is a URL to talk to, when it needs to request an access token. Unless the metadata server is capable of performing URL signing, this could still be impossible with only a ComputeCredential, leaving the following fallback options:

  • Providing a service account credential file even when running on Compute Engine
  • Creating an access key and signing with HMAC-SHA1

(I'm not giving up yet though.)

@minherz
Copy link
minherz commented Jul 9, 2018

Thank you. The first option requires local management of the service account credentials including rotation of private key and re-deployment of the renewal credentials to running instances. Therefore, it isn't good for production.
If it is possible to implement a signing algorithm using ServiceCredential token, it will be a simplest and fastest solution.
An alternative is to implement something in GCS as a new API 😞

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 9, 2018

I've asked internally whether this is feasible; I'll update this issue when I hear back.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 11, 2018

Okay, I've been looking into this a bit more, and I've worked out a way that I believe will work, but I want to see whether it'll be useful to you or not before I implement it.

The IAM API has a SignBlob method which I believe will create the appropriate signature that UrlSigner uses.

UrlSigner needs two things from the ServiceAccountCredentials at the moment:

  • A way of signing an arbitrary blob
  • An ID

You could either create a service account specifically for URL signing, and grant access to the default Compute Engine service account, or you could use the metadata API to discover the default Compute Engine service account and use the directly. With the first option you'd have to specify the account to use as part of your application configuration.

I'm prototyping this now just to prove that it works, and I'll update this issue with a link to a "not for submission" PR if it does.

jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 11, 2018
@jskeet
Copy link
Collaborator Author
jskeet commented Jul 11, 2018

Okay, I've prototyped it in #2325.

The changes in UrlSigner are all I'd propose for the Google.Cloud.Storage.V1 project. That would unblock you, as you'd be able to include the code that's currently in the demo project in your own project.

We might then encapsulate that code in a separate package; it would probably depend on demand.

I was able to use this to sign a URL with a different service account than the one I was authenticated as; I had to:

  • Enable the IAM API in the Cloud Console
  • Give the authenticated service account the role of "Service Account Token Creator"

Please let me know whether this meets your needs. If it does, I'll make the production code changes "properly" (comments, tests etc) and then create another release.

@minherz
Copy link
minherz commented Jul 11, 2018

@jskeet it sounds as a solution. if the role of "Service Account Token Creator" is the only addition beside "Cloud Storage Editor" role that the service account requires, I think it should be safe enough to grant it.
I will try to create a minimal program out of your commit and run it in our environment. I will update you in couple of days about the results.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 11, 2018

@minherz: Great! Glad it sounds like this could work for you. Let me know if you run into any problems building my commit - the tricky bit is likely to be that you'll need to deploy the locally-modified version of Google.Cloud.Storage.V1 with blob signer part.

@minherz
Copy link
minherz commented Jul 12, 2018

@sjkeet is it possible to retrieve a default (for current instance) service account id instead of getting it explicitly?

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

@minherz: You'd have to explicitly fetch it from the metadata service. We could potentially include code to do that, but for the moment this documentation is the appropriate starting point, I think.

@minherz
Copy link
minherz commented Jul 12, 2018

@jskeet: means there is no dotnet support for the metadata REST API?

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

There's Google.Cloud.Metadata.V1 - it's in alpha, but it may do what you need; I haven't tried it for this purpose.

(It's mostly been in alpha for organizational reasons. We could definitely put some more time into reviewing the API, promoting it to beta, and then eventually to GA. We haven't seen much need for it before, but this could be a good use for it.)

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

(And if that's a good starting point but doesn't quite have the functionality it should, we could look into implementing the relevant functionality.)

@minherz
Copy link
minherz commented Jul 12, 2018

I cannot add it to my solution. Executing Install-Package Google.Cloud.Metadata.V1 -Version 1.0.0-alpha07 in Nuget console produces an error: Install-Package : Project 'Default' is not found.
Is there a way to use HttpClient which is a property of the ServiceCredential class to send a request? Should I manually an access token as a header for the request or there is a way to do it implicitly?

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

Hmm... I'm not sure where that error's coming from; I've just tried it myself and it worked. On the other hand, it has some very old dependencies. I'll look at getting that updated, and see whether it does provide service account access.

I don't know offhand whether you need an access token to access the metadata server; I suspect you don't given that if you're running on GCE it "knows" which instance it's from. I suspect you just want to create a new HttpClient instead (which you can cache) just like ComputeCredential does

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

It looks like if you can get Google.Cloud.Metadata.V1 to work, it should give you the information you want:

var metadataClient = MetadataClient.Create();
var metadata = metadataClient.GetInstanceMetadata();
var serviceAccountEmail = metadata.ServiceAccounts.FirstOrDefault()?.Email;

I'll try that in Cloud Shell and see whether it does what I'd expect... then update its dependencies and release either another alpha or possibly a beta. (That won't be this week though; it's near the end of the day in the UK and I'm on vacation tomorrow. I can do the quick experiment today, but not the rest.)

@minherz
Copy link
minherz commented Jul 12, 2018

NP. Thank you.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

Hmm... that didn't fetch any service accounts at all. Will need to look more closely...

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

Hmm. For some reason, running this:

$ curl http://169.254.169.254/computeMetadata/v1/instance/service-accounts/default/email

shows me the service account email, but this (which is what the library does):

$ curl http://169.254.169.254/computeMetadata/v1/instance/?recursive=true

... doesn't include the service accounts. I'll have to ask about that internally. So for now, I'd recommend just fetching the first URL with a newly-minted HttpClient. That's probably only about as much code as using the library anyway :)

@minherz
Copy link
minherz commented Jul 12, 2018

It seems that the service account also requires iam.serviceAccounts.signBlob permission. When I initialize GoogleCredential by a private key file I have things working, but when I do using GetApplicationDefault() it complains at a line that calls to iamService.Projects.ServiceAccounts.SignBlob().
I use your PR to built smth without a need for modified UrlSigner class.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 12, 2018

I'm afraid I don't quite follow what you've done or where the problem is. Are you able to share what you've done?

@minherz
Copy link
minherz commented Jul 12, 2018

Sorry, it seems that a GCE instance i was testing on wasn't bound to the right service account. The role that you described ("Service Account Token Creator") grants this permission.

@minherz
Copy link
minherz commented Jul 12, 2018

Is it right to use HTTP to retrieve metadata?
Beside that, I got it working. Thank you. I believe that releasing a requirement to have a private key file locally in order to use UrlSigner in GCS client library will provide more secure solutions. We will use an aggregating class which will make use of private key for local (not in Cloud) development and signing using sign-blob REST API for production.

@minherz
Copy link
minherz commented Jul 12, 2018

off-topic question, is it possible to get GoogleCredential of the service account based on access key and secret pair like it works in many other systems? Currently, the only way to instantiate GoogleCredential for the service account is by providing a private key file path to GoogleCredential.FromFile().

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 13, 2018

@minherz: Yes, I believe that using HTTP is fine. It's all within Google's infrastructure, and I believe it will be encrypted on the wire anyway.

I don't believe it's possible to create a GoogleCredential just on the access key and secret; you may be able to construct specific other credential types that way though.

You mentioned earlier that you hadn't needed my modifications to UrlSigner - could I ask how you did it in that case? I'm trying to work out whether I should make the change to UrlSigner or not.

@minherz
Copy link
minherz commented Jul 13, 2018

I meant that as part of our solution I cannot propose them to use a cloned version of the library. That's why instead of incorporating a change inside Google.Cloud.Storage.V1.UrlSigner I just did it through fascade class that either uses the google UrlSigner or uses sign-blob API. If you apply the change and the new version will be released, we will modify our code to use it directly. So, please submit your changes to UrlSigner. I will appreciate if you notify me when it is released.

jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 16, 2018
jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 16, 2018
jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 16, 2018
jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 17, 2018
This addresses googleapis#2316 by creating a seam for the signing part of
UrlSigner.

In the future we may want to release a package specifically to make
the IAM integration simple, but for the moment we've just got a
sample in the docs.

(Another possibility would be to add a dependency on the IAM
package and include the IAM signer here, but I'd prefer not to do
that.)
@jskeet
Copy link
Collaborator Author
jskeet commented Jul 17, 2018

Just to come back on the "service accounts from the metadata server" part - while it doesn't show in Cloud Shell, executing this:

$ curl http://169.254.169.254/computeMetadata/v1/instance/?recursive=true

... on a "regular" VM does show the service accounts. So you could use the Metadata package for that if you want - but for just this, it's probably equally simple to fetch the value directly.

@minherz
Copy link
minherz commented Jul 17, 2018

Thank you. As i wrote, i have a problem using metadata client library Nuget package for some reason. We also can have a regulatory problem to approve the use of alfa package. However, the following simple code replaces the need for the package:

var c = new HttpClient();
c.DefaultRequestHeaders.Add("Metadata-Flavor", "Google");
serviceId = await c.GetStringAsync("http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/email");

Thank you again, for the all help. If the changes (from #2325) can be added into Google.Cloud.Storage.V1, we gladly will migrate from the custom solution.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 17, 2018

@minherz: See #2338 which is the "production-ready" version of #2325. (It includes an async version as well as synchronous.) It also gives an example of fetching the default service account. (It's not quite how you've said, but similar.)

jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 17, 2018
This addresses googleapis#2316 by creating a seam for the signing part of
UrlSigner.

In the future we may want to release a package specifically to make
the IAM integration simple, but for the moment we've just got a
sample in the docs.

(Another possibility would be to add a dependency on the IAM
package and include the IAM signer here, but I'd prefer not to do
that.)
jskeet added a commit to jskeet/google-cloud-dotnet that referenced this issue Jul 18, 2018
This addresses googleapis#2316 by creating a seam for the signing part of
UrlSigner.

In the future we may want to release a package specifically to make
the IAM integration simple, but for the moment we've just got a
sample in the docs.

(Another possibility would be to add a dependency on the IAM
package and include the IAM signer here, but I'd prefer not to do
that.)
jskeet added a commit that referenced this issue Jul 18, 2018
This addresses #2316 by creating a seam for the signing part of
UrlSigner.

In the future we may want to release a package specifically to make
the IAM integration simple, but for the moment we've just got a
sample in the docs.

(Another possibility would be to add a dependency on the IAM
package and include the IAM signer here, but I'd prefer not to do
that.)
@jskeet
Copy link
Collaborator Author
jskeet commented Jul 18, 2018

The change is now merged; I'll look into doing a new release shortly.

@jskeet
Copy link
Collaborator Author
jskeet commented Jul 18, 2018

This is now released as 2.2.0-beta02. I don't think there's anything really stopping us from going to GA with it as 2.2.0, other than the desire to let it "bake" in case there are problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants