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

[WIP] Add support for longer RSA keys longer than 2048, init ECDSA support #21

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[WIP] Add support for longer RSA keys longer than 2048, init ECDSA support #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1de7d65
Add support for longer RSA keys longer than 2048, init ECDSA support
tomaszkrzyzanowski Mar 19, 2024
46f71a2
Merge checkKeyType function into wrapPrivateKey
tomaszkrzyzanowski Mar 21, 2024
b46e574
Notary to Vault hash algorithm map comment improvement
tomaszkrzyzanowski Mar 21, 2024
a8e639e
Better descriptions for maps of key length/curve to vault/openbao key…
tomaszkrzyzanowski Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
60 changes: 47 additions & 13 deletions cmd/key-helper/importKey.go
View file
Open in desktop
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"crypto/ecdsa"
"crypto/rand"
"crypto/rsa"
"crypto/sha256"
Expand Down Expand Up @@ -51,11 +52,11 @@ func importKeyCommand() *cobra.Command {
return err
}
fmt.Println("Successfully got wrapping key")
ciphertext, err := wrapPrivateKey(wrappingKey, keyPath)
ciphertext, keyType, err := wrapPrivateKey(wrappingKey, keyPath)
if err != nil {
return err
}
if err := importKeyToTransit(ctx, vaultClient, ciphertext, keyName); err != nil {
if err := importKeyToTransit(ctx, vaultClient, ciphertext, keyName, keyType); err != nil {
return err
}
fmt.Println("Successfully imported key to transit")
Expand Down Expand Up @@ -98,32 +99,65 @@ func getWrappingKey(ctx context.Context, client *vault.Client) (string, error) {
return key, nil
}

func wrapPrivateKey(wrappingKey string, privateKeyPath string) (string, error) {
func wrapPrivateKey(wrappingKey string, privateKeyPath string) (string, string, error) {
// Key mapping length to Vault/OpenBao names (supported by Notary project specification)
supportedRSAKeys := map[int]string{
2048: "rsa-2048",
3072: "rsa-3072",
4096: "rsa-4096",
}
// Key mapping curve name to Vault/OpenBao names (supported by Notary project specification)
supportedECDSAKeys := map[string]string{
"P-256": "ecdsa-p256",
"P-384": "ecdsa-p384",
"P-521": "ecdsa-p521",
}

keyBlock, _ := pem.Decode([]byte(wrappingKey))
privateKey, err := notationx509.ReadPrivateKeyFile(privateKeyPath)
if err != nil {
return "", err
return "", "", err
}
pkcs8PrivateKey, err := x509.MarshalPKCS8PrivateKey(privateKey)
if err != nil {
return "", err
return "", "", err
}

var keyType string
var ok bool
switch key := privateKey.(type) {
case *ecdsa.PrivateKey:
curveName := key.PublicKey.Curve.Params().Name
keyType, ok = supportedECDSAKeys[curveName]

if !ok {
return "", "", fmt.Errorf("unsupported curve type: `%s`", curveName)
}

case *rsa.PrivateKey:
keyLength := key.N.BitLen()
keyType, ok = supportedRSAKeys[keyLength]

if !ok {
return "", "", fmt.Errorf("unsupported RSA keylength: %d", keyLength)
}
}
parsedKey, err := x509.ParsePKIXPublicKey(keyBlock.Bytes)
if err != nil {
return "", err
return "", "", err
}
ephemeralAESKey := make([]byte, 32)
_, err = rand.Read(ephemeralAESKey)
if err != nil {
return "", err
return "", "", err
}
wrapKWP, err := subtle.NewKWP(ephemeralAESKey)
if err != nil {
return "", err
return "", "", err
}
wrappedTargetKey, err := wrapKWP.Wrap(pkcs8PrivateKey)
if err != nil {
return "", err
return "", "", err
}
wrappedAESKey, err := rsa.EncryptOAEP(
sha256.New(),
Expand All @@ -133,14 +167,14 @@ func wrapPrivateKey(wrappingKey string, privateKeyPath string) (string, error) {
[]byte{},
)
if err != nil {
return "", err
return "", "", err
}
combinedCiphertext := append(wrappedAESKey, wrappedTargetKey...)
base64Ciphertext := base64.StdEncoding.EncodeToString(combinedCiphertext)
return base64Ciphertext, nil
return base64Ciphertext, keyType, nil
}

func importKeyToTransit(ctx context.Context, client *vault.Client, ciphertext string, keyName string) error {
func importKeyToTransit(ctx context.Context, client *vault.Client, ciphertext string, keyName string, keyType string) error {
path := fmt.Sprintf("/transit/keys/%s/import", keyName)
req := map[string]interface{}{
"allow_plaintext_backup": false,
Expand All @@ -151,7 +185,7 @@ func importKeyToTransit(ctx context.Context, client *vault.Client, ciphertext st
"derived": false,
"exportable": false,
"hashFunction": "SHA256",
"type": "rsa-2048",
"type": keyType,
"name": "keyName",
}
_, err := client.Logical().WriteWithContext(ctx, path, req)
Expand Down
8 changes: 5 additions & 3 deletions internal/keyvault/keyvault.go
View file
Open in desktop
Expand Up @@ -5,10 +5,11 @@ import (
"crypto/x509"
"encoding/base64"
"errors"
vault "github.com/hashicorp/vault/api"
"github.com/notaryproject/notation-hashicorp-vault/internal/crypto"
"os"
"strings"

vault "github.com/hashicorp/vault/api"
"github.com/notaryproject/notation-hashicorp-vault/internal/crypto"
)

type VaultClientWrapper struct {
Expand Down Expand Up @@ -52,14 +53,15 @@ func (vw *VaultClientWrapper) GetCertificateChain(ctx context.Context) ([]*x509.
return crypto.ParseCertificates(certBytes)
}

func (vw *VaultClientWrapper) SignWithTransit(ctx context.Context, encodedData string, signAlgorithm string) ([]byte, error) {
func (vw *VaultClientWrapper) SignWithTransit(ctx context.Context, encodedData string, signAlgorithm string, hashAlgorithm string) ([]byte, error) {
Copy link

Choose a reason for hiding this comment

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

This I think is a good catch; right now we do not correctly identify the hash algorithm which means our signatures likely do not comply with Notary's specifications: https://github.com/notaryproject/specifications/blob/main/specs/signature-specification.md#signature-algorithm-requirements

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that was one of an issue while using 4096-bit RSA keys while signing :)

// sign with transit SE
transitSignReq := map[string]interface{}{
"input": encodedData,
"marshaling_algorithm": "asn1",
"prehashed": true,
"salt_length": "hash",
"signature_algorithm": signAlgorithm,
"hash_algorithm": hashAlgorithm,
}
path := "transit/sign/" + vw.keyID
resp, err := vw.vaultClient.Logical().WriteWithContext(ctx, path, transitSignReq)
Expand Down
11 changes: 10 additions & 1 deletion internal/signature/signer.go
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/base64"
"errors"
"fmt"

"github.com/notaryproject/notation-go/plugin/proto"
"github.com/notaryproject/notation-hashicorp-vault/internal/keyvault"
)
Expand Down Expand Up @@ -62,6 +63,14 @@ func Sign(ctx context.Context, req *proto.GenerateSignatureRequest) (*proto.Gene
}
}

// Notary to OpenBao/Vault hash algorithm naming conversion map
vaultHashAlgorithms := map[string]string{
"SHA-256": "sha2-256",
"SHA-384": "sha2-384",
"SHA-512": "sha2-512",
}
vaultHashAlgorithm := vaultHashAlgorithms[string(req.Hash)]

// compute hash for the payload
hashData, err := computeHash(keySpec.SignatureAlgorithm().Hash(), req.Payload)
if err != nil {
Expand All @@ -71,7 +80,7 @@ func Sign(ctx context.Context, req *proto.GenerateSignatureRequest) (*proto.Gene
}
}
encodedHash := base64.StdEncoding.EncodeToString(hashData)
sigBytes, err := vaultClient.SignWithTransit(ctx, encodedHash, signAlgorithm)
sigBytes, err := vaultClient.SignWithTransit(ctx, encodedHash, signAlgorithm, vaultHashAlgorithm)
if err != nil {
return nil, &proto.RequestError{
Code: proto.ErrorCodeGeneric,
Expand Down