mirror of
				https://github.com/optim-enterprises-bv/vault.git
				synced 2025-10-31 02:28:09 +00:00 
			
		
		
		
	Address a panic when exporting RSA public keys in transit (#24054)
* Address a panic export RSA public keys in transit - When attempting to export the public key for an RSA key that we only have a private key for, the export panics with a nil deference. - Add additional tests around Transit key exporting * Add cl
This commit is contained in:
		| @@ -5,6 +5,7 @@ package transit | ||||
|  | ||||
| import ( | ||||
| 	"context" | ||||
| 	"crypto" | ||||
| 	"crypto/ecdsa" | ||||
| 	"crypto/elliptic" | ||||
| 	"crypto/x509" | ||||
| @@ -296,18 +297,31 @@ func encodeRSAPublicKey(key *keysutil.KeyEntry) (string, error) { | ||||
| 		return "", errors.New("nil KeyEntry provided") | ||||
| 	} | ||||
|  | ||||
| 	blockType := "RSA PUBLIC KEY" | ||||
| 	derBytes, err := x509.MarshalPKIXPublicKey(key.RSAPublicKey) | ||||
| 	if err != nil { | ||||
| 		return "", err | ||||
| 	var publicKey crypto.PublicKey | ||||
| 	publicKey = key.RSAPublicKey | ||||
| 	if key.RSAKey != nil { | ||||
| 		// Prefer the private key if it exists | ||||
| 		publicKey = key.RSAKey.Public() | ||||
| 	} | ||||
|  | ||||
| 	pemBlock := pem.Block{ | ||||
| 		Type:  blockType, | ||||
| 	if publicKey == nil { | ||||
| 		return "", errors.New("requested to encode an RSA public key with no RSA key present") | ||||
| 	} | ||||
|  | ||||
| 	// Encode the RSA public key in PEM format to return over the API | ||||
| 	derBytes, err := x509.MarshalPKIXPublicKey(publicKey) | ||||
| 	if err != nil { | ||||
| 		return "", fmt.Errorf("error marshaling RSA public key: %w", err) | ||||
| 	} | ||||
| 	pemBlock := &pem.Block{ | ||||
| 		Type:  "PUBLIC KEY", | ||||
| 		Bytes: derBytes, | ||||
| 	} | ||||
| 	pemBytes := pem.EncodeToMemory(pemBlock) | ||||
| 	if pemBytes == nil || len(pemBytes) == 0 { | ||||
| 		return "", fmt.Errorf("failed to PEM-encode RSA public key") | ||||
| 	} | ||||
|  | ||||
| 	pemBytes := pem.EncodeToMemory(&pemBlock) | ||||
| 	return string(pemBytes), nil | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -24,14 +24,58 @@ import ( | ||||
| 	"github.com/stretchr/testify/require" | ||||
| ) | ||||
|  | ||||
| func TestTransit_Export_Unknown_ExportType(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
| 	keyType := "ed25519" | ||||
| 	req := &logical.Request{ | ||||
| 		Storage:   storage, | ||||
| 		Operation: logical.UpdateOperation, | ||||
| 		Path:      "keys/foo", | ||||
| 		Data: map[string]interface{}{ | ||||
| 			"exportable": true, | ||||
| 			"type":       keyType, | ||||
| 		}, | ||||
| 	} | ||||
| 	_, err := b.HandleRequest(context.Background(), req) | ||||
| 	if err != nil { | ||||
| 		t.Fatalf("failed creating key %s: %v", keyType, err) | ||||
| 	} | ||||
|  | ||||
| 	req = &logical.Request{ | ||||
| 		Storage:   storage, | ||||
| 		Operation: logical.ReadOperation, | ||||
| 		Path:      "export/bad-export-type/foo", | ||||
| 	} | ||||
| 	rsp, err := b.HandleRequest(context.Background(), req) | ||||
| 	if err == nil { | ||||
| 		t.Fatalf("did not error on bad export type got: %v", rsp) | ||||
| 	} | ||||
| 	if rsp == nil || !rsp.IsError() { | ||||
| 		t.Fatalf("response did not contain an error on bad export type got: %v", rsp) | ||||
| 	} | ||||
| 	if !strings.Contains(rsp.Error().Error(), "invalid export type") { | ||||
| 		t.Fatalf("failed with unexpected error: %v", err) | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_KeyVersion_ExportsCorrectVersion(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "aes128-gcm96") | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "aes256-gcm96") | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "chacha20-poly1305") | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "rsa-2048") | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "rsa-3072") | ||||
| 	verifyExportsCorrectVersion(t, "encryption-key", "rsa-4096") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "ecdsa-p256") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "ecdsa-p384") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "ecdsa-p521") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "ed25519") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "rsa-2048") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "rsa-3072") | ||||
| 	verifyExportsCorrectVersion(t, "signing-key", "rsa-4096") | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "aes128-gcm96") | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "aes256-gcm96") | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "chacha20-poly1305") | ||||
| @@ -40,6 +84,13 @@ func TestTransit_Export_KeyVersion_ExportsCorrectVersion(t *testing.T) { | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "ecdsa-p521") | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "ed25519") | ||||
| 	verifyExportsCorrectVersion(t, "hmac-key", "hmac") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "rsa-2048") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "rsa-3072") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "rsa-4096") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "ecdsa-p256") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "ecdsa-p384") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "ecdsa-p521") | ||||
| 	verifyExportsCorrectVersion(t, "public-key", "ed25519") | ||||
| } | ||||
|  | ||||
| func verifyExportsCorrectVersion(t *testing.T, exportType, keyType string) { | ||||
| @@ -136,6 +187,8 @@ func verifyExportsCorrectVersion(t *testing.T, exportType, keyType string) { | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_ValidVersionsOnly(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	// First create a key, v1 | ||||
| @@ -236,6 +289,8 @@ func TestTransit_Export_ValidVersionsOnly(t *testing.T) { | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_KeysNotMarkedExportable_ReturnsError(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	req := &logical.Request{ | ||||
| @@ -266,6 +321,8 @@ func TestTransit_Export_KeysNotMarkedExportable_ReturnsError(t *testing.T) { | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_SigningDoesNotSupportSigning_ReturnsError(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	req := &logical.Request{ | ||||
| @@ -294,6 +351,8 @@ func TestTransit_Export_SigningDoesNotSupportSigning_ReturnsError(t *testing.T) | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_EncryptionDoesNotSupportEncryption_ReturnsError(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	testTransit_Export_EncryptionDoesNotSupportEncryption_ReturnsError(t, "ecdsa-p256") | ||||
| 	testTransit_Export_EncryptionDoesNotSupportEncryption_ReturnsError(t, "ecdsa-p384") | ||||
| 	testTransit_Export_EncryptionDoesNotSupportEncryption_ReturnsError(t, "ecdsa-p521") | ||||
| @@ -324,11 +383,55 @@ func testTransit_Export_EncryptionDoesNotSupportEncryption_ReturnsError(t *testi | ||||
| 	} | ||||
| 	_, err = b.HandleRequest(context.Background(), req) | ||||
| 	if err == nil { | ||||
| 		t.Fatal("Key does not support encryption but was exported without error.") | ||||
| 		t.Fatalf("Key %s does not support encryption but was exported without error.", keyType) | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_PublicKeyDoesNotSupportEncryption_ReturnsError(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	testTransit_Export_PublicKeyNotSupported_ReturnsError(t, "chacha20-poly1305") | ||||
| 	testTransit_Export_PublicKeyNotSupported_ReturnsError(t, "aes128-gcm96") | ||||
| 	testTransit_Export_PublicKeyNotSupported_ReturnsError(t, "aes256-gcm96") | ||||
| 	testTransit_Export_PublicKeyNotSupported_ReturnsError(t, "hmac") | ||||
| } | ||||
|  | ||||
| func testTransit_Export_PublicKeyNotSupported_ReturnsError(t *testing.T, keyType string) { | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	req := &logical.Request{ | ||||
| 		Storage:   storage, | ||||
| 		Operation: logical.UpdateOperation, | ||||
| 		Path:      "keys/foo", | ||||
| 		Data: map[string]interface{}{ | ||||
| 			"type": keyType, | ||||
| 		}, | ||||
| 	} | ||||
| 	if keyType == "hmac" { | ||||
| 		req.Data["key_size"] = 32 | ||||
| 	} | ||||
| 	_, err := b.HandleRequest(context.Background(), req) | ||||
| 	if err != nil { | ||||
| 		t.Fatalf("failed creating key %s: %v", keyType, err) | ||||
| 	} | ||||
|  | ||||
| 	req = &logical.Request{ | ||||
| 		Storage:   storage, | ||||
| 		Operation: logical.ReadOperation, | ||||
| 		Path:      "export/public-key/foo", | ||||
| 	} | ||||
| 	_, err = b.HandleRequest(context.Background(), req) | ||||
| 	if err == nil { | ||||
| 		t.Fatalf("Key %s does not support public key exporting but was exported without error.", keyType) | ||||
| 	} | ||||
| 	if !strings.Contains(err.Error(), fmt.Sprintf("unknown key type %s for export type public-key", keyType)) { | ||||
| 		t.Fatalf("unexpected error value for key type: %s: %v", keyType, err) | ||||
| 	} | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_KeysDoesNotExist_ReturnsNotFound(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	req := &logical.Request{ | ||||
| @@ -344,6 +447,8 @@ func TestTransit_Export_KeysDoesNotExist_ReturnsNotFound(t *testing.T) { | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_EncryptionKey_DoesNotExportHMACKey(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	b, storage := createBackendWithSysView(t) | ||||
|  | ||||
| 	req := &logical.Request{ | ||||
| @@ -394,6 +499,8 @@ func TestTransit_Export_EncryptionKey_DoesNotExportHMACKey(t *testing.T) { | ||||
| } | ||||
|  | ||||
| func TestTransit_Export_CertificateChain(t *testing.T) { | ||||
| 	t.Parallel() | ||||
|  | ||||
| 	generateKeys(t) | ||||
|  | ||||
| 	// Create Cluster | ||||
|   | ||||
| @@ -5,9 +5,7 @@ package transit | ||||
|  | ||||
| import ( | ||||
| 	"context" | ||||
| 	"crypto" | ||||
| 	"crypto/elliptic" | ||||
| 	"crypto/x509" | ||||
| 	"encoding/base64" | ||||
| 	"encoding/pem" | ||||
| 	"fmt" | ||||
| @@ -429,27 +427,11 @@ func (b *backend) formatKeyPolicy(p *keysutil.Policy, context []byte) (*logical. | ||||
| 					key.Name = "rsa-4096" | ||||
| 				} | ||||
|  | ||||
| 				var publicKey crypto.PublicKey | ||||
| 				publicKey = v.RSAPublicKey | ||||
| 				if !v.IsPrivateKeyMissing() { | ||||
| 					publicKey = v.RSAKey.Public() | ||||
| 				} | ||||
|  | ||||
| 				// Encode the RSA public key in PEM format to return over the | ||||
| 				// API | ||||
| 				derBytes, err := x509.MarshalPKIXPublicKey(publicKey) | ||||
| 				pubKey, err := encodeRSAPublicKey(&v) | ||||
| 				if err != nil { | ||||
| 					return nil, fmt.Errorf("error marshaling RSA public key: %w", err) | ||||
| 					return nil, err | ||||
| 				} | ||||
| 				pemBlock := &pem.Block{ | ||||
| 					Type:  "PUBLIC KEY", | ||||
| 					Bytes: derBytes, | ||||
| 				} | ||||
| 				pemBytes := pem.EncodeToMemory(pemBlock) | ||||
| 				if pemBytes == nil || len(pemBytes) == 0 { | ||||
| 					return nil, fmt.Errorf("failed to PEM-encode RSA public key") | ||||
| 				} | ||||
| 				key.PublicKey = string(pemBytes) | ||||
| 				key.PublicKey = pubKey | ||||
| 			} | ||||
|  | ||||
| 			retKeys[k] = structs.New(key).Map() | ||||
|   | ||||
							
								
								
									
										3
									
								
								changelog/24054.txt
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										3
									
								
								changelog/24054.txt
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,3 @@ | ||||
| ```release-note:bug | ||||
| secrets/transit: Fix a panic when attempting to export a public RSA key | ||||
| ``` | ||||
		Reference in New Issue
	
	Block a user
	 Steven Clark
					Steven Clark