Mailing List Archive

[PATCH 3/4] ssh: update certificate support
remove useful but not feature related log messages
---
agent/command-ssh.c | 30 +++++++++---------------------
agent/findkey.c | 6 ++----
2 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/agent/command-ssh.c b/agent/command-ssh.c
index dfdc36f97..3983bbeb4 100644
--- a/agent/command-ssh.c
+++ b/agent/command-ssh.c
@@ -1963,11 +1963,11 @@ ssh_key_to_blob (gcry_sexp_t sexp, int with_secret,
}
else
{
- /* Note: This is also used for EdDSA. */
- err = stream_write_cstring (stream, key_spec.ssh_identifier);
- if (err)
- goto out;
- }
+ /* Note: This is also used for EdDSA. */
+ err = stream_write_cstring (stream, key_spec.ssh_identifier);
+ if (err)
+ goto out;
+ }
}

/* Write the parameters. */
@@ -2016,7 +2016,7 @@ ssh_key_to_blob (gcry_sexp_t sexp, int with_secret,
goto out;
}
}
-
+
done:
if (es_fclose_snatch (stream, &blob, &blob_size))
{
@@ -2096,16 +2096,10 @@ ssh_receive_key (estream_t stream, gcry_sexp_t *key_new, int secret,
if (err)
goto out;

- if (opt.verbose)
- log_info("key type: %s", key_type);
-
err = ssh_key_type_lookup (key_type, 0, &spec);
if (err)
goto out;

- if (opt.verbose)
- log_info("key spec flags: 0x%x", spec.flags);
-
unsigned char *cert_buffer = NULL;
u32 cert_buffer_len = 0;

@@ -2129,10 +2123,6 @@ ssh_receive_key (estream_t stream, gcry_sexp_t *key_new, int secret,
err = stream_read_cstring (cert, &cert_key_type);
if (err)
goto out;
-
- if (opt.verbose)
- log_info ("certificate type: %s", cert_key_type);
-
if (strcmp (cert_key_type, key_type) )
{
xfree (cert_key_type);
@@ -2252,8 +2242,6 @@ ssh_receive_key (estream_t stream, gcry_sexp_t *key_new, int secret,
err = stream_read_cstring (stream, &comment);
if (err)
goto out;
- if (opt.verbose)
- log_info("key comment: %s", comment);
}

if (secret)
@@ -2335,9 +2323,9 @@ ssh_receive_key (estream_t stream, gcry_sexp_t *key_new, int secret,
goto out;
}
else
- {
+ {
err = sexp_key_construct (&key, spec, secret, curve_name, mpi_list,
- comment? comment:"");
+ comment? comment:"");
if (err)
goto out;
}
@@ -3244,7 +3232,7 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec,
goto next_try;
}
}
-
+
err = ssh_key_to_protected_buffer (key, pi->pin, &buffer, &buffer_n);
if (err)
goto out;
diff --git a/agent/findkey.c b/agent/findkey.c
index b558ab893..63964ce69 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -1263,7 +1263,7 @@ agent_public_key_from_file (ctrl_t ctrl,
err = read_key_file (grip, &s_skey);
if (err)
return err;
-
+
for (i=0; i < DIM (array); i++)
array[i] = NULL;

@@ -1304,8 +1304,6 @@ agent_public_key_from_file (ctrl_t ctrl,
s_skey = NULL;


- // TODO: the following FIXME is so true -- following code is
- // prone to buffer overrun
/* FIXME: The following thing is pretty ugly code; we should
investigate how to make it cleaner. Probably code to handle
canonical S-expressions in a memory buffer is better suited for
@@ -1314,7 +1312,7 @@ agent_public_key_from_file (ctrl_t ctrl,
them. */
assert (sizeof (size_t) <= sizeof (void*));

- format = xtrymalloc (15+4+7*npkey+10+15+1+1+5+4096);
+ format = xtrymalloc (15+4+7*npkey+10+15+1+1+5+10);
if (!format)
{
err = gpg_error_from_syserror ();
--
2.25.1


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH 3/4] ssh: update certificate support [ In reply to ]
Igor Okulist via Gnupg-devel wrote:
> [...]
> @@ -1304,8 +1304,6 @@ agent_public_key_from_file (ctrl_t ctrl,
> s_skey = NULL;
>
>
> - // TODO: the following FIXME is so true -- following code is
> - // prone to buffer overrun
> /* FIXME: The following thing is pretty ugly code; we should
> investigate how to make it cleaner. Probably code to handle
> canonical S-expressions in a memory buffer is better suited for
> @@ -1314,7 +1312,7 @@ agent_public_key_from_file (ctrl_t ctrl,
> them. */
> assert (sizeof (size_t) <= sizeof (void*));
>
> - format = xtrymalloc (15+4+7*npkey+10+15+1+1+5+4096);
> + format = xtrymalloc (15+4+7*npkey+10+15+1+1+5+10);
> if (!format)
> {
> err = gpg_error_from_syserror ();
>

Are you sure about this? Removing a comment that warns of possible
buffer overruns that need to be addressed without (as far as I can tell)
actually addressing the possible issue while also *reducing* the size of
an allocated buffer strikes me as odd.


-- Jacob


_______________________________________________
Gnupg-devel mailing list
Gnupg-devel@gnupg.org
http://lists.gnupg.org/mailman/listinfo/gnupg-devel
Re: [PATCH 3/4] ssh: update certificate support [ In reply to ]
On Wed, 17 Mar 2021 17:39, Jacob Bachmeyer said:

> Are you sure about this? Removing a comment that warns of possible
> buffer overruns that need to be addressed without (as far as I can

Anyway, I took the opportunity to rewrite that part to use a membuf.
Not a generic solution to create such s-expression but better than
fragile in-advance byte counting.


Shalom-Salam,

Werner

--
Die Gedanken sind frei. Ausnahmen regelt ein Bundesgesetz.