Mailing List Archive

[PATCH 2/4] add signing option -O hashalg=algorithm
---
ssh-keygen.1 | 4 ++++
ssh-keygen.c | 36 ++++++++++++++++++++++++------------
2 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/ssh-keygen.1 b/ssh-keygen.1
index 58b1f682..f0f211c4 100644
--- a/ssh-keygen.1
+++ b/ssh-keygen.1
@@ -161,6 +161,7 @@
.Fl s Ar signature_file
.Nm ssh-keygen
.Fl Y Cm sign
+.Op Fl O Ar option
.Fl f Ar key_file
.Fl n Ar namespace
.Ar
@@ -541,6 +542,9 @@ When performing signature-related options using the
.Fl Y
flag, the following options are accepted:
.Bl -tag -width Ds
+.It Cm hashalg Ns = Ns Ar algorithm
+Selects the hash algorithm to use for hashing the message to be signed.
+Valid algorithms are sha256 and sha512. The default is sha512.
.It Cm print-pubkey
Print the full public key to standard output after signature verification.
.It Cm verify-time Ns = Ns Ar timestamp
diff --git a/ssh-keygen.c b/ssh-keygen.c
index d31dc503..ef99b9b6 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -142,6 +142,9 @@ struct cert_ext {
static struct cert_ext *cert_ext;
static size_t ncert_ext;

+/* Default hash algorithm for -Y sign and verify. */
+#define DEFAULT_SIGN_HASHALG_NAME "sha512"
+
/* Conversion to/from various formats */
enum {
FMT_RFC4716,
@@ -2512,7 +2515,7 @@ load_sign_key(const char *keypath, const struct sshkey *pubkey)

static int
sign_one(struct sshkey *signkey, const char *filename, int fd,
- const char *sig_namespace, sshsig_signer *signer, void *signer_ctx)
+ const char *sig_namespace, const char *hashalg, sshsig_signer *signer, void *signer_ctx)
{
struct sshbuf *sigbuf = NULL, *abuf = NULL;
int r = SSH_ERR_INTERNAL_ERROR, wfd = -1, oerrno;
@@ -2542,7 +2545,7 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
free(fp);
}
}
- if ((r = sshsig_sign_fd(signkey, NULL, sk_provider, pin,
+ if ((r = sshsig_sign_fd(signkey, hashalg, sk_provider, pin,
fd, sig_namespace, &sigbuf, signer, signer_ctx)) != 0) {
error_r(r, "Signing %s failed", filename);
goto out;
@@ -2603,18 +2606,23 @@ sign_one(struct sshkey *signkey, const char *filename, int fd,
}

static int
-sig_process_opts(char * const *opts, size_t nopts, uint64_t *verify_timep,
+sig_process_opts(char * const *opts, size_t nopts, char *hashalg, size_t hashalg_size, uint64_t *verify_timep,
int *print_pubkey)
{
size_t i;
time_t now;

+ if (hashalg != NULL)
+ strlcpy(hashalg, DEFAULT_SIGN_HASHALG_NAME, hashalg_size);
if (verify_timep != NULL)
*verify_timep = 0;
if (print_pubkey != NULL)
*print_pubkey = 0;
for (i = 0; i < nopts; i++) {
- if (verify_timep &&
+ if (hashalg &&
+ strncasecmp(opts[i], "hashalg=", 8) == 0) {
+ strlcpy(hashalg, opts[i] + 8, hashalg_size);
+ } else if (verify_timep &&
strncasecmp(opts[i], "verify-time=", 12) == 0) {
if (parse_absolute_time(opts[i] + 12,
verify_timep) != 0 || *verify_timep == 0) {
@@ -2640,12 +2648,13 @@ sig_process_opts(char * const *opts, size_t nopts, uint64_t *verify_timep,
}

static int
-sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
+sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv, char * const *opts, size_t nopts)
{
int i, fd = -1, r, ret = -1;
int agent_fd = -1;
struct sshkey *pubkey = NULL, *privkey = NULL, *signkey = NULL;
sshsig_signer *signer = NULL;
+ char hashalg[7]; /* "shaXXX" */

/* Check file arguments. */
for (i = 0; i < argc; i++) {
@@ -2655,6 +2664,9 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
fatal("Cannot sign mix of paths and standard input");
}

+ if (sig_process_opts(opts, nopts, hashalg, sizeof(hashalg), NULL, NULL) != 0)
+ goto done; /* error already logged */
+
if ((r = sshkey_load_public(keypath, &pubkey, NULL)) != 0) {
error_r(r, "Couldn't load public key %s", keypath);
goto done;
@@ -2681,7 +2693,7 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)

if (argc == 0) {
if ((r = sign_one(signkey, "(stdin)", STDIN_FILENO,
- sig_namespace, signer, &agent_fd)) != 0)
+ sig_namespace, hashalg, signer, &agent_fd)) != 0)
goto done;
} else {
for (i = 0; i < argc; i++) {
@@ -2693,7 +2705,7 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
goto done;
}
if ((r = sign_one(signkey, argv[i], fd, sig_namespace,
- signer, &agent_fd)) != 0)
+ hashalg, signer, &agent_fd)) != 0)
goto done;
if (fd != STDIN_FILENO)
close(fd);
@@ -2723,7 +2735,7 @@ sig_verify(const char *signature, const char *sig_namespace,
struct sshkey_sig_details *sig_details = NULL;
uint64_t verify_time = 0;

- if (sig_process_opts(opts, nopts, &verify_time, &print_pubkey) != 0)
+ if (sig_process_opts(opts, nopts, NULL, 0, &verify_time, &print_pubkey) != 0)
goto done; /* error already logged */

memset(&sig_details, 0, sizeof(sig_details));
@@ -2811,7 +2823,7 @@ sig_find_principals(const char *signature, const char *allowed_keys,
char *principals = NULL, *cp, *tmp;
uint64_t verify_time = 0;

- if (sig_process_opts(opts, nopts, &verify_time, NULL) != 0)
+ if (sig_process_opts(opts, nopts, NULL, 0, &verify_time, NULL) != 0)
goto done; /* error already logged */

if ((r = sshbuf_load_file(signature, &abuf)) != 0) {
@@ -2857,7 +2869,7 @@ sig_match_principals(const char *allowed_keys, char *principal,
char **principals = NULL;
size_t i, nprincipals = 0;

- if ((r = sig_process_opts(opts, nopts, NULL, NULL)) != 0)
+ if ((r = sig_process_opts(opts, nopts, NULL, 0, NULL, NULL)) != 0)
return r; /* error already logged */

if ((r = sshsig_match_principals(allowed_keys, principal,
@@ -3215,7 +3227,7 @@ usage(void)
" ssh-keygen -Y find-principals -s signature_file -f allowed_signers_file\n"
" ssh-keygen -Y match-principals -I signer_identity -f allowed_signers_file\n"
" ssh-keygen -Y check-novalidate -n namespace -s signature_file\n"
- " ssh-keygen -Y sign -f key_file -n namespace file ...\n"
+ " ssh-keygen -Y sign -f key_file -n namespace file [-O option] ...\n"
" ssh-keygen -Y verify -f allowed_signers_file -I signer_identity\n"
" -n namespace -s signature_file [-r revocation_file]\n");
exit(1);
@@ -3521,7 +3533,7 @@ main(int argc, char **argv)
exit(1);
}
return sig_sign(identity_file, cert_principals,
- argc, argv);
+ argc, argv, opts, nopts);
} else if (strncmp(sign_op, "check-novalidate", 16) == 0) {
if (ca_key_path == NULL) {
error("Too few arguments for check-novalidate: "
--
2.30.2

_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
Re: [PATCH 2/4] add signing option -O hashalg=algorithm [ In reply to ]
On 22.12.2021 09:23, Linus Nordberg wrote:
>---
[...]
>diff --git a/ssh-keygen.c b/ssh-keygen.c
>index d31dc503..ef99b9b6 100644
>--- a/ssh-keygen.c
>+++ b/ssh-keygen.c
>@@ -142,6 +142,9 @@ struct cert_ext {
> static struct cert_ext *cert_ext;
> static size_t ncert_ext;
>
>+/* Default hash algorithm for -Y sign and verify. */
>+#define DEFAULT_SIGN_HASHALG_NAME "sha512"

I'm not a fan of redefining this. sshsig.c has HASHALG_DEFAULT we could
resue when moved to sshsig.h. But I don't think we even need this default
(see below).

[...]
>
> static int
>-sig_process_opts(char * const *opts, size_t nopts, uint64_t *verify_timep,
>+sig_process_opts(char * const *opts, size_t nopts, char *hashalg, size_t hashalg_size, uint64_t *verify_timep,
> int *print_pubkey)
> {
> size_t i;
> time_t now;
>
>+ if (hashalg != NULL)
>+ strlcpy(hashalg, DEFAULT_SIGN_HASHALG_NAME, hashalg_size);
> if (verify_timep != NULL)
> *verify_timep = 0;
> if (print_pubkey != NULL)
> *print_pubkey = 0;
> for (i = 0; i < nopts; i++) {
>- if (verify_timep &&
>+ if (hashalg &&
>+ strncasecmp(opts[i], "hashalg=", 8) == 0) {
>+ strlcpy(hashalg, opts[i] + 8, hashalg_size);

Why copy at all? If we simply return a pointer to the option (or NULL if not
present) we can pass it on as is. This would use the default from sshsig.c
and we would not need to care about one here. The size wouldn't be needed as
well. Basically we could return hashalg = opts[i] + 8, no?

>+ } else if (verify_timep &&
> strncasecmp(opts[i], "verify-time=", 12) == 0) {
> if (parse_absolute_time(opts[i] + 12,
> verify_timep) != 0 || *verify_timep == 0) {
>@@ -2640,12 +2648,13 @@ sig_process_opts(char * const *opts, size_t nopts, uint64_t *verify_timep,
> }
>
> static int
>-sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
>+sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv, char * const *opts, size_t nopts)
> {
> int i, fd = -1, r, ret = -1;
> int agent_fd = -1;
> struct sshkey *pubkey = NULL, *privkey = NULL, *signkey = NULL;
> sshsig_signer *signer = NULL;
>+ char hashalg[7]; /* "shaXXX" */

If we use the pointer into opts directly we are also safe in case sha1024
(or some other hash with a longer name) comes along, without adjusting this
one.

>
> /* Check file arguments. */
> for (i = 0; i < argc; i++) {
>@@ -2655,6 +2664,9 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
> fatal("Cannot sign mix of paths and standard input");
> }
>
>+ if (sig_process_opts(opts, nopts, hashalg, sizeof(hashalg), NULL, NULL) != 0)
>+ goto done; /* error already logged */
>+
> if ((r = sshkey_load_public(keypath, &pubkey, NULL)) != 0) {
> error_r(r, "Couldn't load public key %s", keypath);
> goto done;
>@@ -2681,7 +2693,7 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
>
> if (argc == 0) {
> if ((r = sign_one(signkey, "(stdin)", STDIN_FILENO,
>- sig_namespace, signer, &agent_fd)) != 0)
>+ sig_namespace, hashalg, signer, &agent_fd)) != 0)
> goto done;
> } else {
> for (i = 0; i < argc; i++) {
>@@ -2693,7 +2705,7 @@ sig_sign(const char *keypath, const char *sig_namespace, int argc, char **argv)
> goto done;
> }
> if ((r = sign_one(signkey, argv[i], fd, sig_namespace,
>- signer, &agent_fd)) != 0)
>+ hashalg, signer, &agent_fd)) != 0)
> goto done;
> if (fd != STDIN_FILENO)
> close(fd);
>@@ -2723,7 +2735,7 @@ sig_verify(const char *signature, const char *sig_namespace,
> struct sshkey_sig_details *sig_details = NULL;
> uint64_t verify_time = 0;
>
>- if (sig_process_opts(opts, nopts, &verify_time, &print_pubkey) != 0)
>+ if (sig_process_opts(opts, nopts, NULL, 0, &verify_time, &print_pubkey) != 0)
> goto done; /* error already logged */
>
> memset(&sig_details, 0, sizeof(sig_details));
>@@ -2811,7 +2823,7 @@ sig_find_principals(const char *signature, const char *allowed_keys,
> char *principals = NULL, *cp, *tmp;
> uint64_t verify_time = 0;
>
>- if (sig_process_opts(opts, nopts, &verify_time, NULL) != 0)
>+ if (sig_process_opts(opts, nopts, NULL, 0, &verify_time, NULL) != 0)
> goto done; /* error already logged */
>
> if ((r = sshbuf_load_file(signature, &abuf)) != 0) {
>@@ -2857,7 +2869,7 @@ sig_match_principals(const char *allowed_keys, char *principal,
> char **principals = NULL;
> size_t i, nprincipals = 0;
>
>- if ((r = sig_process_opts(opts, nopts, NULL, NULL)) != 0)
>+ if ((r = sig_process_opts(opts, nopts, NULL, 0, NULL, NULL)) != 0)
> return r; /* error already logged */
>
> if ((r = sshsig_match_principals(allowed_keys, principal,
>@@ -3215,7 +3227,7 @@ usage(void)
> " ssh-keygen -Y find-principals -s signature_file -f allowed_signers_file\n"
> " ssh-keygen -Y match-principals -I signer_identity -f allowed_signers_file\n"
> " ssh-keygen -Y check-novalidate -n namespace -s signature_file\n"
>- " ssh-keygen -Y sign -f key_file -n namespace file ...\n"
>+ " ssh-keygen -Y sign -f key_file -n namespace file [-O option] ...\n"
> " ssh-keygen -Y verify -f allowed_signers_file -I signer_identity\n"
> " -n namespace -s signature_file [-r revocation_file]\n");
> exit(1);
>@@ -3521,7 +3533,7 @@ main(int argc, char **argv)
> exit(1);
> }
> return sig_sign(identity_file, cert_principals,
>- argc, argv);
>+ argc, argv, opts, nopts);
> } else if (strncmp(sign_op, "check-novalidate", 16) == 0) {
> if (ca_key_path == NULL) {
> error("Too few arguments for check-novalidate: "
>--
>2.30.2
>
>_______________________________________________
>openssh-unix-dev mailing list
>openssh-unix-dev@mindrot.org
>https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@mindrot.org
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev