Re: Re: Re: bin/58212: cgdconfig(8): Add zfs verification method

[Malte Dehling <mdehling%gmail.com@localhost>]

Hi,

On Sun, Apr 28, 2024 at 01:13:05PM -0700, Malte Dehling wrote:
> >Submitter-Id:  net
> >Originator:    Malte Dehling
> >Organization:
> >Confidential:  no
> >Synopsis:      cgdconfig(8): Add zfs verification method
> >Severity:      non-critical
> >Priority:      medium
> >Category:      bin
> >Class:         change-request
> >Release:       NetBSD 10.0, -current
> >Environment:
>     NetBSD 10.0 (GENERIC) #4: Wed Apr 24 12:21:26 PDT 2024
>     mdehling@nb-base-dev:/scratch/obj/sys/arch/amd64/compile/GENERIC amd64
>
> >Description:
> I would like to propose the attached patch to add a zfs verification
> method to the cgdconfig(8) program.  With such a method, it will be
> possible to use cgd devices as vdevs in a zpool without an intermediate
> gpt.

How about this version instead?

I _think_ I got the byte order logic right:
- deduce the byte order from the magic bytes and use it to initialize
  the checksum computation with the same byte order
- on-disk sha256 is always stored in big-endian order, so convert
  whatever we compute to BE before comparing

Tested only on LE since I don't have a BE netbsd system handy right now.


---
Implement vdev label checksum verification.  Allows using raw cgd
devices as vdevs in a zpool.
---
 sbin/cgdconfig/Makefile    |  15 +++++
 sbin/cgdconfig/cgdconfig.8 |   2 +
 sbin/cgdconfig/cgdconfig.c | 111 +++++++++++++++++++++++++++++++++++++
 sbin/cgdconfig/params.c    |   9 +++
 sbin/cgdconfig/params.h    |   1 +
 5 files changed, 138 insertions(+)

diff --git a/sbin/cgdconfig/Makefile b/sbin/cgdconfig/Makefile
index fec75b4e375..0ed669ab738 100644
--- a/sbin/cgdconfig/Makefile
+++ b/sbin/cgdconfig/Makefile
@@ -29,4 +29,19 @@ ARGON2_NO_THREADS=1
 .include "${NETBSDSRCDIR}/external/apache2/argon2/lib/libargon2/Makefile.inc"
 .endif

+.if ${MKZFS} != "no"
+CPPFLAGS+=	-DHAVE_ZFS
+
+OSNET=${NETBSDSRCDIR}/external/cddl/osnet
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/include
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/sys
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/dist/head
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/dist/lib/libzpool/common
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/dist/uts/common
+CPPFLAGS.cgdconfig.c+=	-I${OSNET}/dist/uts/common/fs/zfs
+
+COPTS.cgdconfig.c+=	-Wno-unknown-pragmas
+COPTS.cgdconfig.c+=	-Wno-strict-prototypes
+.endif
+
 .include <bsd.prog.mk>
diff --git a/sbin/cgdconfig/cgdconfig.8 b/sbin/cgdconfig/cgdconfig.8
index 2b0da2cb732..e48ccde02c5 100644
--- a/sbin/cgdconfig/cgdconfig.8
+++ b/sbin/cgdconfig/cgdconfig.8
@@ -270,6 +270,8 @@ scan for a valid Master Boot Record.
 scan for a valid GUID partition table.
 .It ffs
 scan for a valid FFS file system.
+.It zfs
+scan for a valid ZFS vdev label (if compiled with MKZFS).
 .It re-enter
 prompt for passphrase twice, and ensure entered passphrases are
 identical.
diff --git a/sbin/cgdconfig/cgdconfig.c b/sbin/cgdconfig/cgdconfig.c
index 9a634ef37e2..de4d09013bf 100644
--- a/sbin/cgdconfig/cgdconfig.c
+++ b/sbin/cgdconfig/cgdconfig.c
@@ -73,6 +73,11 @@ __RCSID("$NetBSD: cgdconfig.c,v 1.61 2022/11/17
06:40:38 chs Exp $");

 #include <ufs/ffs/fs.h>

+#ifdef HAVE_ZFS
+#include <sys/vdev_impl.h>
+#include <sha2.h>
+#endif
+
 #include "params.h"
 #include "pkcs5_pbkdf2.h"
 #include "utils.h"
@@ -170,6 +175,9 @@ static int	 verify_ffs(int);
 static int	 verify_reenter(struct params *);
 static int	 verify_mbr(int);
 static int	 verify_gpt(int);
+#ifdef HAVE_ZFS
+static int	 verify_zfs(int);
+#endif

 __dead static void	 usage(void);

@@ -1024,6 +1032,10 @@ verify(struct params *p, int fd)
 		return verify_mbr(fd);
 	case VERIFY_GPT:
 		return verify_gpt(fd);
+#ifdef HAVE_ZFS
+	case VERIFY_ZFS:
+		return verify_zfs(fd);
+#endif
 	default:
 		warnx("unimplemented verification method");
 		return -1;
@@ -1182,6 +1194,105 @@ verify_gpt(int fd)
 	return ret;
 }

