Use switch instead of if when evaluating curves

Previously, the code would do:

    REQUIRE(alg == CURVE1 || alg == CURVE2);

    [...]

    if (alg == CURVE1) { /* code for CURVE1 */ }
    else { /* code for CURVE2 */ }

This approach is less extensible and also more prone to errors in case
the initial REQUIRE() is forgotten.  The code has been refactored to
use:

    REQUIRE(alg == CURVE1 || alg == CURVE2);

    [...]

    switch (alg) {
    case CURVE1: /* code for CURVE1 */; break;
    case CURVE2: /* code for CURVE2 */; break;
    default: INSIST(0);
    }

diff --git a/lib/dns/pkcs11ecdsa_link.c b/lib/dns/pkcs11ecdsa_link.c
index 8e28a022d9b7d431788a6c3fbc68d542f8820fd2..dbbdc48b653cf7db1e7ff035a9cbb71f4b7c6f83 100644 (file)
--- a/lib/dns/pkcs11ecdsa_link.c
+++ b/lib/dns/pkcs11ecdsa_link.c
@@ -179,12 +179,18 @@ pkcs11ecdsa_sign(dst_context_t *dctx, isc_buffer_t *sig) {
                key->key_alg == DST_ALG_ECDSA384);
        REQUIRE(ec != NULL);
 
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                dgstlen = ISC_SHA256_DIGESTLENGTH;
                siglen = DNS_SIG_ECDSA256SIZE;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                siglen = DNS_SIG_ECDSA384SIZE;
                dgstlen = ISC_SHA384_DIGESTLENGTH;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        PK11_RET(pkcs_C_DigestFinal, (pk11_ctx->session, digest, &dgstlen),
@@ -293,10 +299,16 @@ pkcs11ecdsa_verify(dst_context_t *dctx, const isc_region_t *sig) {
                key->key_alg == DST_ALG_ECDSA384);
        REQUIRE(ec != NULL);
 
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                dgstlen = ISC_SHA256_DIGESTLENGTH;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                dgstlen = ISC_SHA384_DIGESTLENGTH;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        PK11_RET(pkcs_C_DigestFinal, (pk11_ctx->session, digest, &dgstlen),
@@ -419,19 +431,24 @@ pkcs11ecdsa_compare(const dst_key_t *key1, const dst_key_t *key2) {
 }
 
 #define SETCURVE()                                                       \
-       if (key->key_alg == DST_ALG_ECDSA256) {                          \
+       switch (key->key_alg) {                                          \
+       case DST_ALG_ECDSA256:                                           \
                attr->pValue = isc_mem_get(key->mctx,                    \
                                           sizeof(PK11_ECC_PRIME256V1)); \
                memmove(attr->pValue, PK11_ECC_PRIME256V1,               \
                        sizeof(PK11_ECC_PRIME256V1));                    \
                attr->ulValueLen = sizeof(PK11_ECC_PRIME256V1);          \
-       } else {                                                         \
+               break;                                                   \
+       case DST_ALG_ECDSA384:                                           \
                attr->pValue = isc_mem_get(key->mctx,                    \
                                           sizeof(PK11_ECC_SECP384R1));  \
-                                                                         \
                memmove(attr->pValue, PK11_ECC_SECP384R1,                \
                        sizeof(PK11_ECC_SECP384R1));                     \
                attr->ulValueLen = sizeof(PK11_ECC_SECP384R1);           \
+               break;                                                   \
+       default:                                                         \
+               INSIST(0);                                               \
+               ISC_UNREACHABLE();                                       \
        }
 
 #define FREECURVE()                                                     \
@@ -532,10 +549,16 @@ pkcs11ecdsa_generate(dst_key_t *key, int unused, void (*callback)(int)) {
        memset(pk11_ctx, 0, sizeof(*pk11_ctx));
        isc_mem_put(key->mctx, pk11_ctx, sizeof(*pk11_ctx));
 
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                key->key_size = DNS_KEY_ECDSA256SIZE * 4;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                key->key_size = DNS_KEY_ECDSA384SIZE * 4;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        return (ISC_R_SUCCESS);
@@ -607,10 +630,16 @@ pkcs11ecdsa_todns(const dst_key_t *key, isc_buffer_t *data) {
 
        REQUIRE(key->keydata.pkey != NULL);
 
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                len = DNS_KEY_ECDSA256SIZE;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                len = DNS_KEY_ECDSA384SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        ec = key->keydata.pkey;
@@ -643,10 +672,16 @@ pkcs11ecdsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
        REQUIRE(key->key_alg == DST_ALG_ECDSA256 ||
                key->key_alg == DST_ALG_ECDSA384);
 
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                len = DNS_KEY_ECDSA256SIZE;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                len = DNS_KEY_ECDSA384SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        isc_buffer_remainingregion(data, &r);
@@ -664,20 +699,8 @@ pkcs11ecdsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
 
        attr = ec->repr;
        attr->type = CKA_EC_PARAMS;
-       if (key->key_alg == DST_ALG_ECDSA256) {
-               attr->pValue = isc_mem_get(key->mctx,
-                                          sizeof(PK11_ECC_PRIME256V1));
-               memmove(attr->pValue, PK11_ECC_PRIME256V1,
-                       sizeof(PK11_ECC_PRIME256V1));
-               attr->ulValueLen = sizeof(PK11_ECC_PRIME256V1);
-       } else {
-               attr->pValue = isc_mem_get(key->mctx,
-                                          sizeof(PK11_ECC_SECP384R1));
+       SETCURVE();
 
-               memmove(attr->pValue, PK11_ECC_SECP384R1,
-                       sizeof(PK11_ECC_SECP384R1));
-               attr->ulValueLen = sizeof(PK11_ECC_SECP384R1);
-       }
        attr++;
        attr->type = CKA_EC_POINT;
        attr->pValue = isc_mem_get(key->mctx, len + 3);
@@ -945,10 +968,16 @@ pkcs11ecdsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
 
        dst__privstruct_free(&priv, mctx);
        memset(&priv, 0, sizeof(priv));
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                key->key_size = DNS_KEY_ECDSA256SIZE * 4;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                key->key_size = DNS_KEY_ECDSA384SIZE * 4;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        return (ISC_R_SUCCESS);
@@ -1061,10 +1090,16 @@ pkcs11ecdsa_fromlabel(dst_key_t *key, const char *engine, const char *label,
        }
 
        key->label = isc_mem_strdup(key->mctx, label);
-       if (key->key_alg == DST_ALG_ECDSA256) {
+       switch (key->key_alg) {
+       case DST_ALG_ECDSA256:
                key->key_size = DNS_KEY_ECDSA256SIZE * 4;
-       } else {
+               break;
+       case DST_ALG_ECDSA384:
                key->key_size = DNS_KEY_ECDSA384SIZE * 4;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        pk11_return_session(pk11_ctx);

diff --git a/lib/dns/pkcs11eddsa_link.c b/lib/dns/pkcs11eddsa_link.c
index 4d3397ce74b182bdbb672d97b1438963e6a46358..17941287324466b65828ad23900536de8024fca1 100644 (file)
--- a/lib/dns/pkcs11eddsa_link.c
+++ b/lib/dns/pkcs11eddsa_link.c
@@ -137,10 +137,16 @@ pkcs11eddsa_sign(dst_context_t *dctx, isc_buffer_t *sig) {
                key->key_alg == DST_ALG_ED448);
        REQUIRE(ec != NULL);
 
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                siglen = DNS_SIG_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                siglen = DNS_SIG_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        pk11_ctx = isc_mem_get(dctx->mctx, sizeof(*pk11_ctx));
@@ -396,16 +402,22 @@ pkcs11eddsa_compare(const dst_key_t *key1, const dst_key_t *key2) {
 }
 
 #define SETCURVE()                                                             \
-       if (key->key_alg == DST_ALG_ED25519) {                                 \
+       switch (key->key_alg) {                                                \
+       case DST_ALG_ED25519:                                                  \
                attr->pValue = isc_mem_get(key->mctx,                          \
                                           sizeof(PK11_ECX_ED25519));          \
                memmove(attr->pValue, PK11_ECX_ED25519,                        \
                        sizeof(PK11_ECX_ED25519));                             \
                attr->ulValueLen = sizeof(PK11_ECX_ED25519);                   \
-       } else {                                                               \
+               break;                                                         \
+       case DST_ALG_ED448:                                                    \
                attr->pValue = isc_mem_get(key->mctx, sizeof(PK11_ECX_ED448)); \
                memmove(attr->pValue, PK11_ECX_ED448, sizeof(PK11_ECX_ED448)); \
                attr->ulValueLen = sizeof(PK11_ECX_ED448);                     \
+               break;                                                         \
+       default:                                                               \
+               INSIST(0);                                                     \
+               ISC_UNREACHABLE();                                             \
        }
 
 #define FREECURVE()                                                     \
@@ -507,10 +519,16 @@ pkcs11eddsa_generate(dst_key_t *key, int unused, void (*callback)(int)) {
        memset(pk11_ctx, 0, sizeof(*pk11_ctx));
        isc_mem_put(key->mctx, pk11_ctx, sizeof(*pk11_ctx));
 
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                key->key_size = DNS_KEY_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                key->key_size = DNS_KEY_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        return (ISC_R_SUCCESS);
@@ -582,10 +600,16 @@ pkcs11eddsa_todns(const dst_key_t *key, isc_buffer_t *data) {
 
        REQUIRE(key->keydata.pkey != NULL);
 
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                len = DNS_KEY_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                len = DNS_KEY_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        ec = key->keydata.pkey;
@@ -614,10 +638,16 @@ pkcs11eddsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
        REQUIRE(key->key_alg == DST_ALG_ED25519 ||
                key->key_alg == DST_ALG_ED448);
 
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                len = DNS_KEY_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                len = DNS_KEY_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        isc_buffer_remainingregion(data, &r);
@@ -635,16 +665,8 @@ pkcs11eddsa_fromdns(dst_key_t *key, isc_buffer_t *data) {
 
        attr = ec->repr;
        attr->type = CKA_EC_PARAMS;
-       if (key->key_alg == DST_ALG_ED25519) {
-               attr->pValue = isc_mem_get(key->mctx, sizeof(PK11_ECX_ED25519));
-               memmove(attr->pValue, PK11_ECX_ED25519,
-                       sizeof(PK11_ECX_ED25519));
-               attr->ulValueLen = sizeof(PK11_ECX_ED25519);
-       } else {
-               attr->pValue = isc_mem_get(key->mctx, sizeof(PK11_ECX_ED448));
-               memmove(attr->pValue, PK11_ECX_ED448, sizeof(PK11_ECX_ED448));
-               attr->ulValueLen = sizeof(PK11_ECX_ED448);
-       }
+       SETCURVE();
+
        attr++;
        attr->type = CKA_EC_POINT;
        attr->pValue = isc_mem_get(key->mctx, len);
@@ -907,10 +929,16 @@ pkcs11eddsa_parse(dst_key_t *key, isc_lex_t *lexer, dst_key_t *pub) {
 
        dst__privstruct_free(&priv, mctx);
        memset(&priv, 0, sizeof(priv));
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                key->key_size = DNS_KEY_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                key->key_size = DNS_KEY_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        return (ISC_R_SUCCESS);
@@ -1024,10 +1052,16 @@ pkcs11eddsa_fromlabel(dst_key_t *key, const char *engine, const char *label,
        }
 
        key->label = isc_mem_strdup(key->mctx, label);
-       if (key->key_alg == DST_ALG_ED25519) {
+       switch (key->key_alg) {
+       case DST_ALG_ED25519:
                key->key_size = DNS_KEY_ED25519SIZE;
-       } else {
+               break;
+       case DST_ALG_ED448:
                key->key_size = DNS_KEY_ED448SIZE;
+               break;
+       default:
+               INSIST(0);
+               ISC_UNREACHABLE();
        }
 
        pk11_return_session(pk11_ctx);