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

Support workload identity and default app credentials for GCS signed URLs and or provide example of how to sign the blob yourself #2410

Open
red8888 opened this issue May 17, 2023 · 14 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@red8888
Copy link

red8888 commented May 17, 2023

See SO post: https://stackoverflow.com/questions/76266019/how-do-i-generate-signed-urls-for-gcs-with-workload-identity-in-the-c-sharp-sdk/76266912#76266912

Currently workload id is not supported for generating signed URLs for GCS. This is a problem because it forces a bad practice (generating a static json key instead of using workload id which is much more secure).

At the very least can someone provide a full example of how to use the SignBlob method in conjunction with SignedURL to work around this problem?

Also, I think default app creds are also not supported right? This makes the code painful to run locally for testing purposes.

@red8888 red8888 added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels May 17, 2023
@jskeet
Copy link
Collaborator

jskeet commented May 17, 2023

If this feature request is specific to Google.Cloud.Storage.V1, please reopen it on https://github.com/googleapis/google-cloud-dotnet, which is the repo where that library lives.

Also, I think default app creds are also not supported right? This makes the code painful to run locally for testing purposes.

It's unclear what you mean by that. Supported in what respect? Please be more specific.

@jskeet
Copy link
Collaborator

jskeet commented May 17, 2023

Apologies, I hadn't noticed that @amanda-tarafa had specifically requested raising an issue here instead of in the other repo. (She's worked on the signing infrastructure more recently than I have, so I trust her judgement. The sample code change would be in google-cloud-dotnet, but we can handle that.)

@red8888
Copy link
Author

red8888 commented May 17, 2023

Cool thanks! Yeah I'm just looking for some kind of official-ish example snippet so I know I'm doing it right as it seems non-standard/undocumented.

@jskeet
Copy link
Collaborator

jskeet commented May 17, 2023

We can work on creating a sample - we had a sample until recently, which was removed in this commit - you may be able to use that as a starting point until we can potentially restore it in the docs.

@amanda-tarafa Can you remember why we removed the IAM service signer sample? Was it that we didn't expect it to be useful with the new support?

@red8888: If you could clarify what you would expect to be able to do with default application credentials that doesn't currently work, that would help us to help you.

@red8888
Copy link
Author

red8888 commented May 17, 2023

@jskeet Sorry default app creds may work as expected so I guess I shouldn't have assumed. I just saw other somewhat related github issues saying it didn't or it behaved differently: octue/django-gcp#10

I really just wanted to confirm that if I use sigblob the same code will run correctly on gke with workload id and locally with default app creds without needing to introduce logic in the code to support both scenarios.

@jskeet
Copy link
Collaborator

jskeet commented May 17, 2023

@jskeet Sorry default app creds may work as expected so I guess I shouldn't have assumed. I just saw other somewhat related github issues saying it didn't or it behaved differently: octue/django-gcp#10

Right, I believe ADC credentials will work if the underlying credential supports signing (which I believe is still not the case for WIF, as Amanda indicated in the Stack Overflow answer).

I really just wanted to confirm that if I use sigblob the same code will run correctly on gke with workload id and locally with default app creds without needing to introduce logic in the code to support both scenarios.

I'd expect that to be the case if both kinds of credential have access to IAM to perform the signing remotely. But it'll be significantly slower than any local signing.

At this point we're really at the edge of my knowledge though, so I'll assign to @amanda-tarafa for further comment.

@amanda-tarafa
Copy link
Contributor

We might be able to offer some out of the box sopport for URL signing with external account credentials, but we need to confirm a few things first, so please bare with us.

If we cannot offer out of the box support we'll publish a sample.

I'll update here as we know more.

@amanda-tarafa
Copy link
Contributor

