-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
New Data Source: azurerm_storage_account_sas
#1011
Changes from all commits
4fa76c4
048bb76
d9d1dc2
d59dfb1
109629b
27e8d5a
c19295d
a1f12ee
66c8aca
d16f5df
6088083
bcbf6fb
0eace11
5bc7264
71350a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,389 @@ | ||
package azurerm | ||
|
||
import "github.com/hashicorp/terraform/helper/schema" | ||
|
||
import ( | ||
"encoding/base64" | ||
"fmt" | ||
"net/url" | ||
"strings" | ||
|
||
"crypto/hmac" | ||
"crypto/sha256" | ||
"encoding/hex" | ||
) | ||
|
||
const ( | ||
connStringAccountKeyKey = "AccountKey" | ||
connStringAccountNameKey = "AccountName" | ||
sasSignedVersion = "2017-07-29" | ||
) | ||
|
||
// This is an ACCOUNT SAS : https://docs.microsoft.com/en-us/rest/api/storageservices/Constructing-an-Account-SAS | ||
// not Service SAS | ||
func dataSourceArmStorageAccountSharedAccessSignature() *schema.Resource { | ||
return &schema.Resource{ | ||
Read: dataSourceArmStorageAccountSasRead, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"connection_string": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
Sensitive: true, | ||
}, | ||
|
||
"https_only": { | ||
Type: schema.TypeBool, | ||
Optional: true, | ||
Default: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"resource_types": { | ||
Type: schema.TypeList, | ||
Required: true, | ||
ForceNew: true, | ||
MaxItems: 1, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"service": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"container": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"object": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
|
||
"services": { | ||
Type: schema.TypeList, | ||
Required: true, | ||
ForceNew: true, | ||
MaxItems: 1, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"blob": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"queue": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"table": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"file": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think each of these properties also needs |
||
ForceNew: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
|
||
// Always in UTC and must be ISO-8601 format | ||
"start": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want to add some validation to ensure this is formatted correctly? We've got a method which does this for RFC3339 dates here: https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/validators.go#L13 - which can be used like so:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same notes as above. |
||
}, | ||
|
||
// Always in UTC and must be ISO-8601 format | ||
"expiry": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (same here) do we want to add some validation to ensure this is formatted correctly? We've got a method which does this for RFC3339 dates here: https://github.com/terraform-providers/terraform-provider-azurerm/blob/master/azurerm/validators.go#L13 - which can be used like so:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to go the route you specify with the validation function. In my research there are some slight differences between ISO-8601 and RFC3339. MSFT specifies that these are ISO-8601 and since the signatures are based on the actual text characters, the subtle differences may result in breakage/no-working. Given the fragile nature, by design, of the signatures, I think leaving as-is is preferable rather than constraining the input to possibly broken validation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes sense from my side - we can always add it in later based on feedback 👍 |
||
}, | ||
|
||
"permissions": { | ||
Type: schema.TypeList, | ||
Required: true, | ||
ForceNew: true, | ||
MaxItems: 1, | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"read": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"write": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"delete": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"list": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"add": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"create": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"update": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"process": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
ForceNew: true, | ||
}, | ||
}, | ||
}, | ||
}, | ||
|
||
"sas": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 is it worth making this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not really a token in the uuid/JWT sense... its actually presented as a URL query string... with inconsistent URL escaping in the various parts. Notice only some of the fields are passed to I debated including the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
Type: schema.TypeString, | ||
Computed: true, | ||
Sensitive: true, | ||
}, | ||
}, | ||
} | ||
|
||
} | ||
|
||
func dataSourceArmStorageAccountSasRead(d *schema.ResourceData, _ interface{}) error { | ||
|
||
connString := d.Get("connection_string").(string) | ||
httpsOnly := d.Get("https_only").(bool) | ||
resourceTypesIface := d.Get("resource_types").([]interface{}) | ||
servicesIface := d.Get("services").([]interface{}) | ||
start := d.Get("start").(string) | ||
expiry := d.Get("expiry").(string) | ||
permissionsIface := d.Get("permissions").([]interface{}) | ||
|
||
resourceTypes := buildResourceTypesString(resourceTypesIface[0].(map[string]interface{})) | ||
services := buildServicesString(servicesIface[0].(map[string]interface{})) | ||
permissions := buildPermissionsString(permissionsIface[0].(map[string]interface{})) | ||
|
||
// Parse the connection string | ||
kvp, err := parseAzureStorageAccountConnectionString(connString) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Create the string to sign with the key... | ||
|
||
// Details on how to do this are here: | ||
// https://docs.microsoft.com/en-us/rest/api/storageservices/Constructing-an-Account-SAS | ||
accountName := kvp[connStringAccountNameKey] | ||
accountKey := kvp[connStringAccountKeyKey] | ||
var signedProtocol = "https,http" | ||
if httpsOnly { | ||
signedProtocol = "https" | ||
} | ||
signedIp := "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we also expose this value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I debated this. The format is a little odd of the field and it's not all that functionally useful. I say we omit it for now and if anyone asks for this added feature we can add it later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 works for me |
||
signedVersion := sasSignedVersion | ||
|
||
sasToken, err := computeAzureStorageAccountSas(accountName, accountKey, permissions, services, resourceTypes, | ||
start, expiry, signedProtocol, signedIp, signedVersion) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d.Set("sas", sasToken) | ||
tokenHash := sha256.Sum256([]byte(sasToken)) | ||
d.SetId(hex.EncodeToString(tokenHash[:])) | ||
|
||
return nil | ||
} | ||
|
||
func buildPermissionsString(perms map[string]interface{}) string { | ||
retVal := "" | ||
|
||
if val, pres := perms["read"].(bool); pres && val { | ||
retVal += "r" | ||
} | ||
|
||
if val, pres := perms["write"].(bool); pres && val { | ||
retVal += "w" | ||
} | ||
|
||
if val, pres := perms["delete"].(bool); pres && val { | ||
retVal += "d" | ||
} | ||
|
||
if val, pres := perms["list"].(bool); pres && val { | ||
retVal += "l" | ||
} | ||
|
||
if val, pres := perms["add"].(bool); pres && val { | ||
retVal += "a" | ||
} | ||
|
||
if val, pres := perms["create"].(bool); pres && val { | ||
retVal += "c" | ||
} | ||
|
||
if val, pres := perms["update"].(bool); pres && val { | ||
retVal += "u" | ||
} | ||
|
||
if val, pres := perms["process"].(bool); pres && val { | ||
retVal += "p" | ||
} | ||
|
||
return retVal | ||
} | ||
|
||
func buildServicesString(services map[string]interface{}) string { | ||
retVal := "" | ||
|
||
if val, pres := services["blob"].(bool); pres && val { | ||
retVal += "b" | ||
} | ||
|
||
if val, pres := services["queue"].(bool); pres && val { | ||
retVal += "q" | ||
} | ||
|
||
if val, pres := services["table"].(bool); pres && val { | ||
retVal += "t" | ||
} | ||
|
||
if val, pres := services["file"].(bool); pres && val { | ||
retVal += "f" | ||
} | ||
|
||
return retVal | ||
} | ||
|
||
func buildResourceTypesString(resTypes map[string]interface{}) string { | ||
retVal := "" | ||
|
||
if val, pres := resTypes["service"].(bool); pres && val { | ||
retVal += "s" | ||
} | ||
|
||
if val, pres := resTypes["container"].(bool); pres && val { | ||
retVal += "c" | ||
} | ||
|
||
if val, pres := resTypes["object"].(bool); pres && val { | ||
retVal += "o" | ||
} | ||
|
||
return retVal | ||
} | ||
|
||
func computeAzureStorageAccountSas(accountName string, | ||
accountKey string, | ||
permissions string, | ||
services string, | ||
resourceTypes string, | ||
start string, | ||
expiry string, | ||
signedProtocol string, | ||
signedIp string, | ||
signedVersion string) (string, error) { | ||
|
||
// UTF-8 by default... | ||
stringToSign := accountName + "\n" | ||
stringToSign += permissions + "\n" | ||
stringToSign += services + "\n" | ||
stringToSign += resourceTypes + "\n" | ||
stringToSign += start + "\n" | ||
stringToSign += expiry + "\n" | ||
stringToSign += signedIp + "\n" | ||
stringToSign += signedProtocol + "\n" | ||
stringToSign += signedVersion + "\n" | ||
|
||
binaryKey, err := base64.StdEncoding.DecodeString(accountKey) | ||
if err != nil { | ||
return "", err | ||
} | ||
hasher := hmac.New(sha256.New, binaryKey) | ||
hasher.Write([]byte(stringToSign)) | ||
signature := hasher.Sum(nil) | ||
|
||
// Trial and error to determine which fields the Azure portal | ||
// URL encodes for a query string and which it does not. | ||
sasToken := "?sv=" + url.QueryEscape(signedVersion) | ||
sasToken += "&ss=" + url.QueryEscape(services) | ||
sasToken += "&srt=" + url.QueryEscape(resourceTypes) | ||
sasToken += "&sp=" + url.QueryEscape(permissions) | ||
sasToken += "&se=" + (expiry) | ||
sasToken += "&st=" + (start) | ||
sasToken += "&spr=" + (signedProtocol) | ||
|
||
// this is consistent with how the Azure portal builds these. | ||
if len(signedIp) > 0 { | ||
sasToken += "&sip=" + signedIp | ||
} | ||
|
||
sasToken += "&sig=" + url.QueryEscape(base64.StdEncoding.EncodeToString(signature)) | ||
|
||
return sasToken, nil | ||
} | ||
|
||
// This connection string was for a real storage account which has been deleted | ||
// so its safe to include here for reference to understand the format. | ||
// DefaultEndpointsProtocol=https;AccountName=azurermtestsa0;AccountKey=2vJrjEyL4re2nxCEg590wJUUC7PiqqrDHjAN5RU304FNUQieiEwS2bfp83O0v28iSfWjvYhkGmjYQAdd9x+6nw==;EndpointSuffix=core.windows.net | ||
|
||
func parseAzureStorageAccountConnectionString(connString string) (map[string]string, error) { | ||
validKeys := map[string]bool{"DefaultEndpointsProtocol": true, "BlobEndpoint": true, | ||
"AccountName": true, "AccountKey": true, "EndpointSuffix": true} | ||
// The k-v pairs are separated with semi-colons | ||
tokens := strings.Split(connString, ";") | ||
|
||
kvp := make(map[string]string) | ||
|
||
for _, atoken := range tokens { | ||
// The individual k-v are separated by an equals sign. | ||
kv := strings.SplitN(atoken, "=", 2) | ||
key := kv[0] | ||
val := kv[1] | ||
if _, present := validKeys[key]; !present { | ||
return nil, fmt.Errorf("[ERROR] Unknown Key: %s", key) | ||
} | ||
kvp[key] = val | ||
} | ||
|
||
if _, present := kvp[connStringAccountKeyKey]; !present { | ||
return nil, fmt.Errorf("[ERROR] Storage Account Key not found in connection string: %s", connString) | ||
} | ||
|
||
return kvp, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to default all of these (the
resource_types
,permissions
, andservices
parameters) tofalse
and make them optional?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tombuildsstuff Agree with all of the above and will incorporate those changes.
@phekmat I debated that myself and came to the conclusion to be explicitly verbose here since there are security vs function issues here.
If we default all to false, then there's an implicit no-access (all broken) case.
IMHO it's better here to be verbose and demand that folks specify exactly what they want...
This avoids an oops by granting too much access and an oops by granting no access both of which, IMHO, are a worse UX than some extra typing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd agree with this approach for the moment - we can always change it later 👍