Skip to content

Commit

Permalink
upstream: add a RequiredRSASize for checking RSA key length in
Browse files Browse the repository at this point in the history
ssh(1). User authentication keys that fall beneath this limit will be
ignored. If a host presents a host key beneath this limit then the connection
will be terminated (unfortunately there are no fallbacks in the protocol for
host authentication).

feedback deraadt, Dmitry Belyavskiy; ok markus@

OpenBSD-Commit-ID: 430e339b2a79fa9ecc63f2837b06fdd88a7da13a
  • Loading branch information
djmdjm committed Sep 17, 2022
1 parent 07d8771 commit 54b333d
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 17 deletions.
13 changes: 11 additions & 2 deletions readconf.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: readconf.c,v 1.368 2022/06/03 04:30:47 djm Exp $ */
/* $OpenBSD: readconf.c,v 1.369 2022/09/17 10:33:18 djm Exp $ */
/*
* Author: Tatu Ylonen <[email protected]>
* Copyright (c) 1995 Tatu Ylonen <[email protected]>, Espoo, Finland
Expand Down Expand Up @@ -174,7 +174,7 @@ typedef enum {
oStreamLocalBindMask, oStreamLocalBindUnlink, oRevokedHostKeys,
oFingerprintHash, oUpdateHostkeys, oHostbasedAcceptedAlgorithms,
oPubkeyAcceptedAlgorithms, oCASignatureAlgorithms, oProxyJump,
oSecurityKeyProvider, oKnownHostsCommand,
oSecurityKeyProvider, oKnownHostsCommand, oRequiredRSASize,
oIgnore, oIgnoredUnknownOption, oDeprecated, oUnsupported
} OpCodes;

Expand Down Expand Up @@ -320,6 +320,7 @@ static struct {
{ "proxyjump", oProxyJump },
{ "securitykeyprovider", oSecurityKeyProvider },
{ "knownhostscommand", oKnownHostsCommand },
{ "requiredrsasize", oRequiredRSASize },

{ NULL, oBadOption }
};
Expand Down Expand Up @@ -2176,6 +2177,10 @@ process_config_line_depth(Options *options, struct passwd *pw, const char *host,
*charptr = xstrdup(arg);
break;

case oRequiredRSASize:
intptr = &options->required_rsa_size;
goto parse_int;

case oDeprecated:
debug("%s line %d: Deprecated option \"%s\"",
filename, linenum, keyword);
Expand Down Expand Up @@ -2423,6 +2428,7 @@ initialize_options(Options * options)
options->hostbased_accepted_algos = NULL;
options->pubkey_accepted_algos = NULL;
options->known_hosts_command = NULL;
options->required_rsa_size = -1;
}

/*
Expand Down Expand Up @@ -2619,6 +2625,8 @@ fill_default_options(Options * options)
if (options->sk_provider == NULL)
options->sk_provider = xstrdup("$SSH_SK_PROVIDER");
#endif
if (options->required_rsa_size == -1)
options->required_rsa_size = SSH_RSA_MINIMUM_MODULUS_SIZE;

/* Expand KEX name lists */
all_cipher = cipher_alg_list(',', 0);
Expand Down Expand Up @@ -3308,6 +3316,7 @@ dump_client_config(Options *o, const char *host)
dump_cfg_int(oNumberOfPasswordPrompts, o->number_of_password_prompts);
dump_cfg_int(oServerAliveCountMax, o->server_alive_count_max);
dump_cfg_int(oServerAliveInterval, o->server_alive_interval);
dump_cfg_int(oRequiredRSASize, o->required_rsa_size);

/* String options */
dump_cfg_string(oBindAddress, o->bind_address);
Expand Down
4 changes: 3 additions & 1 deletion readconf.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: readconf.h,v 1.147 2022/06/03 04:30:47 djm Exp $ */
/* $OpenBSD: readconf.h,v 1.148 2022/09/17 10:33:18 djm Exp $ */

