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

auth/storage: support for the SharedKey implementation used by the Storage Accounts Data Plane API #512

Merged
merged 2 commits into from
Aug 5, 2020

Conversation

tombuildsstuff
Copy link
Contributor

Well this is a fun one.

After a bunch of debugging it turns out API's in the root of a Storage Data Plane API use a slightly different, undocumented implementation of SharedKey authorization. Specifically this API and this API - but presumably the others in the root too.

Whilst this is unfortunate - attempting to update this API to match the definition at this point in time would be a breaking change and thus is a less ideal solution.

Instead this PR introduces a new mode to the SharedKey Authorizer which supports the varied authorization method used by this endpoint.


Before this PR, when attempting to hit the Account Endpoint using a SharedKey or SharedKeyLite authorizer with this Request using the SharedKey authorizer:

GET /?comp=properties&restype=service HTTP/1.1
Host: unlikely23exst2acctazcnp.blob.core.windows.net
User-Agent: Go/go1.13.5 (amd64-darwin) go-autorest/v13.3.0 tombuildsstuff/giovanni/v0.8.0 storage/2018-11-09
Authorization: SharedKey unlikely23exst2acctazcnp:{removed}
X-Ms-Date: Tue, 10 Mar 2020 10:04:41 GMT
X-Ms-Version: 2018-11-09
Accept-Encoding: gzip

returns the following Response..

HTTP/1.1 403 Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.
Content-Length: 687
Content-Type: application/xml
Server: Microsoft-HTTPAPI/2.0
x-ms-request-id: a3addf0a-b01e-002e-09c3-f6cf21000000
x-ms-error-code: AuthenticationFailed
Date: Tue, 10 Mar 2020 10:04:40 GMT
Connection: keep-alive

<?xml version="1.0" encoding="utf-8"?><Error><Code>AuthenticationFailed</Code><Message>Server failed to authenticate the request. Make sure the value of Authorization header is formed correctly including the signature.
RequestId:a3addf0a-b01e-002e-09c3-f6cf21000000
Time:2020-03-10T10:04:41.2097467Z</Message><AuthenticationErrorDetail>The MAC signature found in the HTTP request '{removed}' is not the same as any computed signature. Server used following string to sign: 'GET











x-ms-date:Tue, 10 Mar 2020 10:04:41 GMT
x-ms-version:2018-11-09
/unlikely23exst2acctazcnp/
comp:properties
restype:service'.</AuthenticationErrorDetail></Error>

With these changes, and this new Authorizer - the following Request:

PUT /?comp=properties&restype=service HTTP/1.1
Host: unlikely23exst2acctggtll.blob.core.windows.net
User-Agent: Go/go1.13.5 (amd64-darwin) go-autorest/v13.3.0 tombuildsstuff/giovanni/v0.8.0 storage/2018-11-09
Content-Length: 146
Authorization: SharedKey unlikely23exst2acctggtll:{removed}
X-Ms-Date: Tue, 10 Mar 2020 10:17:00 GMT
X-Ms-Version: 2018-11-09
Accept-Encoding: gzip

<?xml version="1.0" encoding="UTF-8"?>
<StorageServiceProperties><StaticWebsite><Enabled>true</Enabled></StaticWebsite></StorageServiceProperties>

.. returns the following Response:

HTTP/1.1 202 Accepted
Content-Length: 0
Server: Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0
x-ms-request-id: aca9eb77-301e-007a-67c5-f6b5af000000
x-ms-version: 2018-11-09
Date: Tue, 10 Mar 2020 10:17:02 GMT
Connection: keep-alive

@tombuildsstuff
Copy link
Contributor Author

Worth saying this ends up fixing the associated test on our end:

$ TF_ACC=1 go test -v ./azurerm/internal/services/storage/tests/ -timeout=60m -run=TestAccAzureRMStorageAccount_staticWebsiteEnabled
=== RUN   TestAccAzureRMStorageAccount_staticWebsiteEnabled
=== PAUSE TestAccAzureRMStorageAccount_staticWebsiteEnabled
=== CONT  TestAccAzureRMStorageAccount_staticWebsiteEnabled
--- PASS: TestAccAzureRMStorageAccount_staticWebsiteEnabled (214.03s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage/tests	214.097s

tombuildsstuff added a commit to hashicorp/terraform-provider-azurerm that referenced this pull request Mar 10, 2020
* v0.9.0 of github.com/tombuildsstuff/giovanni
* a fork of github.com/Azure/go-autorest including Azure/go-autorest#512
katbyte pushed a commit to hashicorp/terraform-provider-azurerm that referenced this pull request Mar 19, 2020
This PR supersedes #6049 as nested go modules and merge conflicts do not spark joy - but fundamentally this:

- updates github.com/Azure/azure-sdk-for-go to v40.3.0
- updates github.com/Azure/go-autorest to our fork containing Azure/go-autorest#512
- updates github.com/terraform-providers/terraform-provider-azuread to v0.8.0
- code changes needed for v40.3.0 of the Azure SDK - including opting into the old count 429's as requests which should be retried without adding to the total failure count

Enables #5769
Enables #5696
@tombuildsstuff
Copy link
Contributor Author

ping @jhendrixMSFT :)

@jhendrixMSFT
Copy link
Member

Sorry for the delay.

I think this is a bug in the string-to-sign implementation. We shouldn't have to special-case this based on the root, just if content-length is zero. Take a look at the implementation in the legacy storage package as it works as expected.

Copy link
Member

@jhendrixMSFT jhendrixMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also update version.go and CHANGELOG please.

autorest/azure/auth/go.mod Outdated Show resolved Hide resolved
@@ -35,6 +35,9 @@ const (
// SharedKey is used to authorize against blobs, files and queues services.
SharedKey SharedKeyType = "sharedKey"

// SharedKey is used to authorize against the account.
SharedKeyForAccount SharedKeyType = "sharedKeyAccount"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be removed, just add content length in the string-to-sign input if it's not equal to zero.

In an ideal world this wouldn't be necessary, but I digress.

Unfortunately it appears that the Accounts Data Plane API uses a different method to calculate
the shared key authorization verses other (blob etc) operations which support this auth method.

This commit introduces support for a "from account" type which includes support for this auth
token, so that this authorizer can be used for those endpoints.
Ensure path is never empty when building canonicalilzed resource.
@jhendrixMSFT
Copy link
Member

@tombuildsstuff terribly sorry for the delay on this. I dug into it further, the canonicalized resource requires that the root path is never empty. I adapted the fix from azure-storage-blob-go. It's handled slightly different in the legacy storage package but the end result is the same. I've verified with giovanni that this resolves the issue.

@jhendrixMSFT jhendrixMSFT merged commit 862055f into Azure:master Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants