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

Cloud Storage UrlSigner support for HMAC sigs #6026

Closed
NinoFloris opened this issue Mar 12, 2021 · 6 comments · Fixed by #9509
Closed

Cloud Storage UrlSigner support for HMAC sigs #6026

NinoFloris opened this issue Mar 12, 2021 · 6 comments · Fixed by #9509
Assignees
Labels
api: storage Issues related to the Cloud Storage API. 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.

Comments

@NinoFloris
Copy link

Looking into the UrlSigner I noticed it specifically hardcodes the GOOG4-RSA-SHA256 algo. Could an extension to the UrlSigner be made to support HMAC signing?

The reason for this ask is that RSA signing is rather computationally expensive at the amount of signatures we create per request and we would like to switch to HMAC, ideally without having to maintain the requisite hmac secret derivation and signing bits.

@jskeet jskeet self-assigned this Mar 12, 2021
@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. labels Mar 12, 2021
@jskeet
Copy link
Collaborator

jskeet commented Mar 12, 2021

I've asked a colleague who knows more about what the Storage service supports than I do. (I see in the documentation that there's support for HMAC, but I'm not sure of the implications of that in terms of client libraries.)

@NinoFloris
Copy link
Author

Thanks!

I found these two pages to give a decent overview
https://cloud.google.com/storage/docs/authentication/signatures
https://cloud.google.com/storage/docs/migrating#authentication

it looks a lot like the rsa signing in terms of query params and payload.

@jskeet
Copy link
Collaborator

jskeet commented Mar 17, 2021

There's discussion about this internally now. Just to set expectations, if we decide to do this, we'll want to do it across most/all of the languages for which we have client libraries - which means it's unlikely to be a quick fix, if you see what I mean.

@NinoFloris
Copy link
Author

Meant to reply here earlier. Thanks for bringing it up and letting me know. We can wait, it's just something on our radar to improve when we can.

@jskeet
Copy link
Collaborator

jskeet commented Mar 26, 2021

Thanks - there's been some progress on our side, in terms of discussions. Nothing firm yet, but I'm going to investigate whether I can provide you with a workaround to implement it reasonably simply within your own code without a library change (but still using UrlSigner). I hope to get to that next week. (It may prove a dead-end, and would only be a temporary workaround anyway, but it would be better than nothing.)

@jskeet
Copy link
Collaborator

jskeet commented Mar 30, 2021

Okay, I've looked into this a bit further. There's good news and bad news. The good news is that I have managed to get this to work. The bad news is that there are two reasons it won't work without further changes to the library:

  • The V4Signer always uses an algorithm value of "GOOG4-RSA-SHA256" in the request, whereas for HMAC we need "GOOG4-HMAC-SHA256"
  • The blob signing stage needs to know the date field in the request. While we can fetch "the current date (UTC)" again, it does mean there's a race condition: around midnight we could end up creating a request with a date field of (say) 2021-03-29, and then sign it using a date of 2021-03-30 which would obviously be invalid

So there's no interim workaround - we'll need to wait for this to be prioritized appropriately and designed across multiple languages.

Here's the HMAC blob signer code though, mostly for my own reuse later on:

// Copyright 2021 Google LLC
// 
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// 
//     https://www.apache.org/licenses/LICENSE-2.0
// 
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using Google.Api.Gax;
using System;
using System.Globalization;
using System.Security.Cryptography;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace Google.Cloud.Storage.V1
{
    public sealed partial class UrlSigner
    {
        /// <summary>
        /// Blob signer to implement signing using an HMAC secret.
        /// </summary>
        internal sealed class HmacBlobSigner : IBlobSigner
        {
            private const string Location = "auto";
            private const string Service = "storage";
            private const string Prefix = "GOOG4";
            private const string RequestType = "goog4_request";

            private readonly string _accessId;
            private readonly string _secret;
            private readonly IClock _clock;

            internal HmacBlobSigner(string accessId, string secret, IClock clock) =>
                (_accessId, _secret, _clock) = (accessId, secret, clock);

            public string Id => _accessId;

            public string CreateSignature(byte[] data)
            {
                // TODO: Fix the timing race condition here. It's just possible that 
                // the clock has ticked over from one day to the next. We really want
                // to use the same date that we used to create the request.
                string date = _clock.GetCurrentDateTimeUtc().ToString("yyyyMMdd", CultureInfo.InvariantCulture);

                // Implements the steps in https://cloud.google.com/storage/docs/authentication/signatures#derive-key
                var utf8 = Encoding.UTF8;
                byte[] initialKey = utf8.GetBytes(Prefix + _secret);
                byte[] keyDate = Hash(initialKey, utf8.GetBytes(date));
                byte[] keyRegion = Hash(keyDate, utf8.GetBytes(Location));
                byte[] keyService = Hash(keyRegion, utf8.GetBytes(Service));
                byte[] signingKey = Hash(keyService, utf8.GetBytes(RequestType));
                byte[] signature = Hash(signingKey, data);
                return Convert.ToBase64String(signature);

                static byte[] Hash(byte[] key, byte[] data)
                {
                    using (HMACSHA256 hmac = new HMACSHA256(key))
                    {
                        return hmac.ComputeHash(data);
                    }
                }
            }

            public Task<string> CreateSignatureAsync(byte[] data, CancellationToken cancellationToken) =>
                Task.FromResult(CreateSignature(data));
        }
    }
}

@jskeet jskeet assigned amanda-tarafa and unassigned jskeet Feb 9, 2022
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue May 11, 2022
And to make signing more flexible in general.

Towards googleapis#6026
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue May 12, 2022
And to make signing more flexible in general.

Towards googleapis#6026
amanda-tarafa added a commit that referenced this issue May 12, 2022
And to make signing more flexible in general.

Towards #6026
jskeet pushed a commit that referenced this issue May 13, 2022
And to make signing more flexible in general.

Towards #6026
jskeet pushed a commit to jskeet/google-cloud-dotnet that referenced this issue May 24, 2022
And to make signing more flexible in general.

Towards googleapis#6026
jskeet pushed a commit that referenced this issue May 24, 2022
And to make signing more flexible in general.

Towards #6026
jskeet pushed a commit to jskeet/google-cloud-dotnet that referenced this issue Jun 6, 2022
And to make signing more flexible in general.

Towards googleapis#6026
jskeet pushed a commit that referenced this issue Jun 6, 2022
And to make signing more flexible in general.

Towards #6026
jskeet pushed a commit that referenced this issue Jun 6, 2022
And to make signing more flexible in general.

Towards #6026
@amanda-tarafa amanda-tarafa added the priority: p3 Desirable enhancement or fix. May not be included in next release. label Jun 6, 2022
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 6, 2023
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 6, 2023
amanda-tarafa added a commit to amanda-tarafa/google-cloud-dotnet that referenced this issue Jan 7, 2023
amanda-tarafa added a commit that referenced this issue Jan 10, 2023
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. 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants