Hi
I noticed that sshkey_unshield_private contains a exact duplicate
of the code in private2_check_padding. So by pulling
private2_check_padding up, the code can be reused. Or is there
a reason for this split?
Best,
Martin
P.S.: This diff also removes two trailing spaces while here.
Index: sshkey.c
===================================================================
RCS file: /home/reposync/cvs/src/usr.bin/ssh/sshkey.c,v
retrieving revision 1.120
diff -u -p -r1.120 sshkey.c
--- sshkey.c 6 Jan 2022 22:05:42 -0000 1.120
+++ sshkey.c 4 May 2022 19:12:16 -0000
@@ -2079,14 +2079,38 @@ sshkey_shield_private(struct sshkey *k)
return r;
}
+/* Check deterministic padding after private key */
+static int
+private2_check_padding(struct sshbuf *decrypted)
+{
+ u_char pad;
+ size_t i;
+ int r;
+
+ i = 0;
+ while (sshbuf_len(decrypted)) {
+ if ((r = sshbuf_get_u8(decrypted, &pad)) != 0)
+ goto out;
+ if (pad != (++i & 0xff)) {
+ r = SSH_ERR_INVALID_FORMAT;
+ goto out;
+ }
+ }
+ /* success */
+ r = 0;
+ out:
+ explicit_bzero(&pad, sizeof(pad));
+ explicit_bzero(&i, sizeof(i));
+ return r;
+}
+
int
sshkey_unshield_private(struct sshkey *k)
{
struct sshbuf *prvbuf = NULL;
- u_char pad, *cp, keyiv[SSH_DIGEST_MAX_LENGTH];
+ u_char *cp, keyiv[SSH_DIGEST_MAX_LENGTH];
struct sshcipher_ctx *cctx = NULL;
const struct sshcipher *cipher;
- size_t i;
struct sshkey *kswap = NULL, tmp;
int r = SSH_ERR_INTERNAL_ERROR;
@@ -2148,16 +2172,9 @@ sshkey_unshield_private(struct sshkey *k
/* Parse private key */
if ((r = sshkey_private_deserialize(prvbuf, &kswap)) != 0)
goto out;
- /* Check deterministic padding */
- i = 0;
- while (sshbuf_len(prvbuf)) {
- if ((r = sshbuf_get_u8(prvbuf, &pad)) != 0)
- goto out;
- if (pad != (++i & 0xff)) {
- r = SSH_ERR_INVALID_FORMAT;
- goto out;
- }
- }
+
+ if ((r = private2_check_padding(prvbuf)) != 0)
+ goto out;
/* Swap the parsed key back into place */
tmp = *kswap;
@@ -3966,9 +3983,9 @@ sshkey_private_to_blob2(struct sshkey *p
explicit_bzero(salt, sizeof(salt));
if (key != NULL)
freezero(key, keylen + ivlen);
- if (pubkeyblob != NULL)
+ if (pubkeyblob != NULL)
freezero(pubkeyblob, pubkeylen);
- if (b64 != NULL)
+ if (b64 != NULL)
freezero(b64, strlen(b64));
return r;
}
@@ -4192,31 +4209,6 @@ private2_decrypt(struct sshbuf *decoded,
}
sshbuf_free(kdf);
sshbuf_free(decrypted);
- return r;
-}
-
-/* Check deterministic padding after private key */
-static int
-private2_check_padding(struct sshbuf *decrypted)
-{
- u_char pad;
- size_t i;
- int r = SSH_ERR_INTERNAL_ERROR;
-
- i = 0;
- while (sshbuf_len(decrypted)) {
- if ((r = sshbuf_get_u8(decrypted, &pad)) != 0)
- goto out;
- if (pad != (++i & 0xff)) {
- r = SSH_ERR_INVALID_FORMAT;
- goto out;
- }
- }
- /* success */
- r = 0;
- out:
- explicit_bzero(&pad, sizeof(pad));
- explicit_bzero(&i, sizeof(i));
return r;
}