As described in the documentation, GoogleCredential (The type returnes by ADC) is only supported if the underlying credential type is itself supported, and user crendetial is not currently supported out of the box.
For all currently unsuported credentials you can call the IAM API SignBlob operation and it should work. But we are looking into supporting this ourselves. We still need to confirm a few things from the Auth side of things, but we'll get there I believe.

I'll update when I know more.

@OneEcho
Copy link

OneEcho commented May 30, 2023

Here's a class that I was able to use to generate signed URLs. It uses the IAM API SignBlob operation since we wanted to use default application credentials instead of a service account key.

public class IamServiceBlobSigner : UrlSigner.IBlobSigner
{
    private readonly IAMCredentialsClient _iamService;

    public string Id { get; }

    public string Algorithm => "GOOG4-RSA-SHA256";

    internal IamServiceBlobSigner(IAMCredentialsClient service, string serviceAccountId)
    {
        _iamService = service;
        Id = serviceAccountId;
    }

    public string CreateSignature(byte[] data, UrlSigner.BlobSignerParameters _)
    {
        var request = CreateRequest(data);
        var response = _iamService.SignBlob(request);
        return response.SignedBlob.ToBase64();
    }

    public async Task<string> CreateSignatureAsync(byte[] data, UrlSigner.BlobSignerParameters _, CancellationToken cancellationToken)
    {
        var request = CreateRequest(data);
        var response = await _iamService.SignBlobAsync(request);
        return response.SignedBlob.ToBase64();
    }

    private SignBlobRequest CreateRequest(byte[] data)
    {
        var account = $"projects/-/serviceAccounts/{Id}";
        var base64Data = Convert.ToBase64String(data);
        var request = new SignBlobRequest
        {
            Name = account,
            Payload = ByteString.FromBase64(base64Data)
        };
        return request;
    }
}

@mbamm
Copy link

mbamm commented Dec 13, 2023

@amanda-tarafa any updates on this feature being part of the library? Or do you have an official example?
thanks

@amanda-tarafa
Copy link
Contributor

@mbamm We'll work on this next year but we don't have an ETA yet. I'd expect this to happen in the first half of the year though.

As a sample starting point, you can use the sample that we originally had and was removed in this commit.

For context, we removed the sample because it was meant to demonstrate URL signing using a Compute credential, and that is currently supported by the library out of the box. It's just Workload Indentity Federation credentials that are not supported out of the box at the moment.

@amanda-tarafa amanda-tarafa added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p3 Desirable enhancement or fix. May not be included in next release. labels May 2, 2024
@mbamm
Copy link

mbamm commented Jan 22, 2025

@amanda-tarafa any updates on this feature being part of the library? thanks

@red8888
Copy link
Author

red8888 commented Jan 22, 2025

Just want to circle back on this, we really need doc updates ASAP while we wait on official SDK support.

I just had to have this same conversation with python devs about a month ago. I don't think its hyperbolic at this point to say this is very bad. Google is directly contradicting their own security best practices and the docs are a real problem.

They merely say you MUST use a json key. I have to explain to devs that this is in fact not required and its just a missing feature of the SDKs- essentially convincing them Google's own docs are bad and violate their own security best practices :(

Maybe a stop gap would be to add an info/warning box or something to the docs so I don't have to keep having these conversations? Google is actively pitting my devs against me here lol. The least we could do is add notes/examples to the docs so there is something official that helps me press the devs to do a little extra work and not create static json keys especially after setting up workload id and oidc etc everywhere.

@amanda-tarafa
Copy link
Contributor

@mbamm still on the backlog I'm afraid, but something we still want to support out of the box. I can't offer an ETA at the moment, I'm sorry about that. Please see my previous comment. You can use the sample linked there as a starting point for implementing IAM calls to the signing endpoint using a Workload Identity Credential. Just note the example is for a Compute Credential, which is already supported out of the box by the .NET Clud SDK.

@red8888 if you can provide a link to the documentation contradicting best practices I'm happy to look into getting those fixed or pointing you to the correct repos to make such requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

5 participants