/*
* Author: Tatu Ylonen <[email protected]>
Expand Down Expand Up @@ -176,6 +176,8 @@ typedef struct {

char *known_hosts_command;

int required_rsa_size; /* minimum size of RSA keys */

char *ignored_unknown; /* Pattern list of unknown tokens to ignore */
} Options;

Expand Down
5 changes: 3 additions & 2 deletions ssh.1
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.\"
.\" $OpenBSD: ssh.1,v 1.431 2022/05/28 05:57:56 jmc Exp $
.Dd $Mdocdate: May 28 2022 $
.\" $OpenBSD: ssh.1,v 1.432 2022/09/17 10:33:18 djm Exp $
.Dd $Mdocdate: September 17 2022 $
.Dt SSH 1
.Os
.Sh NAME
Expand Down Expand Up @@ -571,6 +571,7 @@ For full details of the options listed below, and their possible values, see
.It RemoteCommand
.It RemoteForward
.It RequestTTY
.It RequiredRSASize
.It SendEnv
.It ServerAliveInterval
.It ServerAliveCountMax
Expand Down
27 changes: 18 additions & 9 deletions ssh.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: ssh.c,v 1.575 2022/07/01 00:36:30 djm Exp $ */
/* $OpenBSD: ssh.c,v 1.576 2022/09/17 10:33:18 djm Exp $ */
/*
* Author: Tatu Ylonen <[email protected]>
* Copyright (c) 1995 Tatu Ylonen <[email protected]>, Espoo, Finland
Expand Down Expand Up @@ -516,14 +516,22 @@ resolve_canonicalize(char **hostp, int port)
}

/*
* Check the result of hostkey loading, ignoring some errors and
* fatal()ing for others.
* Check the result of hostkey loading, ignoring some errors and either
* discarding the key or fatal()ing for others.
*/
static void
check_load(int r, const char *path, const char *message)
check_load(int r, struct sshkey **k, const char *path, const char *message)
{
switch (r) {
case 0:
/* Check RSA keys size and discard if undersized */
if (k != NULL && *k != NULL &&
(r = sshkey_check_rsa_length(*k,
options.required_rsa_size)) != 0) {
error_r(r, "load %s \"%s\"", message, path);
free(*k);
*k = NULL;
}
break;
case SSH_ERR_INTERNAL_ERROR:
case SSH_ERR_ALLOC_FAIL:
Expand Down Expand Up @@ -1578,15 +1586,16 @@ main(int ac, char **av)
if ((o) >= sensitive_data.nkeys) \
fatal_f("pubkey out of array bounds"); \
check_load(sshkey_load_public(p, &(sensitive_data.keys[o]), NULL), \
p, "pubkey"); \
&(sensitive_data.keys[o]), p, "pubkey"); \
if (sensitive_data.keys[o] != NULL) \
debug2("hostbased key %d: %s key from \"%s\"", o, \
sshkey_ssh_name(sensitive_data.keys[o]), p); \
} while (0)
#define L_CERT(p,o) do { \
if ((o) >= sensitive_data.nkeys) \
fatal_f("cert out of array bounds"); \
check_load(sshkey_load_cert(p, &(sensitive_data.keys[o])), p, "cert"); \
check_load(sshkey_load_cert(p, &(sensitive_data.keys[o])), \
&(sensitive_data.keys[o]), p, "cert"); \
if (sensitive_data.keys[o] != NULL) \
debug2("hostbased key %d: %s cert from \"%s\"", o, \
sshkey_ssh_name(sensitive_data.keys[o]), p); \
Expand Down Expand Up @@ -2265,7 +2274,7 @@ load_public_identity_files(const struct ssh_conn_info *cinfo)
filename = default_client_percent_dollar_expand(cp, cinfo);
free(cp);
check_load(sshkey_load_public(filename, &public, NULL),
filename, "pubkey");
&public, filename, "pubkey");
debug("identity file %s type %d", filename,
public ? public->type : -1);
free(options.identity_files[i]);
Expand All @@ -2284,7 +2293,7 @@ load_public_identity_files(const struct ssh_conn_info *cinfo)
continue;
xasprintf(&cp, "%s-cert", filename);
check_load(sshkey_load_public(cp, &public, NULL),
filename, "pubkey");
&public, filename, "pubkey");
debug("identity file %s type %d", cp,
public ? public->type : -1);
if (public == NULL) {
Expand Down Expand Up @@ -2315,7 +2324,7 @@ load_public_identity_files(const struct ssh_conn_info *cinfo)
free(cp);

check_load(sshkey_load_public(filename, &public, NULL),
filename, "certificate");
&public, filename, "certificate");
debug("certificate file %s type %d", filename,
public ? public->type : -1);
free(options.certificate_files[i]);
Expand Down
15 changes: 13 additions & 2 deletions ssh_config.5
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
.\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
.\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
.\"
.\" $OpenBSD: ssh_config.5,v 1.373 2022/06/24 04:27:14 djm Exp $
.Dd $Mdocdate: June 24 2022 $
.\" $OpenBSD: ssh_config.5,v 1.374 2022/09/17 10:33:18 djm Exp $
.Dd $Mdocdate: September 17 2022 $
.Dt SSH_CONFIG 5
.Os
.Sh NAME
Expand Down Expand Up @@ -1634,6 +1634,17 @@ and
.Fl T
flags for
.Xr ssh 1 .
.It Cm RequiredRSASize
Specifies the minimum RSA key size (in bits) that
.Xr ssh 1
will accept.
User authentication keys smaller than this limit will be ignored.
Servers that present host keys smaller than this limit will cause the
connection to be terminated.
The default is
.Cm 1024
bits.
Note that this limit may only be raised from the default.
.It Cm RevokedHostKeys
Specifies revoked host public keys.
Keys listed in this file will be refused for host authentication.
Expand Down
20 changes: 19 additions & 1 deletion sshconnect2.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* $OpenBSD: sshconnect2.c,v 1.360 2022/08/19 06:07:47 djm Exp $ */
/* $OpenBSD: sshconnect2.c,v 1.361 2022/09/17 10:33:18 djm Exp $ */
/*
* Copyright (c) 2000 Markus Friedl. All rights reserved.
* Copyright (c) 2008 Damien Miller. All rights reserved.
Expand Down Expand Up @@ -96,6 +96,11 @@ static const struct ssh_conn_info *xxx_conn_info;
static int
verify_host_key_callback(struct sshkey *hostkey, struct ssh *ssh)
{
int r;

if ((r = sshkey_check_rsa_length(hostkey,
options.required_rsa_size)) != 0)
fatal_r(r, "Bad server host key");
if (verify_host_key(xxx_host, xxx_hostaddr, hostkey,
xxx_conn_info) == -1)
fatal("Host key verification failed.");
Expand Down Expand Up @@ -1606,6 +1611,13 @@ load_identity_file(Identity *id)
private = NULL;
quit = 1;
}
if (!quit && (r = sshkey_check_rsa_length(private,
options.required_rsa_size)) != 0) {
debug_fr(r, "Skipping key %s", id->filename);
sshkey_free(private);
private = NULL;
quit = 1;
}
if (!quit && private != NULL && id->agent_fd == -1 &&
!(id->key && id->isprivate))
maybe_add_key_to_agent(id->filename, private, comment,
Expand Down Expand Up @@ -1752,6 +1764,12 @@ pubkey_prepare(struct ssh *ssh, Authctxt *authctxt)
/* list of keys supported by the agent */
if ((r = get_agent_identities(ssh, &agent_fd, &idlist)) == 0) {
for (j = 0; j < idlist->nkeys; j++) {
if ((r = sshkey_check_rsa_length(idlist->keys[j],
options.required_rsa_size)) != 0) {
debug_fr(r, "ignoring %s agent key",
sshkey_ssh_name(idlist->keys[j]));
continue;
}
found = 0;
TAILQ_FOREACH(id, &files, next) {
/*
Expand Down

0 comments on commit 54b333d

Please sign in to comment.