From 737833e14eaaf4efbe926f6f0acf14f477d081e6 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 13 Nov 2023 13:45:40 -0500 Subject: [PATCH 1/3] efikeygen: move knowledge of key size and exponents up a level We need to be able to generate keys other than just RSA2048 now. As a result, we need to have the key generation data determined outside of generate_keys() itself. This makes those parameters part of the generate_keys() call, and moves the default values into efikeygen itself. Signed-off-by: Peter Jones --- src/cms_common.c | 7 ++++--- src/cms_common.h | 3 ++- src/efikeygen.c | 5 ++++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cms_common.c b/src/cms_common.c index 1ca0b7b..d05c7f9 100644 --- a/src/cms_common.c +++ b/src/cms_common.c @@ -1783,11 +1783,12 @@ generate_auth_info(cms_context *cms, SECItem *der, char *url) int generate_keys(cms_context *cms, PK11SlotInfo *slot, - SECKEYPrivateKey **privkey, SECKEYPublicKey **pubkey) + SECKEYPrivateKey **privkey, SECKEYPublicKey **pubkey, + int key_bits, unsigned long exponent) { PK11RSAGenParams rsaparams = { - .keySizeInBits = 2048, - .pe = 0x010001, + .keySizeInBits = key_bits, + .pe = exponent, }; SECStatus rv; diff --git a/src/cms_common.h b/src/cms_common.h index 35a128a..692f2d7 100644 --- a/src/cms_common.h +++ b/src/cms_common.h @@ -222,7 +222,8 @@ extern int generate_signature(cms_context *ctx); extern int unlock_nss_token(cms_context *ctx); extern int find_certificate(cms_context *ctx, int needs_private_key); extern int generate_keys(cms_context *cms, PK11SlotInfo *slot, - SECKEYPrivateKey **privkey, SECKEYPublicKey **pubkey); + SECKEYPrivateKey **privkey, SECKEYPublicKey **pubkey, + int key_bits, unsigned long exponent); extern int is_issuer_of(CERTCertificate *c0, CERTCertificate *c1); typedef int (find_cert_match_t)(CERTCertificate *cert, void *cbdata); diff --git a/src/efikeygen.c b/src/efikeygen.c index 010d7cc..53ad077 100644 --- a/src/efikeygen.c +++ b/src/efikeygen.c @@ -716,6 +716,8 @@ int main(int argc, char *argv[]) PRStatus prstatus; void *frees[50] = { NULL, }; int nfrees = 0; + int key_bits = 2048; + unsigned long exponent = 0x010001ul; cms_context *cms = NULL; @@ -1022,7 +1024,8 @@ int main(int argc, char *argv[]) nsserr(1, "could not find NSS slot for token \"%s\"", cms->tokenname); - rc = generate_keys(cms, slot, &privkey, &pubkey); + rc = generate_keys(cms, slot, &privkey, &pubkey, key_bits, + exponent); } if (rc < 0) exit(1); From 1793a672c1ec46894d7893834b6b0201f4c231db Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 13 Nov 2023 14:14:32 -0500 Subject: [PATCH 2/3] efikeygen: Add support for RSA3072 and RSA4096 This adds a "--algorithm" flag to which you can pass rsa2048, rsa3072, and rsa4096. Signed-off-by: Peter Jones --- src/efikeygen.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/src/efikeygen.c b/src/efikeygen.c index 53ad077..f09ea88 100644 --- a/src/efikeygen.c +++ b/src/efikeygen.c @@ -688,6 +688,31 @@ long verbosity(void) return verbose; } +struct algorithm { + char name[16]; + int key_bits; + unsigned long exponent; +}; + +struct algorithm algorithms[] = { + {.name = "rsa2048", + .key_bits = 2048, + .exponent = 0x010001ul, + }, + {.name = "rsa3072", + .key_bits = 3072, + .exponent = 0x010001ul, + }, + {.name = "rsa4096", + .key_bits = 4096, + .exponent = 0x010001ul, + }, + {.name = "", + .key_bits = 0, + .exponent = 0, + } +}; + int main(int argc, char *argv[]) { int is_ca = 0; @@ -718,6 +743,8 @@ int main(int argc, char *argv[]) int nfrees = 0; int key_bits = 2048; unsigned long exponent = 0x010001ul; + char *orig_algo = "rsa2048"; + char *algo = orig_algo; cms_context *cms = NULL; @@ -760,6 +787,12 @@ int main(int argc, char *argv[]) .descrip = "Generate a self-signed certificate" }, /* stuff about the generated key */ + {.longName = "algorithm", + .shortName = 'a', + .argInfo = POPT_ARG_STRING|POPT_ARGFLAG_SHOW_DEFAULT, + .arg = &algo, + .descrip = "Algorithm for keys", + .argDescrip = "" }, {.longName = "kek", .shortName = 'K', .argInfo = POPT_ARG_VAL|POPT_ARGFLAG_OR|POPT_ARGFLAG_DOC_HIDDEN, @@ -917,6 +950,7 @@ int main(int argc, char *argv[]) while ((rc = poptGetNextOpt(optCon)) > 0) { switch (rc) { + case 'a': frees[nfrees++] = algo; break; case 'c': frees[nfrees++] = cn; break; case 'D': frees[nfrees++] = db_path; break; case 'd': frees[nfrees++] = dbdir; break; @@ -943,6 +977,14 @@ int main(int argc, char *argv[]) poptFreeContext(optCon); + if (strcmp(algo, "help") == 0) { + printf("Supported algorithms:"); + for (int i = 0; algorithms[i].name[0] != '\0'; i++) + printf(" %s", algorithms[i].name); + printf("\n"); + exit(0); + } + /* * Scenarios that are okay (x == valid combination) * @@ -971,6 +1013,16 @@ int main(int argc, char *argv[]) if (!is_self_signed && !signer) errx(1, "signing certificate is required"); + for (int i=0; true; i++) { + if (strcmp(algorithms[i].name, "") == 0) + errx(1, "invalid algorithm: \"%s\"", algo); + if (strcmp(algorithms[i].name, algo) == 0) { + key_bits = algorithms[i].key_bits; + exponent = algorithms[i].exponent; + break; + } + } + cms->tokenname = tokenname; cms->certname = signer; From 7aa22f8ad82427006fcc9b737aadefc2a69eac9d Mon Sep 17 00:00:00 2001 From: Egor Ignatov Date: Thu, 7 Mar 2024 13:07:27 -0500 Subject: [PATCH 3/3] efikeygen: Account for the signature size in bundle_signature() In ea7a2c41f92d, when bundling the signature, the bitstring type field is being set manually with a hacky offset. That offset is only valid with specific signature types, and so with any signature of a different size, this is just corrupting data either in the signature or after it. This change from Egor fixes the egregious hack to manually set the type so that it computes the location based on the signature length, rather than hard-coding a value. Signed-off-by: Peter Jones --- src/efikeygen.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/efikeygen.c b/src/efikeygen.c index f09ea88..74e6011 100644 --- a/src/efikeygen.c +++ b/src/efikeygen.c @@ -141,7 +141,8 @@ bundle_signature(cms_context *cms, SECItem *sigder, SECItem *data, errx(1, "could not encode certificate: %s", PORT_ErrorToString(PORT_GetError())); - sigder->data[sigder->len - 261] = DER_BIT_STRING; + //Note: offset is signature size + 5 bytes for DER encoding + sigder->data[sigder->len - (signature->len + 5)] = DER_BIT_STRING; return 0; }