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.

Sign up for GitHub

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

Jump to bottom

Vault-16367: add support RSA padding scheme pkcs1v15 for encryption #16368

Closed
wants to merge 14 commits into from
+190 −18
Closed

Vault-16367: add support RSA padding scheme pkcs1v15 for encryption #16368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
381d091
[transit-pkcs1v15] transit support for the pkcs1v15 padding scheme – …
marcellanz Jul 19, 2022
e61c72f
Merge branch 'hashicorp:main' into feature/transit_pkcs1v15_support
marcellanz Jul 19, 2022
6d92f49
Merge branch 'hashicorp:main' into feature/transit_pkcs1v15_support
marcellanz Jul 19, 2022
9b57e68
Merge branch 'hashicorp:main' into feature/transit_pkcs1v15_support
marcellanz Jul 19, 2022
91d11dc
[transit-pkcs1v15] renamed padding_scheme parameter in transit docume…
marcellanz Jul 19, 2022
cbc17d1
Merge remote-tracking branch 'upstream/feature/transit_pkcs1v15_suppo…
marcellanz Jul 19, 2022
31b0c51
[transit-pkcs1v15] add changelog file.
marcellanz Jul 19, 2022
8a7728d
[transit-pkcs1v15] remove the algorithm path as padding_scheme is cho…
marcellanz Jul 19, 2022
99a82d4
Update ui/app/templates/components/transit-key-action/datakey.hbs
marcellanz Aug 16, 2023
a19b563
Update ui/app/templates/components/transit-key-action/datakey.hbs
marcellanz Aug 16, 2023
efa5fb1
Update ui/app/templates/components/transit-key-action/datakey.hbs
marcellanz Aug 16, 2023
9b56e02
Update website/content/api-docs/secret/transit.mdx
marcellanz Nov 2, 2023
8f79fc0
Update website/content/api-docs/secret/transit.mdx
marcellanz Nov 2, 2023
ee4a3c3
Update website/content/api-docs/secret/transit.mdx
marcellanz Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion builtin/logical/transit/path_datakey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ func (b *backend) pathDatakey() *framework.Path {
ciphertext; "wrapped" will return the ciphertext only.`,
},

"padding_scheme": {
Type: framework.TypeString,
Description: `The padding scheme to use for decrypt. Currently only applies to RSA key types.
Options are 'oaep' or 'pkcs1v15'. Defaults to 'oaep'`,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be great to include Default: "oaep" here for consistency if we could :-) When you call d.Get(...) below, it'll populate it with oaep.

Copy link
Contributor

Choose a reason for hiding this comment

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

This applies in a few places as well below :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

If I were to nitpick, for consistency with sign/verify I might suggest calling this encryption_algorithm, as it'll likely be more widely used than just RSA here. padding_scheme is definitely more precise, but boxes us into a corner w.r.t. EC algorithms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might suggest calling this encryption_algorithm

I think too, this makes a lot of sense and is consistent with the existing API.


"context": {
Type: framework.TypeString,
Description: "Context for key derivation. Required for derived keys.",
Expand Down Expand Up @@ -131,7 +137,8 @@ func (b *backend) pathDatakeyWrite(ctx context.Context, req *logical.Request, d
return nil, err
}

ciphertext, err := p.Encrypt(ver, context, nonce, base64.StdEncoding.EncodeToString(newKey))
paddingScheme := d.Get("padding_scheme").(string)
ciphertext, err := p.Encrypt(ver, context, nonce, base64.StdEncoding.EncodeToString(newKey), paddingScheme)
if err != nil {
switch err.(type) {
case errutil.UserError:
Expand Down
9 changes: 8 additions & 1 deletion builtin/logical/transit/path_decrypt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ func (b *backend) pathDecrypt() *framework.Path {
The ciphertext to decrypt, provided as returned by encrypt.`,
},

"padding_scheme": {
Type: framework.TypeString,
Description: `The padding scheme to use for decrypt. Currently only applies to RSA key types.
Options are 'oaep' or 'pkcs1v15'. Defaults to 'oaep'`,
},

