Skip to content

Commit

Permalink
Disallow remote addition of FIDO/PKCS11 provider libraries to
Browse files Browse the repository at this point in the history
ssh-agent by default.

The old behaviour of allowing remote clients from loading providers
can be restored using `ssh-agent -O allow-remote-pkcs11`.

Detection of local/remote clients requires a ssh(1) that supports
the `session-bind@openssh.com` extension. Forwarding access to a
ssh-agent socket using non-OpenSSH tools may circumvent this control.

ok markus@
  • Loading branch information
djmdjm committed Jul 19, 2023
1 parent f03a4fa commit 7bc29a9
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
26 changes: 22 additions & 4 deletions usr.bin/ssh/ssh-agent.1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.\" $OpenBSD: ssh-agent.1,v 1.75 2022/10/07 06:00:58 jmc Exp $
.\" $OpenBSD: ssh-agent.1,v 1.76 2023/07/19 13:56:33 djm Exp $
.\"
.\" Author: Tatu Ylonen <ylo@cs.hut.fi>
.\" Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
Expand Down Expand Up @@ -34,7 +34,7 @@
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.\"
.Dd $Mdocdate: October 7 2022 $
.Dd $Mdocdate: July 19 2023 $
.Dt SSH-AGENT 1
.Os
.Sh NAME
Expand Down Expand Up @@ -107,9 +107,27 @@ environment variable).
.It Fl O Ar option
Specify an option when starting
.Nm .
Currently only one option is supported:
Currently two options are supported:
.Cm allow-remote-pkcs11
and
.Cm no-restrict-websafe .
This instructs
.Pp
The
.Cm allow-remote-pkcs11
option allows clients of a forwarded
.Nm
to load PKCS#11 or FIDO provider libraries.
By default only local clients may perform this operation.
Note that signalling that a
.Nm
client remote is performed by
.Xr ssh 1 ,
and use of other tools to forward access to the agent socket may circumvent
this restriction.
.Pp
The
.Cm no-restrict-websafe ,
instructs
.Nm
to permit signatures using FIDO keys that might be web authentication
requests.
Expand Down
23 changes: 21 additions & 2 deletions usr.bin/ssh/ssh-agent.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: ssh-agent.c,v 1.299 2023/07/10 04:51:26 djm Exp $ */
/* $OpenBSD: ssh-agent.c,v 1.300 2023/07/19 13:56:33 djm Exp $ */
/*
* Author: Tatu Ylonen <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
Expand Down Expand Up @@ -156,6 +156,12 @@ char socket_dir[PATH_MAX];
/* Pattern-list of allowed PKCS#11/Security key paths */
static char *allowed_providers;

/*
* Allows PKCS11 providers or SK keys that use non-internal providers to
* be added over a remote connection (identified by session-bind@openssh.com).
*/
static int remote_add_provider;

/* locking */
#define LOCK_SIZE 32
#define LOCK_SALT_SIZE 16
Expand Down Expand Up @@ -1215,6 +1221,12 @@ process_add_identity(SocketEntry *e)
if (strcasecmp(sk_provider, "internal") == 0) {
debug_f("internal provider");
} else {
if (e->nsession_ids != 0 && !remote_add_provider) {
verbose("failed add of SK provider \"%.100s\": "
"remote addition of providers is disabled",
sk_provider);
goto out;
}
if (realpath(sk_provider, canonical_provider) == NULL) {
verbose("failed provider \"%.100s\": "
"realpath: %s", sk_provider,
Expand Down Expand Up @@ -1378,6 +1390,11 @@ process_add_smartcard_key(SocketEntry *e)
error_f("failed to parse constraints");
goto send;
}
if (e->nsession_ids != 0 && !remote_add_provider) {
verbose("failed PKCS#11 add of \"%.100s\": remote addition of "
"providers is disabled", provider);
goto send;
}
if (realpath(provider, canonical_provider) == NULL) {
verbose("failed PKCS#11 add of \"%.100s\": realpath: %s",
provider, strerror(errno));
Expand Down Expand Up @@ -2032,7 +2049,9 @@ main(int ac, char **av)
break;
case 'O':
if (strcmp(optarg, "no-restrict-websafe") == 0)
restrict_websafe = 0;
restrict_websafe = 0;
else if (strcmp(optarg, "allow-remote-pkcs11") == 0)
remote_add_provider = 1;
else
fatal("Unknown -O option");
break;
Expand Down

3 comments on commit 7bc29a9

@alexleel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a question, seems the code above " if (e->nsession_ids != 0 && !remote_add_provider) {...". can avoid entering incorrect logic , however, what if the version of openssh does not have this variable 'nsession_ids '?(openssh7.4). Can I say that our version is not affected?

@2473067939
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I know how to use it ?

@Vincent-Lee9870522
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse me,Someone can tell me how to download this file and how to use it? I'm not a server user.

Please sign in to comment.