Download raw body.
isakmpd: avoid undefined function pointer casts
On Thu, Jul 17, 2025 at 08:39:17AM +0200, Theo Buehler wrote:
> On Wed, Jul 16, 2025 at 09:46:22PM +0200, Theo Buehler wrote:
> > This is a bit of a stupid diff with lots of ugly boilerplate, but I
> > don't see a better way. The problem is the second sentence of
> > C99 6.3.2.3, 8:
> >
> > A pointer to a function of one type may be converted to a pointer to a
> > function of another type and back again; the result shall compare equal
> > to the original pointer. If a converted pointer is used to call a
> > function whose type is not compatible with the pointed-to type, the
> > behavior is undefined.
> >
> > The notion of compatibility for the functions here boils down to
> > compatible return type and compatible argument list, which in particular
> > means the same number of arguments. See 6.7.5.3, 15 for the gory details.
> > There's also a version understandable to a human being lacking the brain
> > damage necessary for successfully deciphering the C99 standard's language:
> > https://en.cppreference.com/w/c/language/type.html
> >
> > So calling 'void CKSUM_Final(CKSUM_CTX *);' through the final member of
> > struct hash_function is clearly UB since the numbers of parameters don't
> > match. If that was all, the fix would be trivial. But the same goes for
> > void MD5Final(MD5_CTX *, char *); because MD5_CTX * and void * aren't
> > compatible in this sense: by definition, two pointer types are compatible
> > if they point to compatible types. Or, if you prefer: void * has looser
> > alignment requirements than MD5_CTX *, so they aren't compatible.
> >
> > The below avoids this by adding wrappers that have the correct types
> > and that then propagate their parameters in a defined way. As I said,
> > it's stupid and ugly and the one upside apart from appeasing the
> > language lawyers I can see is that we no longer have function pointer
> > casts.
> >
> > PS: there is similar code in isakmpd/hash.c.
>
> Here's the corresponding change for isakmpd:
>
ok mvs
> Index: hash.c
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/hash.c,v
> diff -u -p -r1.25 hash.c
> --- hash.c 21 Nov 2024 10:07:30 -0000 1.25
> +++ hash.c 17 Jul 2025 06:31:42 -0000
> @@ -43,7 +43,7 @@ void hmac_init(struct hash *, unsigned c
> void hmac_final(unsigned char *, struct hash *);
>
> /* Temporary hash contexts. */
> -static union {
> +static union ANY_CTX {
> MD5_CTX md5ctx;
> SHA1_CTX sha1ctx;
> SHA2_CTX sha2ctx;
> @@ -54,45 +54,135 @@ static unsigned char digest[HASH_MAX];
>
> /* Encapsulation of hash functions. */
>
> +static void
> +md5_init(union ANY_CTX *ctx)
> +{
> + MD5Init(&ctx->md5ctx);
> +}
> +
> +static void
> +md5_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
> +{
> + MD5Update(&ctx->md5ctx, data, len);
> +}
> +
> +static void
> +md5_final(unsigned char *digest, union ANY_CTX *ctx)
> +{
> + MD5Final(digest, &ctx->md5ctx);
> +}
> +
> +static void
> +sha1_init(union ANY_CTX *ctx)
> +{
> + SHA1Init(&ctx->sha1ctx);
> +}
> +
> +static void
> +sha1_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
> +{
> + SHA1Update(&ctx->sha1ctx, data, len);
> +}
> +
> +static void
> +sha1_final(unsigned char *digest, union ANY_CTX *ctx)
> +{
> + SHA1Final(digest, &ctx->sha1ctx);
> +}
> +
> +static void
> +sha256_init(union ANY_CTX *ctx)
> +{
> + SHA256Init(&ctx->sha2ctx);
> +}
> +
> +static void
> +sha256_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
> +{
> + SHA256Update(&ctx->sha2ctx, data, len);
> +}
> +
> +static void
> +sha256_final(unsigned char *digest, union ANY_CTX *ctx)
> +{
> + SHA256Final(digest, &ctx->sha2ctx);
> +}
> +
> +static void
> +sha384_init(union ANY_CTX *ctx)
> +{
> + SHA384Init(&ctx->sha2ctx);
> +}
> +
> +static void
> +sha384_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
> +{
> + SHA384Update(&ctx->sha2ctx, data, len);
> +}
> +
> +static void
> +sha384_final(unsigned char *digest, union ANY_CTX *ctx)
> +{
> + SHA384Final(digest, &ctx->sha2ctx);
> +}
> +
> +static void
> +sha512_init(union ANY_CTX *ctx)
> +{
> + SHA512Init(&ctx->sha2ctx);
> +}
> +
> +static void
> +sha512_update(union ANY_CTX *ctx, const unsigned char *data, size_t len)
> +{
> + SHA512Update(&ctx->sha2ctx, data, len);
> +}
> +
> +static void
> +sha512_final(unsigned char *digest, union ANY_CTX *ctx)
> +{
> + SHA512Final(digest, &ctx->sha2ctx);
> +}
> +
> static struct hash hashes[] = {
> {
> HASH_MD5, 5, MD5_SIZE, MD5_BLOCK_LENGTH, (void *)&Ctx.md5ctx, digest,
> sizeof(MD5_CTX), (void *)&Ctx2.md5ctx,
> - (void (*)(void *))MD5Init,
> - (void (*)(void *, unsigned char *, unsigned int))MD5Update,
> - (void (*)(unsigned char *, void *))MD5Final,
> + md5_init,
> + md5_update,
> + md5_final,
> hmac_init,
> hmac_final
> }, {
> HASH_SHA1, 6, SHA1_SIZE, SHA1_BLOCK_LENGTH, (void *)&Ctx.sha1ctx,
> digest, sizeof(SHA1_CTX), (void *)&Ctx2.sha1ctx,
> - (void (*)(void *))SHA1Init,
> - (void (*)(void *, unsigned char *, unsigned int))SHA1Update,
> - (void (*)(unsigned char *, void *))SHA1Final,
> + sha1_init,
> + sha1_update,
> + sha1_final,
> hmac_init,
> hmac_final
> }, {
> HASH_SHA2_256, 7, SHA2_256_SIZE, SHA256_BLOCK_LENGTH,
> (void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
> - (void (*)(void *))SHA256Init,
> - (void (*)(void *, unsigned char *, unsigned int))SHA256Update,
> - (void (*)(u_int8_t *, void *))SHA256Final,
> + sha256_init,
> + sha256_update,
> + sha256_final,
> hmac_init,
> hmac_final
> }, {
> HASH_SHA2_384, 8, SHA2_384_SIZE, SHA384_BLOCK_LENGTH,
> (void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
> - (void (*)(void *))SHA384Init,
> - (void (*)(void *, unsigned char *, unsigned int))SHA384Update,
> - (void (*)(u_int8_t *, void *))SHA384Final,
> + sha384_init,
> + sha384_update,
> + sha384_final,
> hmac_init,
> hmac_final
> }, {
> HASH_SHA2_512, 9, SHA2_512_SIZE, SHA512_BLOCK_LENGTH,
> (void *)&Ctx.sha2ctx, digest, sizeof(SHA2_CTX), (void *)&Ctx2.sha2ctx,
> - (void (*)(void *))SHA512Init,
> - (void (*)(void *, unsigned char *, unsigned int))SHA512Update,
> - (void (*)(u_int8_t *, void *))SHA512Final,
> + sha512_init,
> + sha512_update,
> + sha512_final,
> hmac_init,
> hmac_final
> }
> Index: hash.h
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/hash.h,v
> diff -u -p -r1.9 hash.h
> --- hash.h 21 Nov 2024 10:07:30 -0000 1.9
> +++ hash.h 17 Jul 2025 06:34:50 -0000
> @@ -49,6 +49,8 @@ enum hashes {
> HASH_SHA2_512
> };
>
> +union ANY_CTX;
> +
> struct hash {
> enum hashes type;
> int id; /* ISAKMP/Oakley ID */
> @@ -58,9 +60,9 @@ struct hash {
> unsigned char *digest; /* Pointer to a digest */
> int ctxsize;
> void *ctx2; /* Pointer to a 2nd context, for HMAC octx */
> - void (*Init) (void *);
> - void (*Update) (void *, unsigned char *, unsigned int);
> - void (*Final) (unsigned char *, void *);
> + void (*Init) (union ANY_CTX *);
> + void (*Update) (union ANY_CTX *, const unsigned char *, size_t);
> + void (*Final) (unsigned char *, union ANY_CTX *);
> void (*HMACInit) (struct hash *, unsigned char *, unsigned int);
> void (*HMACFinal) (unsigned char *, struct hash *);
> };
>
isakmpd: avoid undefined function pointer casts