Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

crypto: virtio - Drop sign/verify operations

The virtio crypto driver exposes akcipher sign/verify operations in a
user space ABI. This blocks removal of sign/verify from akcipher_alg.

Herbert opines:

"I would say that this is something that we can break. Breaking it
is no different to running virtio on a host that does not support
these algorithms. After all, a software implementation must always
be present.

I deliberately left akcipher out of crypto_user because the API
is still in flux. We should not let virtio constrain ourselves."
https://lore.kernel.org/all/ZtqoNAgcnXnrYhZZ@gondor.apana.org.au/

"I would remove virtio akcipher support in its entirety. This API
was never meant to be exposed outside of the kernel."
https://lore.kernel.org/all/Ztqql_gqgZiMW8zz@gondor.apana.org.au/

Drop sign/verify support from virtio crypto. There's no strong reason
to also remove encrypt/decrypt support, so keep it.

A key selling point of virtio crypto is to allow guest access to crypto
accelerators on the host. So far the only akcipher algorithm supported
by virtio crypto is RSA. Dropping sign/verify merely means that the
PKCS#1 padding is now always generated or verified inside the guest,
but the actual signature generation/verification (which is an RSA
decrypt/encrypt operation) may still use an accelerator on the host.

Generating or verifying the PKCS#1 padding is cheap, so a hardware
accelerator won't be of much help there. Which begs the question
whether virtio crypto support for sign/verify makes sense at all.

It would make sense for the sign operation if the host has a security
chip to store asymmetric private keys. But the kernel doesn't even
have an asymmetric_key_subtype yet for hardware-based private keys.
There's at least one rudimentary driver for such chips (atmel-ecc.c for
ATECC508A), but it doesn't implement the sign operation. The kernel
would first have to grow support for a hardware asymmetric_key_subtype
and at least one driver implementing the sign operation before exposure
to guests via virtio makes sense.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

authored by

Lukas Wunner and committed by
Herbert Xu
5b553e06 778206d8

