Jitendra Sharma
2019-Jul-08 11:57 UTC
[PATCH] Remove perm_ok argument from sshkey_load_private_type
Remove perm_ok, which could potentially be used to check
for private identification file permissions.
But as sshkey_load_private_type would return
SSH_ERR_KEY_BAD_PERMISSIONS for bad permission.
Hence perm_ok seems redundant.
---
authfile.c | 22 +++++++---------------
authfile.h | 4 ++--
sshconnect2.c | 4 ++--
3 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/authfile.c b/authfile.c
index 2166c168..269209c1 100644
--- a/authfile.c
+++ b/authfile.c
@@ -164,10 +164,9 @@ sshkey_perm_ok(int fd, const char *filename)
return 0;
}
-/* XXX kill perm_ok now that we have SSH_ERR_KEY_BAD_PERMISSIONS? */
int
sshkey_load_private_type(int type, const char *filename, const char
*passphrase,
- struct sshkey **keyp, char **commentp, int *perm_ok)
+ struct sshkey **keyp, char **commentp)
{
int fd, r;
@@ -176,19 +175,12 @@ sshkey_load_private_type(int type, const char *filename,
const char *passphrase,
if (commentp != NULL)
*commentp = NULL;
- if ((fd = open(filename, O_RDONLY)) == -1) {
- if (perm_ok != NULL)
- *perm_ok = 0;
+ if ((fd = open(filename, O_RDONLY)) == -1)
return SSH_ERR_SYSTEM_ERROR;
- }
- if (sshkey_perm_ok(fd, filename) != 0) {
- if (perm_ok != NULL)
- *perm_ok = 0;
- r = SSH_ERR_KEY_BAD_PERMISSIONS;
+
+ r = sshkey_perm_ok(fd, filename);
+ if (r != 0)
goto out;
- }
- if (perm_ok != NULL)
- *perm_ok = 1;
r = sshkey_load_private_type_fd(fd, type, passphrase, keyp, commentp);
if (r == 0 && keyp && *keyp)
@@ -387,7 +379,7 @@ sshkey_load_cert(const char *filename, struct sshkey **keyp)
/* Load private key and certificate */
int
sshkey_load_private_cert(int type, const char *filename, const char
*passphrase,
- struct sshkey **keyp, int *perm_ok)
+ struct sshkey **keyp)
{
struct sshkey *key = NULL, *cert = NULL;
int r;
@@ -410,7 +402,7 @@ sshkey_load_private_cert(int type, const char *filename,
const char *passphrase,
}
if ((r = sshkey_load_private_type(type, filename,
- passphrase, &key, NULL, perm_ok)) != 0 ||
+ passphrase, &key, NULL)) != 0 ||
(r = sshkey_load_cert(filename, &cert)) != 0)
goto out;
diff --git a/authfile.h b/authfile.h
index 624d269f..a6b9759c 100644
--- a/authfile.h
+++ b/authfile.h
@@ -40,9 +40,9 @@ 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 *);
+ struct sshkey **);
int sshkey_load_private_type(int, const char *, const char *,
- struct sshkey **, char **, int *);
+ struct sshkey **, char **);
int sshkey_load_private_type_fd(int fd, int type, const char *passphrase,
struct sshkey **keyp, char **commentp);
int sshkey_perm_ok(int, const char *);
diff --git a/sshconnect2.c b/sshconnect2.c
index cb8d2193..deb0a740 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1404,7 +1404,7 @@ load_identity_file(Identity *id)
{
struct sshkey *private = NULL;
char prompt[300], *passphrase, *comment;
- int r, perm_ok = 0, quit = 0, i;
+ int r, quit = 0, i;
struct stat st;
if (stat(id->filename, &st) == -1) {
@@ -1426,7 +1426,7 @@ load_identity_file(Identity *id)
}
}
switch ((r = sshkey_load_private_type(KEY_UNSPEC, id->filename,
- passphrase, &private, &comment, &perm_ok))) {
+ passphrase, &private, &comment))) {
case 0:
break;
case SSH_ERR_KEY_WRONG_PASSPHRASE:
--
2.20.1
Sharma, Jitendra
2019-Jul-22 05:27 UTC
[PATCH] Remove perm_ok argument from sshkey_load_private_type
Dear Team/Maintainers,
Could this patch be reviewed?
In addition, I have also created a pull request for the same
https://github.com/openssh/openssh-portable/pull/136
Thanks,
Jitendra
-----Original Message-----
From: Sharma, Jitendra
Sent: Monday, July 8, 2019 5:27 PM
To: openssh-unix-dev at mindrot.org
Cc: Sharma, Jitendra <jitendra.sharma at intel.com>
Subject: [PATCH] Remove perm_ok argument from sshkey_load_private_type
Remove perm_ok, which could potentially be used to check for private
identification file permissions.
But as sshkey_load_private_type would return SSH_ERR_KEY_BAD_PERMISSIONS for bad
permission.
Hence perm_ok seems redundant.
---
authfile.c | 22 +++++++---------------
authfile.h | 4 ++--
sshconnect2.c | 4 ++--
3 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/authfile.c b/authfile.c
index 2166c168..269209c1 100644
--- a/authfile.c
+++ b/authfile.c
@@ -164,10 +164,9 @@ sshkey_perm_ok(int fd, const char *filename)
return 0;
}
-/* XXX kill perm_ok now that we have SSH_ERR_KEY_BAD_PERMISSIONS? */ int
sshkey_load_private_type(int type, const char *filename, const char *passphrase,
- struct sshkey **keyp, char **commentp, int *perm_ok)
+ struct sshkey **keyp, char **commentp)
{
int fd, r;
@@ -176,19 +175,12 @@ sshkey_load_private_type(int type, const char *filename,
const char *passphrase,
if (commentp != NULL)
*commentp = NULL;
- if ((fd = open(filename, O_RDONLY)) == -1) {
- if (perm_ok != NULL)
- *perm_ok = 0;
+ if ((fd = open(filename, O_RDONLY)) == -1)
return SSH_ERR_SYSTEM_ERROR;
- }
- if (sshkey_perm_ok(fd, filename) != 0) {
- if (perm_ok != NULL)
- *perm_ok = 0;
- r = SSH_ERR_KEY_BAD_PERMISSIONS;
+
+ r = sshkey_perm_ok(fd, filename);
+ if (r != 0)
goto out;
- }
- if (perm_ok != NULL)
- *perm_ok = 1;
r = sshkey_load_private_type_fd(fd, type, passphrase, keyp, commentp);
if (r == 0 && keyp && *keyp)
@@ -387,7 +379,7 @@ sshkey_load_cert(const char *filename, struct sshkey **keyp)
/* Load private key and certificate */
int
sshkey_load_private_cert(int type, const char *filename, const char
*passphrase,
- struct sshkey **keyp, int *perm_ok)
+ struct sshkey **keyp)
{
struct sshkey *key = NULL, *cert = NULL;
int r;
@@ -410,7 +402,7 @@ sshkey_load_private_cert(int type, const char *filename,
const char *passphrase,
}
if ((r = sshkey_load_private_type(type, filename,
- passphrase, &key, NULL, perm_ok)) != 0 ||
+ passphrase, &key, NULL)) != 0 ||
(r = sshkey_load_cert(filename, &cert)) != 0)
goto out;
diff --git a/authfile.h b/authfile.h
index 624d269f..a6b9759c 100644
--- a/authfile.h
+++ b/authfile.h
@@ -40,9 +40,9 @@ 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 *);
+ struct sshkey **);
int sshkey_load_private_type(int, const char *, const char *,
- struct sshkey **, char **, int *);
+ struct sshkey **, char **);
int sshkey_load_private_type_fd(int fd, int type, const char *passphrase,
struct sshkey **keyp, char **commentp); int sshkey_perm_ok(int, const char
*); diff --git a/sshconnect2.c b/sshconnect2.c index cb8d2193..deb0a740 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1404,7 +1404,7 @@ load_identity_file(Identity *id) {
struct sshkey *private = NULL;
char prompt[300], *passphrase, *comment;
- int r, perm_ok = 0, quit = 0, i;
+ int r, quit = 0, i;
struct stat st;
if (stat(id->filename, &st) == -1) {
@@ -1426,7 +1426,7 @@ load_identity_file(Identity *id)
}
}
switch ((r = sshkey_load_private_type(KEY_UNSPEC, id->filename,
- passphrase, &private, &comment, &perm_ok))) {
+ passphrase, &private, &comment))) {
case 0:
break;
case SSH_ERR_KEY_WRONG_PASSPHRASE:
--
2.20.1
Darren Tucker
2019-Aug-05 11:54 UTC
[PATCH] Remove perm_ok argument from sshkey_load_private_type
On Mon, 8 Jul 2019 at 22:00, Jitendra Sharma <jitendra.sharma at intel.com> wrote:> Remove perm_ok, which could potentially be used to checkApplied to OpenBSD, thanks. -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.