From 6597daea70bd9b93fda5c6d0dc95f9621dc6d5bb Mon Sep 17 00:00:00 2001 From: jhb Date: Wed, 25 Nov 2020 00:10:54 +0000 Subject: [PATCH] Remove the cloned file descriptors for /dev/crypto. Crypto file descriptors were added in the original OCF import as a way to provide per-open data (specifically the list of symmetric sessions). However, this gives a bit of a confusing API where one has to open /dev/crypto and then invoke an ioctl to obtain a second file descriptor. This also does not match the API used with /dev/crypto on other BSDs or with Linux's /dev/crypto driver. Character devices have gained support for per-open data via cdevpriv since OCF was imported, so use cdevpriv to simplify the userland API by permitting ioctls directly on /dev/crypto descriptors. To provide backwards compatibility, CRIOGET now opens another /dev/crypto descriptor via kern_openat() rather than dup'ing the existing file descriptor. This preserves prior semantics in case CRIOGET is invoked multiple times on a single file descriptor. Reviewed by: markj Relnotes: yes Sponsored by: Chelsio Communications Differential Revision: https://reviews.freebsd.org/D27302 --- lib/libprocstat/libprocstat.c | 1 - lib/libprocstat/libprocstat.h | 2 +- share/man/man4/crypto.4 | 29 +--- sys/opencrypto/cryptodev.c | 214 ++++++++++++------------------ sys/opencrypto/cryptodev.h | 9 -- sys/sys/user.h | 2 +- tools/tools/crypto/cryptocheck.c | 19 +-- usr.bin/procstat/procstat.1 | 4 +- usr.bin/procstat/procstat_files.c | 5 - 9 files changed, 98 insertions(+), 187 deletions(-) diff --git a/lib/libprocstat/libprocstat.c b/lib/libprocstat/libprocstat.c index a3545b919896..8d10c2900459 100644 --- a/lib/libprocstat/libprocstat.c +++ b/lib/libprocstat/libprocstat.c @@ -708,7 +708,6 @@ kinfo_type2fst(int kftype) int fst_type; } kftypes2fst[] = { { KF_TYPE_PROCDESC, PS_FST_TYPE_PROCDESC }, - { KF_TYPE_CRYPTO, PS_FST_TYPE_CRYPTO }, { KF_TYPE_DEV, PS_FST_TYPE_DEV }, { KF_TYPE_FIFO, PS_FST_TYPE_FIFO }, { KF_TYPE_KQUEUE, PS_FST_TYPE_KQUEUE }, diff --git a/lib/libprocstat/libprocstat.h b/lib/libprocstat/libprocstat.h index 6e03ef515444..4ef34da75ec2 100644 --- a/lib/libprocstat/libprocstat.h +++ b/lib/libprocstat/libprocstat.h @@ -64,7 +64,7 @@ #define PS_FST_TYPE_PIPE 4 #define PS_FST_TYPE_PTS 5 #define PS_FST_TYPE_KQUEUE 6 -#define PS_FST_TYPE_CRYPTO 7 +/* was PS_FST_TYPE_CRYPTO 7 */ #define PS_FST_TYPE_MQUEUE 8 #define PS_FST_TYPE_SHM 9 #define PS_FST_TYPE_SEM 10 diff --git a/share/man/man4/crypto.4 b/share/man/man4/crypto.4 index b31a02efdcb5..851b8b1b0f31 100644 --- a/share/man/man4/crypto.4 +++ b/share/man/man4/crypto.4 @@ -60,7 +60,7 @@ .\" .\" $FreeBSD$ .\" -.Dd November 6, 2020 +.Dd November 24, 2020 .Dt CRYPTO 4 .Os .Sh NAME @@ -122,19 +122,11 @@ Open the .Pa /dev/crypto device. .It -Create a new cryptography file descriptor via -.Dv CRIOGET -to use for all subsequent -.Xr ioctl 2 -commands. -.It -Close the -.Pa /dev/crypto -device. -.It If any symmetric-keyed cryptographic or digest operations will be performed, create a session with -.Dv CIOCGSESSION . +.Dv CIOCGSESSION +or +.Dv CIOCGSESSION2 . Most applications will require at least one symmetric session. Since cipher and MAC keys are tied to sessions, many applications will require more. @@ -152,8 +144,9 @@ or Optionally destroy a session with .Dv CIOCFSESSION . .It -Close the cryptography file descriptor with -.Xr close 2 . +Close the +.Pa /dev/crypto +device. This will automatically close any remaining sessions associated with the file desriptor. .El @@ -458,11 +451,3 @@ session: if you request a algorithm, you must supply a suitably-sized buffer. .Pp The scheme for passing arguments for asymmetric requests is baroque. -.Pp -.Dv CRIOGET -should not exist. -It should be possible to use the -.Dv CIOC Ns \&* -commands directly on a -.Pa /dev/crypto -file descriptor. diff --git a/sys/opencrypto/cryptodev.c b/sys/opencrypto/cryptodev.c index 290dc4def74a..9bb95bcb21f0 100644 --- a/sys/opencrypto/cryptodev.c +++ b/sys/opencrypto/cryptodev.c @@ -47,9 +47,8 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include -#include -#include #include #include #include @@ -57,8 +56,8 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include +#include #include #include @@ -67,6 +66,17 @@ SDT_PROVIDER_DECLARE(opencrypto); SDT_PROBE_DEFINE1(opencrypto, dev, ioctl, error, "int"/*line number*/); +#ifdef COMPAT_FREEBSD12 +/* + * Previously, most ioctls were performed against a cloned descriptor + * of /dev/crypto obtained via CRIOGET. Now all ioctls are performed + * against /dev/crypto directly. + */ +#define CRIOGET _IOWR('c', 100, uint32_t) +#endif + +/* the following are done against the cloned descriptor */ + #ifdef COMPAT_FREEBSD32 #include #include @@ -351,29 +361,6 @@ SYSCTL_TIMEVAL_SEC(_kern, OID_AUTO, cryptodev_warn_interval, CTLFLAG_RW, &warninterval, "Delay in seconds between warnings of deprecated /dev/crypto algorithms"); -static int cryptof_ioctl(struct file *, u_long, void *, struct ucred *, - struct thread *); -static int cryptof_stat(struct file *, struct stat *, struct ucred *, - struct thread *); -static int cryptof_close(struct file *, struct thread *); -static int cryptof_fill_kinfo(struct file *, struct kinfo_file *, - struct filedesc *); - -static struct fileops cryptofops = { - .fo_read = invfo_rdwr, - .fo_write = invfo_rdwr, - .fo_truncate = invfo_truncate, - .fo_ioctl = cryptof_ioctl, - .fo_poll = invfo_poll, - .fo_kqfilter = invfo_kqfilter, - .fo_stat = cryptof_stat, - .fo_close = cryptof_close, - .fo_chmod = invfo_chmod, - .fo_chown = invfo_chown, - .fo_sendfile = invfo_sendfile, - .fo_fill_kinfo = cryptof_fill_kinfo, -}; - /* * Check a crypto identifier to see if it requested * a software device/driver. This can be done either @@ -762,7 +749,7 @@ cse_delete(struct fcrypt *fcr, u_int ses) } static struct cryptop_data * -cod_alloc(struct csession *cse, size_t aad_len, size_t len, struct thread *td) +cod_alloc(struct csession *cse, size_t aad_len, size_t len) { struct cryptop_data *cod; @@ -808,8 +795,7 @@ cryptodev_cb(struct cryptop *crp) } static int -cryptodev_op(struct csession *cse, const struct crypt_op *cop, - struct ucred *active_cred, struct thread *td) +cryptodev_op(struct csession *cse, const struct crypt_op *cop) { struct cryptop_data *cod = NULL; struct cryptop *crp = NULL; @@ -845,7 +831,7 @@ cryptodev_op(struct csession *cse, const struct crypt_op *cop, } } - cod = cod_alloc(cse, 0, cop->len + cse->hashsize, td); + cod = cod_alloc(cse, 0, cop->len + cse->hashsize); dst = cop->dst; crp = crypto_getreq(cse->cses, M_WAITOK); @@ -1019,8 +1005,7 @@ cryptodev_op(struct csession *cse, const struct crypt_op *cop, } static int -cryptodev_aead(struct csession *cse, struct crypt_aead *caead, - struct ucred *active_cred, struct thread *td) +cryptodev_aead(struct csession *cse, struct crypt_aead *caead) { struct cryptop_data *cod = NULL; struct cryptop *crp = NULL; @@ -1050,7 +1035,7 @@ cryptodev_aead(struct csession *cse, struct crypt_aead *caead, } } - cod = cod_alloc(cse, caead->aadlen, caead->len + cse->hashsize, td); + cod = cod_alloc(cse, caead->aadlen, caead->len + cse->hashsize); dst = caead->dst; crp = crypto_getreq(cse->cses, M_WAITOK); @@ -1358,12 +1343,44 @@ cryptodev_find(struct crypt_find_op *find) return (0); } +static void +fcrypt_dtor(void *data) +{ + struct fcrypt *fcr = data; + struct csession *cse; + + while ((cse = TAILQ_FIRST(&fcr->csessions))) { + TAILQ_REMOVE(&fcr->csessions, cse, next); + KASSERT(refcount_load(&cse->refs) == 1, + ("%s: crypto session %p with %d refs", __func__, cse, + refcount_load(&cse->refs))); + cse_free(cse); + } + mtx_destroy(&fcr->lock); + free(fcr, M_XDATA); +} + static int -cryptof_ioctl(struct file *fp, u_long cmd, void *data, - struct ucred *active_cred, struct thread *td) +crypto_open(struct cdev *dev, int oflags, int devtype, struct thread *td) +{ + struct fcrypt *fcr; + int error; + + fcr = malloc(sizeof(struct fcrypt), M_XDATA, M_WAITOK | M_ZERO); + TAILQ_INIT(&fcr->csessions); + mtx_init(&fcr->lock, "fcrypt", NULL, MTX_DEF); + error = devfs_set_cdevpriv(fcr, fcrypt_dtor); + if (error) + fcrypt_dtor(fcr); + return (error); +} + +static int +crypto_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag, + struct thread *td) { static struct timeval keywarn, featwarn; - struct fcrypt *fcr = fp->f_data; + struct fcrypt *fcr; struct csession *cse; struct session2_op *sop; struct crypt_op *cop; @@ -1390,14 +1407,14 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, cmd32 = cmd; data32 = data; cmd = CIOCGSESSION; - data = &thunk.sopc; + data = (void *)&thunk.sopc; session_op_from_32((struct session_op32 *)data32, &thunk.sopc); break; case CIOCGSESSION232: cmd32 = cmd; data32 = data; cmd = CIOCGSESSION2; - data = &thunk.sopc; + data = (void *)&thunk.sopc; session2_op_from_32((struct session2_op32 *)data32, &thunk.sopc); break; @@ -1405,14 +1422,14 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, cmd32 = cmd; data32 = data; cmd = CIOCCRYPT; - data = &thunk.copc; + data = (void *)&thunk.copc; crypt_op_from_32((struct crypt_op32 *)data32, &thunk.copc); break; case CIOCCRYPTAEAD32: cmd32 = cmd; data32 = data; cmd = CIOCCRYPTAEAD; - data = &thunk.aeadc; + data = (void *)&thunk.aeadc; crypt_aead_from_32((struct crypt_aead32 *)data32, &thunk.aeadc); break; case CIOCKEY32: @@ -1423,24 +1440,41 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, cmd = CIOCKEY; else cmd = CIOCKEY2; - data = &thunk.kopc; + data = (void *)&thunk.kopc; crypt_kop_from_32((struct crypt_kop32 *)data32, &thunk.kopc); break; } #endif + devfs_get_cdevpriv((void **)&fcr); + switch (cmd) { +#ifdef COMPAT_FREEBSD12 + case CRIOGET: + /* + * NB: This may fail in cases that the old + * implementation did not if the current process has + * restricted filesystem access (e.g. running in a + * jail that does not expose /dev/crypto or in + * capability mode). + */ + error = kern_openat(td, AT_FDCWD, "/dev/crypto", UIO_SYSSPACE, + O_RDWR, 0); + if (error == 0) + *(uint32_t *)data = td->td_retval[0]; + break; +#endif case CIOCGSESSION: case CIOCGSESSION2: if (cmd == CIOCGSESSION) { - session2_op_from_op(data, &thunk.sopc); + session2_op_from_op((void *)data, &thunk.sopc); sop = &thunk.sopc; } else sop = (struct session2_op *)data; error = cse_create(fcr, sop); if (cmd == CIOCGSESSION && error == 0) - session2_op_to_op(sop, data); + session2_op_to_op(sop, (void *)data); break; case CIOCFSESSION: ses = *(uint32_t *)data; @@ -1456,7 +1490,7 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, SDT_PROBE1(opencrypto, dev, ioctl, error, __LINE__); return (EINVAL); } - error = cryptodev_op(cse, cop, active_cred, td); + error = cryptodev_op(cse, cop); cse_free(cse); break; case CIOCKEY: @@ -1509,7 +1543,7 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, SDT_PROBE1(opencrypto, dev, ioctl, error, __LINE__); return (EINVAL); } - error = cryptodev_aead(cse, caead, active_cred, td); + error = cryptodev_aead(cse, caead); cse_free(cse); break; default: @@ -1522,109 +1556,33 @@ cryptof_ioctl(struct file *fp, u_long cmd, void *data, switch (cmd32) { case CIOCGSESSION32: if (error == 0) - session_op_to_32(data, data32); + session_op_to_32((void *)data, data32); break; case CIOCGSESSION232: if (error == 0) - session2_op_to_32(data, data32); + session2_op_to_32((void *)data, data32); break; case CIOCCRYPT32: if (error == 0) - crypt_op_to_32(data, data32); + crypt_op_to_32((void *)data, data32); break; case CIOCCRYPTAEAD32: if (error == 0) - crypt_aead_to_32(data, data32); + crypt_aead_to_32((void *)data, data32); break; case CIOCKEY32: case CIOCKEY232: - crypt_kop_to_32(data, data32); + crypt_kop_to_32((void *)data, data32); break; } #endif return (error); } -/* ARGSUSED */ -static int -cryptof_stat(struct file *fp, struct stat *sb, struct ucred *active_cred, - struct thread *td) -{ - - return (EOPNOTSUPP); -} - -/* ARGSUSED */ -static int -cryptof_close(struct file *fp, struct thread *td) -{ - struct fcrypt *fcr = fp->f_data; - struct csession *cse; - - while ((cse = TAILQ_FIRST(&fcr->csessions))) { - TAILQ_REMOVE(&fcr->csessions, cse, next); - KASSERT(cse->refs == 1, - ("%s: crypto session %p with %d refs", __func__, cse, - cse->refs)); - cse_free(cse); - } - free(fcr, M_XDATA); - fp->f_data = NULL; - return 0; -} - -static int -cryptof_fill_kinfo(struct file *fp, struct kinfo_file *kif, - struct filedesc *fdp) -{ - - kif->kf_type = KF_TYPE_CRYPTO; - return (0); -} - -static int -cryptoioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag, - struct thread *td) -{ - struct file *f; - struct fcrypt *fcr; - int fd, error; - - switch (cmd) { - case CRIOGET: - error = falloc_noinstall(td, &f); - if (error) - break; - - fcr = malloc(sizeof(struct fcrypt), M_XDATA, M_WAITOK | M_ZERO); - TAILQ_INIT(&fcr->csessions); - mtx_init(&fcr->lock, "fcrypt", NULL, MTX_DEF); - - finit(f, FREAD | FWRITE, DTYPE_CRYPTO, fcr, &cryptofops); - error = finstall(td, f, &fd, 0, NULL); - if (error) { - mtx_destroy(&fcr->lock); - free(fcr, M_XDATA); - } else - *(uint32_t *)data = fd; - fdrop(f, td); - break; - case CRIOFINDDEV: - error = cryptodev_find((struct crypt_find_op *)data); - break; - case CRIOASYMFEAT: - error = crypto_getfeat((int *)data); - break; - default: - error = EINVAL; - break; - } - return (error); -} - static struct cdevsw crypto_cdevsw = { .d_version = D_VERSION, - .d_ioctl = cryptoioctl, + .d_open = crypto_open, + .d_ioctl = crypto_ioctl, .d_name = "crypto", }; static struct cdev *crypto_dev; diff --git a/sys/opencrypto/cryptodev.h b/sys/opencrypto/cryptodev.h index b83a7c04674b..ecb1d929d1db 100644 --- a/sys/opencrypto/cryptodev.h +++ b/sys/opencrypto/cryptodev.h @@ -316,15 +316,6 @@ struct crypt_kop { #define CRF_DSA_VERIFY (1 << CRK_DSA_VERIFY) #define CRF_DH_COMPUTE_KEY (1 << CRK_DH_COMPUTE_KEY) -/* - * done against open of /dev/crypto, to get a cloned descriptor. - * Please use F_SETFD against the cloned descriptor. - */ -#define CRIOGET _IOWR('c', 100, uint32_t) -#define CRIOASYMFEAT CIOCASYMFEAT -#define CRIOFINDDEV CIOCFINDDEV - -/* the following are done against the cloned descriptor */ #define CIOCGSESSION _IOWR('c', 101, struct session_op) #define CIOCFSESSION _IOW('c', 102, uint32_t) #define CIOCCRYPT _IOWR('c', 103, struct crypt_op) diff --git a/sys/sys/user.h b/sys/sys/user.h index 800bf9497679..42bcc4f3ec3a 100644 --- a/sys/sys/user.h +++ b/sys/sys/user.h @@ -257,7 +257,7 @@ struct user { #define KF_TYPE_PIPE 3 #define KF_TYPE_FIFO 4 #define KF_TYPE_KQUEUE 5 -#define KF_TYPE_CRYPTO 6 +/* was KF_TYPE_CRYPTO 6 */ #define KF_TYPE_MQUEUE 7 #define KF_TYPE_SHM 8 #define KF_TYPE_SEM 9 diff --git a/tools/tools/crypto/cryptocheck.c b/tools/tools/crypto/cryptocheck.c index a61c3d2bae77..efb68f8966ef 100644 --- a/tools/tools/crypto/cryptocheck.c +++ b/tools/tools/crypto/cryptocheck.c @@ -352,23 +352,11 @@ crfind(int crid) bzero(&find, sizeof(find)); find.crid = crid; - if (ioctl(devcrypto(), CRIOFINDDEV, &find) == -1) + if (ioctl(devcrypto(), CIOCFINDDEV, &find) == -1) err(1, "ioctl(CIOCFINDDEV): crid %d", crid); return (find.name); } -static int -crget(void) -{ - int fd; - - if (ioctl(devcrypto(), CRIOGET, &fd) == -1) - err(1, "ioctl(CRIOGET)"); - if (fcntl(fd, F_SETFD, 1) == -1) - err(1, "fcntl(F_SETFD) (crget)"); - return fd; -} - static char rdigit(void) { @@ -436,11 +424,10 @@ ocf_init_session(struct session2_op *sop, const char *type, const char *name, { int fd; - fd = crget(); + fd = devcrypto(); if (ioctl(fd, CIOCGSESSION2, sop) < 0) { warn("cryptodev %s %s not supported for device %s", type, name, crfind(sop->crid)); - close(fd); ses->fd = -1; return (false); } @@ -458,8 +445,6 @@ ocf_destroy_session(struct ocf_session *ses) if (ioctl(ses->fd, CIOCFSESSION, &ses->ses) < 0) warn("ioctl(CIOCFSESSION)"); - - close(ses->fd); } static void diff --git a/usr.bin/procstat/procstat.1 b/usr.bin/procstat/procstat.1 index 8f05c730e55a..174f932a9dde 100644 --- a/usr.bin/procstat/procstat.1 +++ b/usr.bin/procstat/procstat.1 @@ -25,7 +25,7 @@ .\" .\" $FreeBSD$ .\" -.Dd October 5, 2020 +.Dd November 24, 2020 .Dt PROCSTAT 1 .Os .Sh NAME @@ -311,8 +311,6 @@ file path or socket addresses (if available) The following file descriptor types may be displayed: .Pp .Bl -tag -width X -compact -.It c -crypto .It e POSIX semaphore .It f diff --git a/usr.bin/procstat/procstat_files.c b/usr.bin/procstat/procstat_files.c index 660f421c31f1..74c6e48eab01 100644 --- a/usr.bin/procstat/procstat_files.c +++ b/usr.bin/procstat/procstat_files.c @@ -384,11 +384,6 @@ procstat_files(struct procstat *procstat, struct kinfo_proc *kipp) xo_emit("{eq:fd_type/kqueue}"); break; - case PS_FST_TYPE_CRYPTO: - str = "c"; - xo_emit("{eq:fd_type/crypto}"); - break; - case PS_FST_TYPE_MQUEUE: str = "m"; xo_emit("{eq:fd_type/mqueue}");