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

KeyVault: Feature/update security domain spec #12863

Merged
merged 11 commits into from
Feb 9, 2021

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,45 @@
"application/json"
],
"paths": {
"/securitydomain/download/pending": {
"get": {
"tags": [
"HSMSecurityDomain"
],
"operationId": "HSMSecurityDomain_DownloadPending",
"description": "Retrieves the Security Domain download operation status",
"responses": {
"200": {
"description": "Security Domain download operation status",
"schema": {
"$ref": "#/definitions/SecurityDomainOperationStatus"
}
},
"default": {
"description": "Key Vault error response describing why the operation failed.",
"schema": {
"$ref": "common.json#/definitions/KeyVaultError"
}
}
},
"x-ms-examples": {
"Find Security Domain download operation status": {
"$ref": "./examples/securitydomainoperationstatus-example.json"
}
}
}
},
"/securitydomain/download": {
"post": {
"tags": [
"HSMSecurityDomain"
],
"x-ms-long-running-operation": true,
"x-ms-long-running-operation-options": {
"final-state-via": "azure-async-operation"
},
"operationId": "HSMSecurityDomain_Download",
"description": "Retrieves Security domain from HSM enclave",
"description": "Retrieves the Security Domain from the managed HSM. Calling this endpoint can be used to activate a provisioned managed HSM resource.",
"parameters": [
{
"in": "body",
Expand All @@ -42,15 +74,31 @@
"schema": {
"$ref": "#/definitions/CertificateInfoObject"
},
"description": "Security domain download operation requires customer to provide N certificates (minimum 3 and maximum 10) containing public key in JWK format."
"description": "The Security Domain download operation requires customer to provide N certificates (minimum 3 and maximum 10) containing a public key in JWK format."
},
{
"$ref": "#/parameters/ApiVersionParameter"
}
],
"responses": {
"202": {
"description": "The response contains the Security Domain that is being confirmed.",
"schema": {
"$ref": "#/definitions/SecurityDomainObject"
},
"headers": {
"Retry-After": {
"description": "The recommended number of seconds to wait before calling the URI specified in Azure-AsyncOperation.",
"type": "integer"
},
"Azure-AsyncOperation": {
"description": "The URI to poll for completion status.",
"type": "string"
}
}
},
"200": {
Copy link
Member

Choose a reason for hiding this comment

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

Why return both a 202 or a 200? When should the SDK - and customers - expect 1 vs. the other? This isn't consistent with other LROs in Key Vault. Looking at others, they return a 202 only that represents the operation. Polling the pending method is how status and the eventual object (security domain) is retrieved.

Copy link
Contributor Author

@docschmidt docschmidt Feb 5, 2021

Choose a reason for hiding this comment

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

When a managed HSM resource is created, the resource is in what we call the Provisioned state. At this point, customers have to call either the /securitydomain/upload endpoint to restore a Security Domain or the /securitydomain/download endpoint to confirm the Security Domain of the created pool. Both restoring and confirming are long running operations. Once complete the managed HSM goes into Active state at which point the managed HSM will accept other service calls.

Once the managed HSM is active, we want to allow customers to redownload the Security Domain, for example to rotate encryption keys. Merely downloading the Security Domain can be satisfied right away, and so the service would return 200 instead of 202. If this is an anti-pattern, please let me know. We may need to implement an additional endpoint then (or change to "get" for download only).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this can be a problem. In one case you're asking to return a long-running operation object (LRO) but in another the desired model. Even if we used a union of sorts for strongly-typed languages (.NET doesn't support this, but we could fake it) how do customers know to intuitively use one over the other?

Instead, always returning an LRO allows you to return one in an already-resolved state, which we support today.

/cc @johanste @JeffreyRichter for guidance.

Copy link
Member

Choose a reason for hiding this comment

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

Another possibility - though not ideal since we can't really rely on the generated code - is to have the SDKs always return an LRO anyway (e.g. in .NET: something deriving from Operation<T>) that could be either in a fully resolved state for HTTP 200, or act like a normal LRO for HTTP 202. We actually do something like this for delete and recover operations, which aren't declared as LROs but may take a while, so we "override" those methods to work like LROs even if they are already completed. (Also, the REST APIs themselves return the model right away regardless of whether the operation is done, so we already have the model.)

Copy link
Member

Choose a reason for hiding this comment

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

The service should not flip-flop between 200 and 202 responses. It should pick one pattern and stick with it. It becomes challenging for clients to have to fork their code paths depending on if the request could be satisfied "synchronously" or not.

"description": "This is the specification of the security domain as downloaded from the new pool",
"description": "The response contains the confirmed Security Domain.",
"schema": {
"$ref": "#/definitions/SecurityDomainObject"
}
Expand All @@ -63,7 +111,7 @@
}
},
"x-ms-examples": {
"Retrieve Security domain": {
"Retrieve Security Domain": {
"$ref": "./examples/securitydomaindownloadpost-example.json"
}
}
Expand All @@ -80,10 +128,10 @@
}
],
"operationId": "HSMSecurityDomain_TransferKey",
"description": "Retrieve security domain transfer key",
"description": "Retrieve Security Domain transfer key",
"responses": {
"200": {
"description": "Security domain transfer key operation",
"description": "Security Domain transfer key response",
"schema": {
"$ref": "#/definitions/TransferKey"
}
Expand All @@ -96,7 +144,7 @@
}
},
"x-ms-examples": {
"Retrieve security domain transfer key": {
"Retrieve Security Domain transfer key": {
"$ref": "./examples/securitydomaintransferkey-example.json"
}
}
Expand All @@ -112,21 +160,21 @@
"x-ms-long-running-operation-options": {
"final-state-via": "azure-async-operation"
},
"description": "Request Security domain upload operation",
"description": "Restore the provided Security Domain.",
"parameters": [
{
"in": "body",
"name": "security_domain",
"description": "security domain",
"description": "The Security Domain to be restored.",
"required": true,
"schema": {
"$ref": "#/definitions/SecurityDomainUploadObject"
"$ref": "#/definitions/SecurityDomainObject"
}
}
],
"responses": {
"202": {
"description": "Security domain upload operation started",
"description": "Restore of the Security Domain started.",
"headers": {
"Retry-After": {
"description": "The recommended number of seconds to wait before calling the URI specified in Azure-AsyncOperation.",
Expand All @@ -141,6 +189,9 @@
"$ref": "#/definitions/SecurityDomainOperationStatus"
}
},
"204": {
"description": "Final response"
},
"default": {
"description": "Key Vault error response describing why the operation failed.",
"schema": {
Expand All @@ -149,7 +200,7 @@
}
},
"x-ms-examples": {
"Security domain upload operation": {
"Security Domain upload operation": {
"$ref": "./examples/securitydomainuploadoperation-example.json"
}
}
Expand All @@ -161,10 +212,10 @@
"HSMSecurityDomain"
],
"operationId": "HSMSecurityDomain_UploadPending",
"description": "Get Security domain upload operation status",
"description": "Get Security Domain upload operation status",
"responses": {
"200": {
"description": "Security domain upload operation status",
"description": "Security Domain upload operation status",
"schema": {
"$ref": "#/definitions/SecurityDomainOperationStatus"
}
Expand All @@ -177,7 +228,7 @@
}
},
"x-ms-examples": {
"Find security domain upload operation status": {
"Find Security Domain upload operation status": {
"$ref": "./examples/securitydomainoperationstatus-example.json"
}
}
Expand All @@ -198,7 +249,7 @@
"description": "Certificates needed from customer"
},
"required": {
"description": "Customer to specify the number of certificates (minimum 2 and maximum 10) to restore security domain",
"description": "Customer to specify the number of certificates (minimum 2 and maximum 10) to restore Security Domain",
"type": "integer",
"default": 2,
"minimum": 2,
Expand Down Expand Up @@ -238,112 +289,15 @@
},
"SecurityDomainObject": {
"properties": {
"data": {
"type": "object",
"properties": {
"EncData": {
"$ref": "#/definitions/EncDataSet",
"description": "Array of encrypted data set"
},
"SharedKeys": {
"properties": {
"key_algorithm": {
"type": "string",
"default": "shamir_share",
"description": "The Algorithm used for shared keys"
},
"required": {
"type": "integer",
"minimum": 2,
"maximum": 10,
"description": "The number of keys (minimum 2 and maximum 10) required for security domain. "
},
"enc_shares": {
"type": "array",
"items": {
"$ref": "#/definitions/Key",
"minItems": 3,
"maxItems": 10
},
"uniqueItems": true,
"description": "Compact JWE wrapped shares array"
}
},
"required": [
"key_algorithm",
"required",
"enc_shares"
],
"description": "Array of shared keys"
},
"version": {
"type": "integer"
}
},
"required": [
"EncData",
"SharedKeys",
"version"
]
}
},
"description": "Security domain",
"required": [
"data"
]
},
"EncDataSet": {
"properties": {
"data": {
"type": "array",
"items": {
"$ref": "#/definitions/EncDataSetItem",
"minItems": 2
},
"description": "Array of encrypted security domain",
"uniqueItems": true
},
"kdf": {
"type": "string",
"default": "sp108_kdf",
"description": "The key derivation function used"
}
},
"required": [
"data",
"kdf"
]
},
"EncDataSetItem": {
"properties": {
"compact_jwe": {
"type": "string",
"description": "Encrypted data"
},
"tag": {
"type": "string",
"description": "hsm backup tag"
}
},
"required": [
"compact_jwe",
"tag"
]
},
"Key": {
"properties": {
"enc_key": {
"type": "string",
"description": "Compact JWE wrapped share"
},
"x5t_256": {
"value": {
"type": "string",
"description": "SHA 256 hash of certificate"
"format": "base64url",
"description": "A blob containing the Security Domain."
Copy link
Member

Choose a reason for hiding this comment

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

Like the security policy for key export/release, should this have some sort of contentType to inform the customer what it is or how to store it or, more importantly, that needs to be sent back as-is for compatibility? Or will this opaque blob always work (or maybe even contain an embedded version)?

/cc @herveyw-msft

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The opaque blob contains a version number and it is our intent that the opaque blob will always work.

}
},
"description": "The Security Domain.",
"required": [
"enc_key",
"x5t_256"
"value"
]
},
"SecurityDomainOperationStatus": {
Expand Down Expand Up @@ -374,7 +328,7 @@
},
"kty": {
"type": "string",
"description": "JsonWebKey Key Type (kty), as defined in https://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-40. For security domain this value must be RSA"
"description": "JsonWebKey Key Type (kty), as defined in https://tools.ietf.org/html/draft-ietf-jose-json-web-algorithms-40. For Security Domain this value must be RSA."
},
"key_ops": {
"type": "array",
Expand Down Expand Up @@ -425,44 +379,6 @@
"n",
"e"
]
},
"SecurityDomainUploadObject": {
"properties": {
"value": {
"type": "object",
"properties": {
"EncData": {
"$ref": "#/definitions/EncDataSet",
"description": "Array of encrypted data set"
},
"WrappedKey": {
"properties": {
"enc_key": {
"type": "string",
"description": "Encryption key used to encrypt the EncData"
},
"x5t_256": {
"type": "string",
"description": "Thumbprint used to determine which certificate was used to encrypt the enc_key field"
}
},
"required": [
"enc_key",
"x5t_256"
],
"description": "Key object containing the encryption key used to encrypt EncData object"
}
},
"required": [
"EncData",
"WrappedKey"
]
}
},
"description": "Security domain object uploaded to a new pool",
"required": [
"value"
]
}
},
"parameters": {
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Loading