+#ifdef HAVE_ZFS
+
+#define ZIO_CHECKSUM_BE(zcp)					\
+{								\
+	(zcp)->zc_word[0] = BE_64((zcp)->zc_word[0]);		\
+	(zcp)->zc_word[1] = BE_64((zcp)->zc_word[1]);		\
+	(zcp)->zc_word[2] = BE_64((zcp)->zc_word[2]);		\
+	(zcp)->zc_word[3] = BE_64((zcp)->zc_word[3]);		\
+}
+
+static int
+verify_zfs(int fd)
+{
+	struct stat sb;
+	vdev_phys_t *vdev_phys;
+	off_t vdev_phys_off;
+	ssize_t ret;
+	bool byteswap;
+	zio_cksum_t cksum_found, cksum_real;
+	SHA256_CTX ctx;
+
+	if (fstat(fd, &sb)) {
+		warn("fstat");
+		return 1;
+	} else if (sb.st_size == 0) {
+		warnx("fstat returned size zero");
+		return 1;
+	}
+
+	vdev_phys = emalloc(sizeof *vdev_phys);
+	for (int l = 0; l < VDEV_LABELS; l++) {
+		vdev_phys_off = ( l < VDEV_LABELS/2 ?
+		    l * sizeof(vdev_label_t) :
+		    sb.st_size - (VDEV_LABELS-l) * sizeof(vdev_label_t) ) \
+		    + offsetof(vdev_label_t, vl_vdev_phys);
+
+		ret = prog_pread(fd, vdev_phys, sizeof *vdev_phys,
+		    vdev_phys_off);
+		if (ret < 0) {
+			warn("pread");
+			ret = 1;
+			break;
+		} else if ((size_t)ret < sizeof *vdev_phys) {
+			warnx("pread: incomplete block");
+			ret = 1;
+			break;
+		}
+
+		ret = 0;
+
+		if (vdev_phys->vp_zbt.zec_magic == BSWAP_64(ZEC_MAGIC))
+			byteswap = true;
+		else if (vdev_phys->vp_zbt.zec_magic == ZEC_MAGIC)
+			byteswap = false;
+		else {
+			ret = 1;
+			break;
+		};
+
+		cksum_found = vdev_phys->vp_zbt.zec_cksum;
+
+		ZIO_SET_CHECKSUM(&vdev_phys->vp_zbt.zec_cksum,
+		    vdev_phys_off, 0, 0, 0);
+		if (byteswap)
+			ZIO_CHECKSUM_BSWAP(&vdev_phys->vp_zbt.zec_cksum);
+
+		SHA256Init(&ctx);
+		SHA256Update(&ctx, (uint8_t *)vdev_phys, sizeof *vdev_phys);
+		SHA256Final(&cksum_real, &ctx);
+
+		/*
+		 * For historical reasons the on-disk sha256 checksums are
+		 * always in big endian format.
+		 * (see cddl/osnet/dist/uts/common/fs/zfs/sha256.c)
+		 */
+		ZIO_CHECKSUM_BE(&cksum_real);
+
+		if (!ZIO_CHECKSUM_EQUAL(cksum_found, cksum_real)) {
+			warnx("checksum mismatch on vdev label %d", l);
+			warnx("found %lx, %lx, %lx, %lx",
+			    (unsigned long)cksum_found.zc_word[0],
+			    (unsigned long)cksum_found.zc_word[1],
+			    (unsigned long)cksum_found.zc_word[2],
+			    (unsigned long)cksum_found.zc_word[3]);
+			warnx("expected %lx, %lx, %lx, %lx",
+			    (unsigned long)cksum_real.zc_word[0],
+			    (unsigned long)cksum_real.zc_word[1],
+			    (unsigned long)cksum_real.zc_word[2],
+			    (unsigned long)cksum_real.zc_word[3]);
+			ret = 1;
+			break;
+		}
+	}
+	free(vdev_phys);
+	return ret;
+}
+
+#endif
+
 static off_t sblock_try[] = SBLOCKSEARCH;

 static int
diff --git a/sbin/cgdconfig/params.c b/sbin/cgdconfig/params.c
index 91af02c21cc..5da25ec0595 100644
--- a/sbin/cgdconfig/params.c
+++ b/sbin/cgdconfig/params.c
@@ -287,6 +287,10 @@ params_verify_method(string_t *in)
 		p->verify_method = VERIFY_MBR;
 	if (!strcmp("gpt", vm))
 		p->verify_method = VERIFY_GPT;
+#ifdef HAVE_ZFS
+	if (!strcmp("zfs", vm))
+		p->verify_method = VERIFY_ZFS;
+#endif

 	string_free(in);

@@ -1065,6 +1069,11 @@ params_fput(struct params *p, FILE *f)
 	case VERIFY_GPT:
 		print_kvpair_cstr(f, ts, "verify_method", "gpt");
 		break;
+#ifdef HAVE_ZFS
+	case VERIFY_ZFS:
+		print_kvpair_cstr(f, ts, "verify_method", "zfs");
+		break;
+#endif
 	default:
 		warnx("unsupported verify_method (%d)", p->verify_method);
 		return -1;
diff --git a/sbin/cgdconfig/params.h b/sbin/cgdconfig/params.h
index fb79a0deb80..ef49dde6aba 100644
--- a/sbin/cgdconfig/params.h
+++ b/sbin/cgdconfig/params.h
@@ -81,6 +81,7 @@ struct params {
 #define VERIFY_REENTER		0x4
 #define VERIFY_MBR      	0x5
 #define VERIFY_GPT      	0x6
+#define VERIFY_ZFS      	0x7

 /* shared key derivation methods */

-- 
2.44.0


Cheers,
-- 
Malte Dehling