+22 -44
+21 -44
drivers/crypto/virtio/virtio_crypto_akcipher_algs.c
··· 83 83 case VIRTIO_CRYPTO_BADMSG: 84 84 error = -EBADMSG; 85 85 break; 86 - 87 - case VIRTIO_CRYPTO_KEY_REJECTED: 88 - error = -EKEYREJECTED; 89 - break; 90 - 91 86 default: 92 87 error = -EIO; 93 88 break; 94 89 } 95 90 96 91 akcipher_req = vc_akcipher_req->akcipher_req; 97 - if (vc_akcipher_req->opcode != VIRTIO_CRYPTO_AKCIPHER_VERIFY) { 98 - /* actuall length maybe less than dst buffer */ 99 - akcipher_req->dst_len = len - sizeof(vc_req->status); 100 - sg_copy_from_buffer(akcipher_req->dst, sg_nents(akcipher_req->dst), 101 - vc_akcipher_req->dst_buf, akcipher_req->dst_len); 102 - } 92 + /* actual length maybe less than dst buffer */ 93 + akcipher_req->dst_len = len - sizeof(vc_req->status); 94 + sg_copy_from_buffer(akcipher_req->dst, sg_nents(akcipher_req->dst), 95 + vc_akcipher_req->dst_buf, akcipher_req->dst_len); 103 96 virtio_crypto_akcipher_finalize_req(vc_akcipher_req, akcipher_req, error); 104 97 } 105 98 ··· 223 230 int node = dev_to_node(&vcrypto->vdev->dev); 224 231 unsigned long flags; 225 232 int ret; 226 - bool verify = vc_akcipher_req->opcode == VIRTIO_CRYPTO_AKCIPHER_VERIFY; 227 - unsigned int src_len = verify ? req->src_len + req->dst_len : req->src_len; 228 233 229 234 /* out header */ 230 235 sg_init_one(&outhdr_sg, req_data, sizeof(*req_data)); 231 236 sgs[num_out++] = &outhdr_sg; 232 237 233 238 /* src data */ 234 - src_buf = kcalloc_node(src_len, 1, GFP_KERNEL, node); 239 + src_buf = kcalloc_node(req->src_len, 1, GFP_KERNEL, node); 235 240 if (!src_buf) 236 241 return -ENOMEM; 237 242 238 - if (verify) { 239 - /* for verify operation, both src and dst data work as OUT direction */ 240 - sg_copy_to_buffer(req->src, sg_nents(req->src), src_buf, src_len); 241 - sg_init_one(&srcdata_sg, src_buf, src_len); 242 - sgs[num_out++] = &srcdata_sg; 243 - } else { 244 - sg_copy_to_buffer(req->src, sg_nents(req->src), src_buf, src_len); 245 - sg_init_one(&srcdata_sg, src_buf, src_len); 246 - sgs[num_out++] = &srcdata_sg; 243 + sg_copy_to_buffer(req->src, sg_nents(req->src), src_buf, req->src_len); 244 + sg_init_one(&srcdata_sg, src_buf, req->src_len); 245 + sgs[num_out++] = &srcdata_sg; 247 246 248 - /* dst data */ 249 - dst_buf = kcalloc_node(req->dst_len, 1, GFP_KERNEL, node); 250 - if (!dst_buf) 251 - goto free_src; 247 + /* dst data */ 248 + dst_buf = kcalloc_node(req->dst_len, 1, GFP_KERNEL, node); 249 + if (!dst_buf) 250 + goto free_src; 252 251 253 - sg_init_one(&dstdata_sg, dst_buf, req->dst_len); 254 - sgs[num_out + num_in++] = &dstdata_sg; 255 - } 252 + sg_init_one(&dstdata_sg, dst_buf, req->dst_len); 253 + sgs[num_out + num_in++] = &dstdata_sg; 256 254 257 255 vc_akcipher_req->src_buf = src_buf; 258 256 vc_akcipher_req->dst_buf = dst_buf; ··· 334 350 static int virtio_crypto_rsa_decrypt(struct akcipher_request *req) 335 351 { 336 352 return virtio_crypto_rsa_req(req, VIRTIO_CRYPTO_AKCIPHER_DECRYPT); 337 - } 338 - 339 - static int virtio_crypto_rsa_sign(struct akcipher_request *req) 340 - { 341 - return virtio_crypto_rsa_req(req, VIRTIO_CRYPTO_AKCIPHER_SIGN); 342 - } 343 - 344 - static int virtio_crypto_rsa_verify(struct akcipher_request *req) 345 - { 346 - return virtio_crypto_rsa_req(req, VIRTIO_CRYPTO_AKCIPHER_VERIFY); 347 353 } 348 354 349 355 static int virtio_crypto_rsa_set_key(struct crypto_akcipher *tfm, ··· 498 524 .algo.base = { 499 525 .encrypt = virtio_crypto_rsa_encrypt, 500 526 .decrypt = virtio_crypto_rsa_decrypt, 501 - .sign = virtio_crypto_rsa_sign, 502 - .verify = virtio_crypto_rsa_verify, 527 + /* 528 + * Must specify an arbitrary hash algorithm upon 529 + * set_{pub,priv}_key (even though it's not used 530 + * by encrypt/decrypt) because qemu checks for it. 531 + */ 503 532 .set_pub_key = virtio_crypto_p1pad_rsa_sha1_set_pub_key, 504 533 .set_priv_key = virtio_crypto_p1pad_rsa_sha1_set_priv_key, 505 534 .max_size = virtio_crypto_rsa_max_size, 506 535 .init = virtio_crypto_rsa_init_tfm, 507 536 .exit = virtio_crypto_rsa_exit_tfm, 508 537 .base = { 509 - .cra_name = "pkcs1pad(rsa,sha1)", 510 - .cra_driver_name = "virtio-pkcs1-rsa-with-sha1", 538 + .cra_name = "pkcs1pad(rsa)", 539 + .cra_driver_name = "virtio-pkcs1-rsa", 511 540 .cra_priority = 150, 512 541 .cra_module = THIS_MODULE, 513 542 .cra_ctxsize = sizeof(struct virtio_crypto_akcipher_ctx),
+1
include/uapi/linux/virtio_crypto.h
··· 329 329 VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x00) 330 330 #define VIRTIO_CRYPTO_AKCIPHER_DECRYPT \ 331 331 VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x01) 332 + /* akcipher sign/verify opcodes are deprecated */ 332 333 #define VIRTIO_CRYPTO_AKCIPHER_SIGN \ 333 334 VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AKCIPHER, 0x02) 334 335 #define VIRTIO_CRYPTO_AKCIPHER_VERIFY \