"context": {
Type: framework.TypeString,
Description: `
Expand Down Expand Up @@ -147,7 +153,8 @@ func (b *backend) pathDecryptWrite(ctx context.Context, req *logical.Request, d
continue
}

plaintext, err := p.Decrypt(item.DecodedContext, item.DecodedNonce, item.Ciphertext)
paddingScheme := d.Get("padding_scheme").(string)
plaintext, err := p.Decrypt(item.DecodedContext, item.DecodedNonce, item.Ciphertext, paddingScheme)
if err != nil {
switch err.(type) {
case errutil.InternalError:
Expand Down
9 changes: 8 additions & 1 deletion builtin/logical/transit/path_encrypt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ func (b *backend) pathEncrypt() *framework.Path {
Description: "Base64 encoded plaintext value to be encrypted",
},

"padding_scheme": {
Type: framework.TypeString,
Description: `The padding scheme to use for decrypt. Currently only applies to RSA key types.
Options are 'oaep' or 'pkcs1v15'. Defaults to 'oaep'`,
},

"context": {
Type: framework.TypeString,
Description: "Base64 encoded context for key derivation. Required if key derivation is enabled",
Expand Down Expand Up @@ -384,7 +390,8 @@ func (b *backend) pathEncryptWrite(ctx context.Context, req *logical.Request, d
warnAboutNonceUsage = true
}

ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, item.Plaintext)
paddingScheme := d.Get("padding_scheme").(string)
ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, item.Plaintext, paddingScheme)
if err != nil {
switch err.(type) {
case errutil.InternalError:
Expand Down
11 changes: 9 additions & 2 deletions builtin/logical/transit/path_rewrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ func (b *backend) pathRewrap() *framework.Path {
Description: "Ciphertext value to rewrap",
},

"padding_scheme": {
Type: framework.TypeString,
Description: `The padding scheme to use for decrypt. Currently only applies to RSA key types.
Options are 'oaep' or 'pkcs1v15'. Defaults to 'oaep'`,
},

"context": {
Type: framework.TypeString,
Description: "Base64 encoded context for key derivation. Required for derived keys.",
Expand Down Expand Up @@ -135,7 +141,8 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
continue
}

plaintext, err := p.Decrypt(item.DecodedContext, item.DecodedNonce, item.Ciphertext)
paddingScheme := d.Get("padding_scheme").(string)
plaintext, err := p.Decrypt(item.DecodedContext, item.DecodedNonce, item.Ciphertext, paddingScheme)
if err != nil {
switch err.(type) {
case errutil.UserError:
Expand All @@ -151,7 +158,7 @@ func (b *backend) pathRewrapWrite(ctx context.Context, req *logical.Request, d *
warnAboutNonceUsage = true
}

ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, plaintext)
ciphertext, err := p.Encrypt(item.KeyVersion, item.DecodedContext, item.DecodedNonce, plaintext, paddingScheme)
if err != nil {
switch err.(type) {
case errutil.UserError:
Expand Down
3 changes: 3 additions & 0 deletions changelog/16368.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
secrets/transit: Add support for RSA padding scheme pkcs1v15 for encryption
```
4 changes: 2 additions & 2 deletions sdk/helper/keysutil/encrypted_key_storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (s *encryptedKeyStorage) List(ctx context.Context, prefix string) ([]string
}

// Decrypt the data with the object's key policy.
encodedPlaintext, err := s.policy.Decrypt(context, nil, string(decoded[:]))
encodedPlaintext, err := s.policy.Decrypt(context, nil, string(decoded[:]), "")
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -268,7 +268,7 @@ func (s *encryptedKeyStorage) encryptPath(path string) (string, error) {
context := strings.TrimSuffix(s.prefix, "/")
for _, p := range parts {
encoded := base64.StdEncoding.EncodeToString([]byte(p))
ciphertext, err := s.policy.Encrypt(0, []byte(context), nil, encoded)
ciphertext, err := s.policy.Encrypt(0, []byte(context), nil, encoded, "")
if err != nil {
return "", err
}
Expand Down
29 changes: 25 additions & 4 deletions sdk/helper/keysutil/policy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ func (p *Policy) convergentVersion(ver int) int {
return convergentVersion
}

func (p *Policy) Encrypt(ver int, context, nonce []byte, value string) (string, error) {
func (p *Policy) Encrypt(ver int, context, nonce []byte, value string, paddingScheme string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I should double check I haven't violated this myself recently, but in general, we wish to avoid modifying these functions as they lie in our public sdk/ space.

I think you could change this to Encrypt->EncryptWithAlgorithm and have the former call the latter with oaep by default for consistency perhaps...?

Then if/when we introduce ECEIS based schemes, I think it might make the most sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes perfectly sense for me too. Does the new EncryptWithAlgorithm have to be documented somewhere then I assume?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I don't think our SDK has much documentation sadly. I think it'd be cool to get better inline godoc in the future, but we don't have much presently.

(If you want to work on this, happy to review such PRs in the future too!)

if !p.Type.EncryptionSupported() {
return "", errutil.UserError{Err: fmt.Sprintf("message encryption not supported for key type %v", p.Type)}
}
Expand Down Expand Up @@ -894,8 +894,19 @@ func (p *Policy) Encrypt(ver int, context, nonce []byte, value string) (string,
if err != nil {
return "", err
}
if paddingScheme == "" {
paddingScheme = "oaep"
}
key := keyEntry.RSAKey
ciphertext, err = rsa.EncryptOAEP(sha256.New(), rand.Reader, &key.PublicKey, plaintext, nil)
switch paddingScheme {
case "pkcs1v15":
ciphertext, err = rsa.EncryptPKCS1v15(rand.Reader, &key.PublicKey, plaintext)
case "oaep":
ciphertext, err = rsa.EncryptOAEP(sha256.New(), rand.Reader, &key.PublicKey, plaintext, nil)
default:
return "", errutil.InternalError{Err: fmt.Sprintf("unsupported RSA padding scheme %s", paddingScheme)}
}

if err != nil {
return "", errutil.InternalError{Err: fmt.Sprintf("failed to RSA encrypt the plaintext: %v", err)}
}
Expand All @@ -913,7 +924,7 @@ func (p *Policy) Encrypt(ver int, context, nonce []byte, value string) (string,
return encoded, nil
}

func (p *Policy) Decrypt(context, nonce []byte, value string) (string, error) {
func (p *Policy) Decrypt(context, nonce []byte, value string, paddingScheme string) (string, error) {
cipherboy marked this conversation as resolved.
Show resolved Hide resolved
if !p.Type.DecryptionSupported() {
return "", errutil.UserError{Err: fmt.Sprintf("message decryption not supported for key type %v", p.Type)}
}
Expand Down Expand Up @@ -994,8 +1005,18 @@ func (p *Policy) Decrypt(context, nonce []byte, value string) (string, error) {
if err != nil {
return "", err
}
if paddingScheme == "" {
paddingScheme = "oaep"
}
key := keyEntry.RSAKey
plain, err = rsa.DecryptOAEP(sha256.New(), rand.Reader, key, decoded, nil)
switch paddingScheme {
case "pkcs1v15":
plain, err = rsa.DecryptPKCS1v15(rand.Reader, key, decoded)
case "oaep":
plain, err = rsa.DecryptOAEP(sha256.New(), rand.Reader, key, decoded, nil)
default:
return "", errutil.InternalError{Err: fmt.Sprintf("unsupported RSA padding scheme %s", paddingScheme)}
}
if err != nil {
return "", errutil.InternalError{Err: fmt.Sprintf("failed to RSA decrypt the ciphertext: %v", err)}
}
Expand Down
7 changes: 4 additions & 3 deletions ui/app/components/transit-key-actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const TRANSIT_PARAMS = {
hash_algorithm: 'sha2-256',
algorithm: 'sha2-256',
signature_algorithm: 'pss',
padding_scheme: 'oaep',
bits: 256,
bytes: 32,
ciphertext: null,
Expand Down Expand Up @@ -39,9 +40,9 @@ const PARAMS_FOR_ACTION = {
sign: ['input', 'hash_algorithm', 'key_version', 'prehashed', 'signature_algorithm'],
verify: ['input', 'hmac', 'signature', 'hash_algorithm', 'prehashed'],
hmac: ['input', 'algorithm', 'key_version'],
encrypt: ['plaintext', 'context', 'nonce', 'key_version'],
decrypt: ['ciphertext', 'context', 'nonce'],
rewrap: ['ciphertext', 'context', 'nonce', 'key_version'],
encrypt: ['plaintext', 'context', 'padding_scheme', 'nonce', 'key_version'],
decrypt: ['ciphertext', 'context', 'padding_scheme', 'nonce'],
rewrap: ['ciphertext', 'context', 'padding_scheme', 'nonce', 'key_version'],
};
const SUCCESS_MESSAGE_FOR_ACTION = {
sign: 'Signed your data',
Expand Down
25 changes: 24 additions & 1 deletion ui/app/templates/components/transit-key-action/datakey.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<form onsubmit={{action @doSubmit (hash param=@param context=@context nonce=@nonce bits=@bits)}}>
<form
onsubmit={{action @doSubmit (hash param=@param padding_scheme=@padding_scheme context=@context nonce=@nonce bits=@bits)}}
>
<div class="box is-sideless is-fullwidth is-marginless">
<NamespaceReminder @mode="perform" @noun="datakey creation" />
<div class="content">
Expand Down Expand Up @@ -60,6 +62,27 @@
</div>
</div>
</div>
{{#if (includes @key.type (array "rsa-2048" "rsa-3072" "rsa-4096"))}}
<div class="field">
<label for="padding_scheme" class="is-label">Padding Scheme</label>
<div class="control is-expanded">
<div class="select is-fullwidth">
<select
name="padding_scheme"
id="padding_scheme"
data-test-padding-scheme-select
onchange={{action (mut @padding_scheme) value="target.value"}}
>
{{#each (array "oaep" "pkcs1v15") as |scheme|}}
<option selected={{eq @padding_scheme scheme}} value={{scheme}}>
{{scheme}}
</option>
{{/each}}
</select>
</div>
</div>
</div>
{{/if}}
</div>
<div class="field is-grouped box is-fullwidth is-bottomless">
<div class="control">
Expand Down
25 changes: 24 additions & 1 deletion ui/app/templates/components/transit-key-action/decrypt.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
<form onsubmit={{action @doSubmit (hash ciphertext=@ciphertext context=@context nonce=@nonce)}}>
<form
onsubmit={{action @doSubmit (hash ciphertext=@ciphertext padding_scheme=@padding_scheme context=@context nonce=@nonce)}}
>
<div class="box is-sideless is-fullwidth is-marginless">
<div class="content">
<p>You can decrypt ciphertext using <code>{{@key.name}}</code> as the encryption key.</p>
Expand Down Expand Up @@ -28,6 +30,27 @@
</div>
</div>
{{/if}}
{{#if (or (eq @key.type "rsa-2048") (eq @key.type "rsa-3072") (eq @key.type "rsa-4096"))}}
<div class="field">
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here as above ☝️

<label for="padding_scheme" class="is-label">Padding Scheme</label>
<div class="control is-expanded">
<div class="select is-fullwidth">
<select
name="padding_scheme"
id="padding_scheme"
data-test-padding-scheme="true"
onchange={{action (mut @padding_scheme) value="target.value"}}
>
{{#each (array "oaep" "pkcs1v15") as |scheme|}}
<option selected={{if @padding_scheme (eq @padding_scheme scheme) (eq scheme "oaep")}} value={{scheme}}>
{{scheme}}
Copy link
Contributor

Choose a reason for hiding this comment

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

and here! 😄

</option>
{{/each}}
</select>
</div>
</div>
</div>
{{/if}}
{{#if (eq @key.convergentEncryptionVersion 1)}}
<div class="field">
<label for="nonce" class="is-label">Nonce</label>
Expand Down
30 changes: 29 additions & 1 deletion ui/app/templates/components/transit-key-action/encrypt.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
<form
onsubmit={{action
@doSubmit
(hash plaintext=@plaintext context=@context nonce=@nonce key_version=@key_version encodedBase64=@encodedBase64)
(hash
plaintext=@plaintext
padding_scheme=@padding_scheme
context=@context
nonce=@nonce
key_version=@key_version
encodedBase64=@encodedBase64
)
}}
>
<div class="box is-sideless is-fullwidth is-marginless">
Expand Down Expand Up @@ -40,6 +47,27 @@
</div>
</div>
{{/if}}
{{#if (or (eq @key.type "rsa-2048") (eq @key.type "rsa-3072") (eq @key.type "rsa-4096"))}}
Copy link
Contributor

Choose a reason for hiding this comment

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

also this file and line 62

<div class="field">
<label for="padding_scheme" class="is-label">Padding Scheme</label>
<div class="control is-expanded">
<div class="select is-fullwidth">
<select
name="padding_scheme"
id="padding_scheme"
data-test-padding-scheme="true"
onchange={{action (mut @padding_scheme) value="target.value"}}
>
{{#each (array "oaep" "pkcs1v15") as |scheme|}}
<option selected={{if @padding_scheme (eq @padding_scheme scheme) (eq scheme "oaep")}} value={{scheme}}>
{{scheme}}
</option>
{{/each}}
</select>
</div>
</div>
</div>
{{/if}}
{{#if (eq @key.convergentEncryptionVersion 1)}}
<div class="field">
<div class="level">
Expand Down
28 changes: 27 additions & 1 deletion ui/app/templates/components/transit-key-action/rewrap.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
<form onsubmit={{action @doSubmit (hash ciphertext=@ciphertext context=@context nonce=@nonce key_version=@key_version)}}>
<form
onsubmit={{action
@doSubmit
(hash ciphertext=@ciphertext padding_scheme=@padding_scheme context=@context nonce=@nonce key_version=@key_version)
}}
>
<div class="box is-sideless is-fullwidth is-marginless">
<NamespaceReminder @mode="perform" @noun="rewrap" />
<div class="content">
Expand Down Expand Up @@ -29,6 +34,27 @@
</div>
</div>
{{/if}}
{{#if (or (eq @key.type "rsa-2048") (eq @key.type "rsa-3072") (eq @key.type "rsa-4096"))}}
<div class="field">
Copy link
Contributor

Choose a reason for hiding this comment

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

last one!

<label for="padding_scheme" class="is-label">Padding Scheme</label>
<div class="control is-expanded">
<div class="select is-fullwidth">
<select
name="padding_scheme"
id="padding_scheme"
data-test-padding-scheme="true"
onchange={{action (mut @padding_scheme) value="target.value"}}
>
{{#each (array "oaep" "pkcs1v15") as |scheme|}}
<option selected={{if @padding_scheme (eq @padding_scheme scheme) (eq scheme "oaep")}} value={{scheme}}>
{{scheme}}
</option>
{{/each}}
</select>
</div>
</div>
</div>
{{/if}}
{{#if (eq @key.convergentEncryptionVersion 1)}}
<div class="field">
<label for="nonce" class="is-label">Nonce</label>
Expand Down
1 change: 1 addition & 0 deletions ui/app/templates/components/transit-key-actions.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
signature=this.signature
signature_algorithm=this.signature_algorithm
hash_algorithm=this.hash_algorithm
padding_scheme=this.padding_scheme
prehashed=this.prehashed
wrappedToken=this.wrappedToken
exportKeyType=this.exportKeyType
Expand Down
Loading
Loading