On Mon, Oct 05, 2020 at 07:41:33AM +0000, Satya Tangirala wrote:
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 871d773..a82d753 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -2,10 +2,10 @@
>
> lib_LTLIBRARIES = libf2fs.la
>
> -libf2fs_la_SOURCES = libf2fs.c libf2fs_io.c libf2fs_zoned.c nls_utf8.c
> +libf2fs_la_SOURCES = libf2fs.c libf2fs_io.c libf2fs_zoned.c nls_utf8.c f2fs_metadata_crypt.c
> libf2fs_la_CFLAGS = -Wall
> libf2fs_la_CPPFLAGS = -I$(top_srcdir)/include
> -libf2fs_la_LDFLAGS = -version-info $(LIBF2FS_CURRENT):$(LIBF2FS_REVISION):$(LIBF2FS_AGE)
> +libf2fs_la_LDFLAGS = -lkeyutils -version-info $(LIBF2FS_CURRENT):$(LIBF2FS_REVISION):$(LIBF2FS_AGE)
This introduces a dependency on libkeyutils. Doesn't that need to be checked in
configure.ac?
> diff --git a/lib/f2fs_metadata_crypt.c b/lib/f2fs_metadata_crypt.c
> new file mode 100644
> index 0000000..faf399a
> --- /dev/null
> +++ b/lib/f2fs_metadata_crypt.c
> @@ -0,0 +1,226 @@
> +/**
> + * f2fs_metadata_crypt.c
> + *
> + * Copyright (c) 2020 Google LLC
> + *
> + * Dual licensed under the GPL or LGPL version 2 licenses.
> + */
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/socket.h>
> +#include <linux/if_alg.h>
> +#include <linux/socket.h>
> +#include <assert.h>
> +#include <errno.h>
> +#include <keyutils.h>
> +
> +#include "f2fs_fs.h"
> +#include "f2fs_metadata_crypt.h"
> +
> +extern struct f2fs_configuration c;
> +struct f2fs_crypt_mode {
> + const char *friendly_name;
> + const char *cipher_str;
> + unsigned int keysize;
> + unsigned int ivlen;
> +} f2fs_crypt_modes[] = {
Use 'const' for static or global data that isn't modified.
> +void f2fs_print_crypt_algs(void)
> +{
> + int i;
> +
> + for (i = 1; i <= __FSCRYPT_MODE_MAX; i++) {
> + if (!f2fs_crypt_modes[i].friendly_name)
> + continue;
> + MSG(0, "\t%s\n", f2fs_crypt_modes[i].friendly_name);
> + }
> +}
> +
> +int f2fs_get_crypt_alg(const char *optarg)
> +{
> + int i;
> +
> + for (i = 1; i <= __FSCRYPT_MODE_MAX; i++) {
> + if (f2fs_crypt_modes[i].friendly_name &&
> + !strcmp(f2fs_crypt_modes[i].friendly_name, optarg)) {
> + return i;
> + }
> + }
> +
> + return 0;
> +}
Although __FSCRYPT_MODE_MAX is defined in <linux/fscrypt.h>, it isn't intended
to be used in userspace programs, as its value will change depending on the
version of the kernel headers. Just use ARRAY_SIZE(f2fs_crypt_modes) instead.
> +int f2fs_metadata_crypt_block(void *buf, size_t len, __u64 blk_addr,
> + bool encrypt)
> +{
> + struct f2fs_crypt_mode *crypt_mode;
> + int sockfd, fd;
> + struct sockaddr_alg sa = {
> + .salg_family = AF_ALG,
> + .salg_type = "skcipher",
> + };
> + struct msghdr msg = {};
> + struct cmsghdr *cmsg;
> + char cbuf[CMSG_SPACE(4) + CMSG_SPACE(4 + MAX_IV_LEN)] = {0};
> + int blk_offset;
> + struct af_alg_iv *iv;
> + struct iovec iov;
> + int err;
> +
> + crypt_mode = &f2fs_crypt_modes[c.metadata_crypt_alg];
> + memcpy(sa.salg_name, crypt_mode->cipher_str,
> + strlen(crypt_mode->cipher_str));
> +
> + sockfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
> + if (sockfd < 0)
> + return errno;
This will fail if AF_ALG isn't enabled in the kernel config, or if the process
isn't allowed to use AF_ALG by SELinux policy. Can you show a proper error
message?
> + err = bind(sockfd, (struct sockaddr *)&sa, sizeof(sa));
> + if (err) {
> + MSG(0, "\tCouldn't bind crypto socket. Maybe support for the crypto algorithm isn't enabled?\n");
> + close(sockfd);
> + return errno;
> + }
This will fail if either CRYPTO_USER_API_SKCIPHER isn't enabled in the kernel
config, or if the required crypto algorithm isn't enabled in the kernel config.
Can you show a better error message?
Also, these new kernel config option dependencies should be documented in the
documentation for f2fs-tools.
> + err = setsockopt(sockfd, SOL_ALG, ALG_SET_KEY, c.metadata_crypt_key,
> + crypt_mode->keysize);
> + if (err) {
> + MSG(0, "\tCouldn't set crypto socket options.\n");
> + close(sockfd);
> + return errno;
> + }
> + fd = accept(sockfd, NULL, 0);
> + if (fd < 0)
> + goto err_out;
It's a lot of work to allocate an AF_ALG algorithm socket, set the key, and
allocate a request socket for every block. Can any of this be cached? For
single threaded use, it seems the request socket can be cached; otherwise the
algorithm socket with a key set can be cached.
- Eric
|