-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add path for retrieving the public key #5
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
package gcpkms | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/hashicorp/errwrap" | ||
"github.com/hashicorp/vault/logical" | ||
"github.com/hashicorp/vault/logical/framework" | ||
|
||
kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1" | ||
) | ||
|
||
func (b *backend) pathPubkey() *framework.Path { | ||
return &framework.Path{ | ||
Pattern: "pubkey/" + framework.GenericNameRegex("key"), | ||
|
||
HelpSynopsis: "Retrieve the public key associated with the named key", | ||
HelpDescription: ` | ||
Retrieve the PEM-encoded Google Cloud KMS public key associated with the Vault | ||
named key. The key will only be available if the key is asymmetric. | ||
`, | ||
|
||
Fields: map[string]*framework.FieldSchema{ | ||
"key": &framework.FieldSchema{ | ||
Type: framework.TypeString, | ||
Description: ` | ||
Name of the key for which to get the public key. This key must already exist in | ||
Vault and Google Cloud KMS. | ||
`, | ||
}, | ||
|
||
"key_version": &framework.FieldSchema{ | ||
Type: framework.TypeInt, | ||
Description: ` | ||
Integer version of the crypto key version from which to exact the public key. | ||
This field is required. | ||
`, | ||
}, | ||
}, | ||
|
||
Callbacks: map[logical.Operation]framework.OperationFunc{ | ||
logical.ReadOperation: withFieldValidator(b.pathPubkeyRead), | ||
}, | ||
} | ||
} | ||
|
||
// pathPubkeyRead corresponds to GET gcpkms/pubkey/:key and is used to read the | ||
// public key contents of the crypto key version. | ||
func (b *backend) pathPubkeyRead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) { | ||
key := d.Get("key").(string) | ||
keyVersion := d.Get("key_version").(int) | ||
|
||
if keyVersion == 0 { | ||
return nil, errMissingFields("key_version") | ||
} | ||
|
||
k, err := b.Key(ctx, req.Storage, key) | ||
if err != nil { | ||
if err == ErrKeyNotFound { | ||
return logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest | ||
} | ||
return nil, err | ||
} | ||
|
||
if k.MinVersion > 0 && keyVersion < k.MinVersion { | ||
resp := fmt.Sprintf("requested version %d is less than minimum allowed version of %d", | ||
keyVersion, k.MinVersion) | ||
return logical.ErrorResponse(resp), logical.ErrPermissionDenied | ||
} | ||
|
||
if k.MaxVersion > 0 && keyVersion > k.MaxVersion { | ||
resp := fmt.Sprintf("requested version %d is greater than maximum allowed version of %d", | ||
keyVersion, k.MaxVersion) | ||
return logical.ErrorResponse(resp), logical.ErrPermissionDenied | ||
} | ||
|
||
kmsClient, closer, err := b.KMSClient(req.Storage) | ||
if err != nil { | ||
return nil, err | ||
} | ||
defer closer() | ||
|
||
pk, err := kmsClient.GetPublicKey(ctx, &kmspb.GetPublicKeyRequest{ | ||
Name: fmt.Sprintf("%s/cryptoKeyVersions/%d", k.CryptoKeyID, keyVersion), | ||
}) | ||
if err != nil { | ||
return nil, errwrap.Wrapf("failed to get public key: {{err}}", err) | ||
} | ||
|
||
return &logical.Response{ | ||
Data: map[string]interface{}{ | ||
"pem": pk.Pem, | ||
"algorithm": algorithmToString(pk.Algorithm), | ||
}, | ||
}, nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
package gcpkms | ||
|
||
import ( | ||
"context" | ||
"crypto/x509" | ||
"encoding/pem" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/hashicorp/vault/logical" | ||
|
||
kmspb "google.golang.org/genproto/googleapis/cloud/kms/v1" | ||
) | ||
|
||
func TestPathPubkey_Read(t *testing.T) { | ||
t.Parallel() | ||
|
||
t.Run("field_validation", func(t *testing.T) { | ||
t.Parallel() | ||
testFieldValidation(t, logical.ReadOperation, "pubkey/my-key") | ||
}) | ||
|
||
t.Run("asymmetric_decrypt", func(t *testing.T) { | ||
t.Parallel() | ||
|
||
algorithms := []kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm{ | ||
kmspb.CryptoKeyVersion_RSA_DECRYPT_OAEP_2048_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_DECRYPT_OAEP_3072_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_DECRYPT_OAEP_4096_SHA256, | ||
} | ||
|
||
for _, algo := range algorithms { | ||
algo := algo | ||
name := strings.ToLower(algo.String()) | ||
|
||
t.Run(name, func(t *testing.T) { | ||
t.Parallel() | ||
|
||
cryptoKey, cleanup := testCreateKMSCryptoKeyAsymmetricDecrypt(t, algo) | ||
defer cleanup() | ||
|
||
b, storage := testBackend(t) | ||
|
||
ctx := context.Background() | ||
if err := storage.Put(ctx, &logical.StorageEntry{ | ||
Key: "keys/my-key", | ||
Value: []byte(`{"name":"my-key", "crypto_key_id":"` + cryptoKey + `"}`), | ||
}); err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Get the public key | ||
resp, err := b.HandleRequest(ctx, &logical.Request{ | ||
Storage: storage, | ||
Operation: logical.ReadOperation, | ||
Path: "pubkey/my-key", | ||
Data: map[string]interface{}{ | ||
"key_version": 1, | ||
}, | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Verify it's a pem public key (this is kinda testing KMS, but it's | ||
// a good test to ensure the API doesn't change). | ||
pemStr, ok := resp.Data["pem"].(string) | ||
if !ok { | ||
t.Fatal("missing pem") | ||
} | ||
|
||
// Extract the PEM-encoded data block | ||
block, _ := pem.Decode([]byte(pemStr)) | ||
if block == nil { | ||
t.Fatalf("not pem: %s", pemStr) | ||
} | ||
|
||
// Decode the public key | ||
if _, err := x509.ParsePKIXPublicKey(block.Bytes); err != nil { | ||
t.Fatal(err) | ||
} | ||
}) | ||
} | ||
}) | ||
|
||
t.Run("asymmetric_sign", func(t *testing.T) { | ||
t.Parallel() | ||
|
||
algorithms := []kmspb.CryptoKeyVersion_CryptoKeyVersionAlgorithm{ | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PSS_2048_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PSS_3072_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PSS_4096_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_2048_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_3072_SHA256, | ||
kmspb.CryptoKeyVersion_RSA_SIGN_PKCS1_4096_SHA256, | ||
kmspb.CryptoKeyVersion_EC_SIGN_P256_SHA256, | ||
kmspb.CryptoKeyVersion_EC_SIGN_P384_SHA384, | ||
} | ||
|
||
for _, algo := range algorithms { | ||
algo := algo | ||
name := strings.ToLower(algo.String()) | ||
|
||
t.Run(name, func(t *testing.T) { | ||
t.Parallel() | ||
|
||
cryptoKey, cleanup := testCreateKMSCryptoKeyAsymmetricSign(t, algo) | ||
defer cleanup() | ||
|
||
b, storage := testBackend(t) | ||
|
||
ctx := context.Background() | ||
if err := storage.Put(ctx, &logical.StorageEntry{ | ||
Key: "keys/my-key", | ||
Value: []byte(`{"name":"my-key", "crypto_key_id":"` + cryptoKey + `"}`), | ||
}); err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Get the public key | ||
resp, err := b.HandleRequest(ctx, &logical.Request{ | ||
Storage: storage, | ||
Operation: logical.ReadOperation, | ||
Path: "pubkey/my-key", | ||
Data: map[string]interface{}{ | ||
"key_version": 1, | ||
}, | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Verify it's a pem public key (this is kinda testing KMS, but it's | ||
// a good test to ensure the API doesn't change). | ||
pemStr, ok := resp.Data["pem"].(string) | ||
if !ok { | ||
t.Fatal("missing pem") | ||
} | ||
|
||
// Extract the PEM-encoded data block | ||
block, _ := pem.Decode([]byte(pemStr)) | ||
if block == nil { | ||
t.Fatalf("not pem: %s", pemStr) | ||
} | ||
|
||
// Decode the public key | ||
if _, err := x509.ParsePKIXPublicKey(block.Bytes); err != nil { | ||
t.Fatal(err) | ||
} | ||
}) | ||
} | ||
}) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -258,6 +258,65 @@ $ curl \ | |
} | ||
``` | ||
|
||
## Get Public Key | ||
|
||
This endpoint uses the named encryption key to retrieve the PEM-encoded contents | ||
of the corresponding public key on Google Cloud KMS. This only applies to | ||
asymmetric key types. | ||
|
||
| Method | Path | Produces | | ||
| :------- | :--------------------------| :------------------------ | | ||
| `GET` | `gcpkms/pubkey/:key` | `200 application/json` | | ||
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. Change to POST if the code review comments re: Update are made. Your curl example is already correct though 🙂. |
||
|
||
### Example Policy | ||
|
||
```hcl | ||
path "gcpkms/pubkey/my-key" { | ||
capabilities = ["read"] | ||
} | ||
``` | ||
|
||
### Parameters | ||
|
||
- `key` (`string: <required>`) - | ||
Name of the key in Vault for which to retrieve the public key. This key must | ||
already exist in Vault and must map back to a Google Cloud KMS key. This is | ||
specified as part of the URL. | ||
|
||
- `key_version` (`int: <required>`) - | ||
Integer version of the crypto key version for which to retrieve the public key. | ||
This field is required. | ||
|
||
|
||
### Sample Payload | ||
|
||
```json | ||
{ | ||
"key_version": 1 | ||
} | ||
``` | ||
|
||
### Sample Request | ||
|
||
```text | ||
$ curl \ | ||
--header "X-Vault-Token: ..." \ | ||
--request POST \ | ||
--data @payload.json \ | ||
https://127.0.0.1:8200/v1/gcpkms/pubkey/my-key | ||
``` | ||
|
||
### Sample Response | ||
|
||
```json | ||
{ | ||
"data": { | ||
"pem": "----BEGIN PUBLIC KEY-----\nMIICIjANBgkq...", | ||
"algorithm": "rsa_decrypt_oaep_4096_sha256" | ||
} | ||
} | ||
``` | ||
|
||
## Re-Encrypt Existing Ciphertext | ||
|
||
This endpoint uses the named encryption key to re-encrypt the underlying | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should this be an Update operation instead since you're requiring
key_version
?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.
Hmm I actually think it should be GET. It's possible to have params for get requests (they turn into URL params). I'm going to fix the curl command because
vault write gcpkms/pubkey/my-key key_verison=1
doesn't feel right.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 agree that read feels correct, though as is now the
key_version
would be a query parameter which is uncommon in Vault (AFAIK), and a little odd with it being required. Maybe it could be a path element, which is a pattern that would match: https://www.vaultproject.io/api/secret/transit/index.html#export-key@briankassouf Might have some thoughts on the API conventions here.
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.
As in
vault read gcpkms/pubkey/my-key/1
? I'm not opposed to that, but can @briankassouf weight in quickly before I make that change?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.
Using a GET parameter is fine, it's not a common pattern because it wasn't part of our framework implementation until recently. The kv version 2 API uses get parameters to retrieve a specific version of a secret. This aligns with that same usecase.
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.
Okay let's keep it as-is then 😄- thanks bk. @kalafut what other changes do we need to merge this
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.
Hey @sethvargo one last question. The KVV2 API defaults to the latest key version if
version
isn't provided. Does that same concept make sense here? I briefly skimmed the Google docs but it wasn't totally clear to me.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 thought about it, but it would involve an extra API call (we'd have to list the key versions from the KMS api and grab the latest one). Two problems arise:
That's why I opted to keep it required.