Remove sshkey_load_private(), as this function's role
is similar to sshkey_load_private_type().
---
Dependency:
This change depends over recently merged change in openbsd:
https://github.com/openbsd/src/commit/b0c328c8f066f6689874bef7f338179145ce58d0
Change log:
v1->v2
- Remove declaration of sshkey_load_private() in authfile.h
authfile.c | 38 --------------------------------------
authfile.h | 1 -
ssh-keygen.c | 20 +++++++++++---------
sshd.c | 5 +++--
4 files changed, 14 insertions(+), 50 deletions(-)
diff --git a/authfile.c b/authfile.c
index c28652c8bdf..6d86c2dd4c6 100644
--- a/authfile.c
+++ b/authfile.c
@@ -215,44 +215,6 @@ sshkey_load_private_type_fd(int fd, int type, const char
*passphrase,
return r;
}
-/* XXX this is almost identical to sshkey_load_private_type() */
-int
-sshkey_load_private(const char *filename, const char *passphrase,
- struct sshkey **keyp, char **commentp)
-{
- struct sshbuf *buffer = NULL;
- int r, fd;
-
- if (keyp != NULL)
- *keyp = NULL;
- if (commentp != NULL)
- *commentp = NULL;
-
- if ((fd = open(filename, O_RDONLY)) == -1)
- return SSH_ERR_SYSTEM_ERROR;
- if (sshkey_perm_ok(fd, filename) != 0) {
- r = SSH_ERR_KEY_BAD_PERMISSIONS;
- goto out;
- }
-
- if ((buffer = sshbuf_new()) == NULL) {
- r = SSH_ERR_ALLOC_FAIL;
- goto out;
- }
- if ((r = sshkey_load_file(fd, buffer)) != 0 ||
- (r = sshkey_parse_private_fileblob(buffer, passphrase, keyp,
- commentp)) != 0)
- goto out;
- if (keyp && *keyp &&
- (r = sshkey_set_filename(*keyp, filename)) != 0)
- goto out;
- r = 0;
- out:
- close(fd);
- sshbuf_free(buffer);
- return r;
-}
-
static int
sshkey_try_load_public(struct sshkey *k, const char *filename, char **commentp)
{
diff --git a/authfile.h b/authfile.h
index a6b9759c5ea..0279a89e2b4 100644
--- a/authfile.h
+++ b/authfile.h
@@ -38,7 +38,6 @@ int sshkey_save_private(struct sshkey *, const char *,
int sshkey_load_file(int, struct sshbuf *);
int sshkey_load_cert(const char *, struct sshkey **);
int sshkey_load_public(const char *, struct sshkey **, char **);
-int sshkey_load_private(const char *, const char *, struct sshkey **, char **);
int sshkey_load_private_cert(int, const char *, const char *,
struct sshkey **);
int sshkey_load_private_type(int, const char *, const char *,
diff --git a/ssh-keygen.c b/ssh-keygen.c
index ea3c0e63888..215693eaca6 100644
--- a/ssh-keygen.c
+++ b/ssh-keygen.c
@@ -275,7 +275,8 @@ load_identity(char *filename)
struct sshkey *prv;
int r;
- if ((r = sshkey_load_private(filename, "", &prv, NULL)) == 0)
+ if ((r = sshkey_load_private_type(KEY_UNSPEC, filename, "",
+ &prv, NULL)) == 0)
return prv;
if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
fatal("Load key \"%s\": %s", filename, ssh_err(r));
@@ -283,7 +284,7 @@ load_identity(char *filename)
pass = xstrdup(identity_passphrase);
else
pass = read_passphrase("Enter passphrase: ", RP_ALLOW_STDIN);
- r = sshkey_load_private(filename, pass, &prv, NULL);
+ r = sshkey_load_private_type(KEY_UNSPEC, filename, pass, &prv, NULL);
explicit_bzero(pass, strlen(pass));
free(pass);
if (r != 0)
@@ -855,7 +856,7 @@ fingerprint_private(const char *path)
fatal("%s: %s", path, strerror(errno));
if ((r = sshkey_load_public(path, &public, &comment)) != 0) {
debug("load public \"%s\": %s", path, ssh_err(r));
- if ((r = sshkey_load_private(path, NULL,
+ if ((r = sshkey_load_private_type(KEY_UNSPEC, path, NULL,
&public, &comment)) != 0) {
debug("load private \"%s\": %s", path, ssh_err(r));
fatal("%s is not a key file.", path);
@@ -1349,7 +1350,8 @@ do_change_passphrase(struct passwd *pw)
if (stat(identity_file, &st) == -1)
fatal("%s: %s", identity_file, strerror(errno));
/* Try to load the file with empty passphrase. */
- r = sshkey_load_private(identity_file, "", &private,
&comment);
+ r = sshkey_load_private_type(KEY_UNSPEC, identity_file, "",
+ &private, &comment);
if (r == SSH_ERR_KEY_WRONG_PASSPHRASE) {
if (identity_passphrase)
old_passphrase = xstrdup(identity_passphrase);
@@ -1357,8 +1359,8 @@ do_change_passphrase(struct passwd *pw)
old_passphrase read_passphrase("Enter old passphrase: ",
RP_ALLOW_STDIN);
- r = sshkey_load_private(identity_file, old_passphrase,
- &private, &comment);
+ r = sshkey_load_private_type(KEY_UNSPEC, identity_file,
+ old_passphrase, &private, &comment);
explicit_bzero(old_passphrase, strlen(old_passphrase));
free(old_passphrase);
if (r != 0)
@@ -1461,7 +1463,7 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
ask_filename(pw, "Enter file in which the key is");
if (stat(identity_file, &st) == -1)
fatal("%s: %s", identity_file, strerror(errno));
- if ((r = sshkey_load_private(identity_file, "",
+ if ((r = sshkey_load_private_type(KEY_UNSPEC, identity_file, "",
&private, &comment)) == 0)
passphrase = xstrdup("");
else if (r != SSH_ERR_KEY_WRONG_PASSPHRASE)
@@ -1476,8 +1478,8 @@ do_change_comment(struct passwd *pw, const char
*identity_comment)
passphrase = read_passphrase("Enter passphrase: ",
RP_ALLOW_STDIN);
/* Try to load using the passphrase. */
- if ((r = sshkey_load_private(identity_file, passphrase,
- &private, &comment)) != 0) {
+ if ((r = sshkey_load_private_type(KEY_UNSPEC, identity_file,
+ passphrase, &private, &comment)) != 0) {
explicit_bzero(passphrase, strlen(passphrase));
free(passphrase);
fatal("Cannot load private key \"%s\": %s.",
diff --git a/sshd.c b/sshd.c
index 11571c01011..cea85de2404 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1719,8 +1719,9 @@ main(int ac, char **av)
if (options.host_key_files[i] == NULL)
continue;
- if ((r = sshkey_load_private(options.host_key_files[i], "",
- &key, NULL)) != 0 && r != SSH_ERR_SYSTEM_ERROR)
+ if ((r = sshkey_load_private_type(KEY_UNSPEC,
+ options.host_key_files[i], "", &key, NULL)) != 0 &&
+ r != SSH_ERR_SYSTEM_ERROR)
do_log2(ll, "Unable to load host key \"%s\": %s",
options.host_key_files[i], ssh_err(r));
if (r == 0 && (r = sshkey_shield_private(key)) != 0) {
--
2.22.0
I think I'd prefer that sshkey_load_private() remains as a wrapper. On Tue, 6 Aug 2019, Jitendra Sharma wrote:> Remove sshkey_load_private(), as this function's role > is similar to sshkey_load_private_type(). > --- > Dependency: > This change depends over recently merged change in openbsd: > https://github.com/openbsd/src/commit/b0c328c8f066f6689874bef7f338179145ce58d0 > > Change log: > v1->v2 > - Remove declaration of sshkey_load_private() in authfile.h > > authfile.c | 38 -------------------------------------- > authfile.h | 1 - > ssh-keygen.c | 20 +++++++++++--------- > sshd.c | 5 +++-- > 4 files changed, 14 insertions(+), 50 deletions(-) > > diff --git a/authfile.c b/authfile.c > index c28652c8bdf..6d86c2dd4c6 100644 > --- a/authfile.c > +++ b/authfile.c > @@ -215,44 +215,6 @@ sshkey_load_private_type_fd(int fd, int type, const char *passphrase, > return r; > } > > -/* XXX this is almost identical to sshkey_load_private_type() */ > -int > -sshkey_load_private(const char *filename, const char *passphrase, > - struct sshkey **keyp, char **commentp) > -{ > - struct sshbuf *buffer = NULL; > - int r, fd; > - > - if (keyp != NULL) > - *keyp = NULL; > - if (commentp != NULL) > - *commentp = NULL; > - > - if ((fd = open(filename, O_RDONLY)) == -1) > - return SSH_ERR_SYSTEM_ERROR; > - if (sshkey_perm_ok(fd, filename) != 0) { > - r = SSH_ERR_KEY_BAD_PERMISSIONS; > - goto out; > - } > - > - if ((buffer = sshbuf_new()) == NULL) { > - r = SSH_ERR_ALLOC_FAIL; > - goto out; > - } > - if ((r = sshkey_load_file(fd, buffer)) != 0 || > - (r = sshkey_parse_private_fileblob(buffer, passphrase, keyp, > - commentp)) != 0) > - goto out; > - if (keyp && *keyp && > - (r = sshkey_set_filename(*keyp, filename)) != 0) > - goto out; > - r = 0; > - out: > - close(fd); > - sshbuf_free(buffer); > - return r; > -} > - > static int > sshkey_try_load_public(struct sshkey *k, const char *filename, char **commentp) > { > diff --git a/authfile.h b/authfile.h > index a6b9759c5ea..0279a89e2b4 100644 > --- a/authfile.h > +++ b/authfile.h > @@ -38,7 +38,6 @@ int sshkey_save_private(struct sshkey *, const char *, > int sshkey_load_file(int, struct sshbuf *); > int sshkey_load_cert(const char *, struct sshkey **); > int sshkey_load_public(const char *, struct sshkey **, char **); > -int sshkey_load_private(const char *, const char *, struct sshkey **, char **); > int sshkey_load_private_cert(int, const char *, const char *, > struct sshkey **); > int sshkey_load_private_type(int, const char *, const char *, > diff --git a/ssh-keygen.c b/ssh-keygen.c > index ea3c0e63888..215693eaca6 100644 > --- a/ssh-keygen.c > +++ b/ssh-keygen.c > @@ -275,7 +275,8 @@ load_identity(char *filename) > struct sshkey *prv; > int r; > > - if ((r = sshkey_load_private(filename, "", &prv, NULL)) == 0) > + if ((r = sshkey_load_private_type(KEY_UNSPEC, filename, "", > + &prv, NULL)) == 0) > return prv; > if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) > fatal("Load key \"%s\": %s", filename, ssh_err(r)); > @@ -283,7 +284,7 @@ load_identity(char *filename) > pass = xstrdup(identity_passphrase); > else > pass = read_passphrase("Enter passphrase: ", RP_ALLOW_STDIN); > - r = sshkey_load_private(filename, pass, &prv, NULL); > + r = sshkey_load_private_type(KEY_UNSPEC, filename, pass, &prv, NULL); > explicit_bzero(pass, strlen(pass)); > free(pass); > if (r != 0) > @@ -855,7 +856,7 @@ fingerprint_private(const char *path) > fatal("%s: %s", path, strerror(errno)); > if ((r = sshkey_load_public(path, &public, &comment)) != 0) { > debug("load public \"%s\": %s", path, ssh_err(r)); > - if ((r = sshkey_load_private(path, NULL, > + if ((r = sshkey_load_private_type(KEY_UNSPEC, path, NULL, > &public, &comment)) != 0) { > debug("load private \"%s\": %s", path, ssh_err(r)); > fatal("%s is not a key file.", path); > @@ -1349,7 +1350,8 @@ do_change_passphrase(struct passwd *pw) > if (stat(identity_file, &st) == -1) > fatal("%s: %s", identity_file, strerror(errno)); > /* Try to load the file with empty passphrase. */ > - r = sshkey_load_private(identity_file, "", &private, &comment); > + r = sshkey_load_private_type(KEY_UNSPEC, identity_file, "", > + &private, &comment); > if (r == SSH_ERR_KEY_WRONG_PASSPHRASE) { > if (identity_passphrase) > old_passphrase = xstrdup(identity_passphrase); > @@ -1357,8 +1359,8 @@ do_change_passphrase(struct passwd *pw) > old_passphrase > read_passphrase("Enter old passphrase: ", > RP_ALLOW_STDIN); > - r = sshkey_load_private(identity_file, old_passphrase, > - &private, &comment); > + r = sshkey_load_private_type(KEY_UNSPEC, identity_file, > + old_passphrase, &private, &comment); > explicit_bzero(old_passphrase, strlen(old_passphrase)); > free(old_passphrase); > if (r != 0) > @@ -1461,7 +1463,7 @@ do_change_comment(struct passwd *pw, const char *identity_comment) > ask_filename(pw, "Enter file in which the key is"); > if (stat(identity_file, &st) == -1) > fatal("%s: %s", identity_file, strerror(errno)); > - if ((r = sshkey_load_private(identity_file, "", > + if ((r = sshkey_load_private_type(KEY_UNSPEC, identity_file, "", > &private, &comment)) == 0) > passphrase = xstrdup(""); > else if (r != SSH_ERR_KEY_WRONG_PASSPHRASE) > @@ -1476,8 +1478,8 @@ do_change_comment(struct passwd *pw, const char *identity_comment) > passphrase = read_passphrase("Enter passphrase: ", > RP_ALLOW_STDIN); > /* Try to load using the passphrase. */ > - if ((r = sshkey_load_private(identity_file, passphrase, > - &private, &comment)) != 0) { > + if ((r = sshkey_load_private_type(KEY_UNSPEC, identity_file, > + passphrase, &private, &comment)) != 0) { > explicit_bzero(passphrase, strlen(passphrase)); > free(passphrase); > fatal("Cannot load private key \"%s\": %s.", > diff --git a/sshd.c b/sshd.c > index 11571c01011..cea85de2404 100644 > --- a/sshd.c > +++ b/sshd.c > @@ -1719,8 +1719,9 @@ main(int ac, char **av) > > if (options.host_key_files[i] == NULL) > continue; > - if ((r = sshkey_load_private(options.host_key_files[i], "", > - &key, NULL)) != 0 && r != SSH_ERR_SYSTEM_ERROR) > + if ((r = sshkey_load_private_type(KEY_UNSPEC, > + options.host_key_files[i], "", &key, NULL)) != 0 && > + r != SSH_ERR_SYSTEM_ERROR) > do_log2(ll, "Unable to load host key \"%s\": %s", > options.host_key_files[i], ssh_err(r)); > if (r == 0 && (r = sshkey_shield_private(key)) != 0) { > -- > 2.22.0 > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev >
Thanks Damien for replying.> -----Original Message----- > From: Damien Miller [mailto:djm at mindrot.org] > Sent: Thursday, August 8, 2019 4:57 AM > To: Sharma, Jitendra <jitendra.sharma at intel.com> > Cc: openssh-unix-dev at mindrot.org > Subject: Re: [PATCH v2] Remove sshkey_load_private() > > I think I'd prefer that sshkey_load_private() remains as a wrapper. >I assume there must be a strong reason for keeping sshkey_load_private wrapper, which I am not aware of ? However if the intent is to provide a facility/wrapper to consumers of this function, then to avoid duplicate code, we could provide a macro interface to end users. Could you please review this code, if it sounds good: jitendra at clear-linux~/openssh-portable $ git diff diff --git a/authfile.c b/authfile.c index 5e335ce4..89b5ca39 100644 --- a/authfile.c +++ b/authfile.c @@ -215,44 +215,6 @@ sshkey_load_private_type_fd(int fd, int type, const char *passphrase, return r; } -/* XXX this is almost identical to sshkey_load_private_type() */ -int -sshkey_load_private(const char *filename, const char *passphrase, - struct sshkey **keyp, char **commentp) -{ - struct sshbuf *buffer = NULL; - int r, fd; - - if (keyp != NULL) - *keyp = NULL; - if (commentp != NULL) - *commentp = NULL; - - if ((fd = open(filename, O_RDONLY)) == -1) - return SSH_ERR_SYSTEM_ERROR; - if (sshkey_perm_ok(fd, filename) != 0) { - r = SSH_ERR_KEY_BAD_PERMISSIONS; - goto out; - } - - if ((buffer = sshbuf_new()) == NULL) { - r = SSH_ERR_ALLOC_FAIL; - goto out; - } - if ((r = sshkey_load_file(fd, buffer)) != 0 || - (r = sshkey_parse_private_fileblob(buffer, passphrase, keyp, - commentp)) != 0) - goto out; - if (keyp && *keyp && - (r = sshkey_set_filename(*keyp, filename)) != 0) - goto out; - r = 0; - out: - close(fd); - sshbuf_free(buffer); - return r; -} - static int sshkey_try_load_public(struct sshkey *k, const char *filename, char **commentp) { diff --git a/authfile.h b/authfile.h index 54df169b..2962fbba 100644 --- a/authfile.h +++ b/authfile.h @@ -30,6 +30,9 @@ struct sshbuf; struct sshkey; +#define sshkey_load_private(filename, passphrase, keyp, commentp) \ + sshkey_load_private_type(KEY_UNSPEC, filename, passphrase, keyp, commentp) + /* XXX document these */ /* XXX some of these could probably be merged/retired */ @@ -38,7 +41,6 @@ int sshkey_save_private(struct sshkey *, const char *, int sshkey_load_file(int, struct sshbuf *); int sshkey_load_cert(const char *, struct sshkey **); int sshkey_load_public(const char *, struct sshkey **, char **); -int sshkey_load_private(const char *, const char *, struct sshkey **, char **); int sshkey_load_private_cert(int, const char *, const char *, struct sshkey **); int sshkey_load_private_type(int, const char *, const char *, (END)> On Tue, 6 Aug 2019, Jitendra Sharma wrote: > > > Remove sshkey_load_private(), as this function's role is similar to > > sshkey_load_private_type(). > > ---