openssh bugs - Sep 2019 - [Bug 3068] New: Duplicate code in sshkey_load_private() function

https://bugzilla.mindrot.org/show_bug.cgi?id=3068

            Bug ID: 3068
           Summary: Duplicate code in sshkey_load_private() function
           Product: Portable OpenSSH
           Version: 8.0p1
          Hardware: Other
                OS: Windows 10
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: ssh-keygen
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jitendra.sharma at intel.com

sshkey_load_private() function's role is almost similar to
sshkey_load_private_type().

By checking at the source code for sshkey_load_private(), we could see
that there is lot of code duplicacy there (as similar code is present
in sshkey_load_private_type()).

Could we explore if we could remove sshkey_load_private(), and keep
sshkey_load_private_type(). This will help in avoiding code duplicacy.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3068

--- Comment #1 from Jitendra Sharma <jitendra.sharma at intel.com> ---
In the openssh mailing list already that discussion started,
https://lists.gt.net/openssh/dev/74450

Where it was suggested that it would be preferred to keep
sshkey_load_private() as a wrapper.

If this is the case could we explore if we could make
sshkey_load_private as a macro, so that its client users would not get
impacted and they will have this wrapper functionality available to
them.

If this idea seems ok, then could this approach be implemented like
below:

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)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3068

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org
             Blocks|                            |3162
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
sshkey_load_private() was removed back in April


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3162
[Bug 3162] Tracking bug for 8.4 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3068

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
Mass close of all bugs fixed in 8.4 release.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.