From 383f10fb84a0fee3c01f9d97594f3e22aa3cd5e0 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 16 Nov 2015 00:30:02 +0000 Subject: upstream commit Add a new authorized_keys option "restrict" that includes all current and future key restrictions (no-*-forwarding, etc). Also add permissive versions of the existing restrictions, e.g. "no-pty" -> "pty". This simplifies the task of setting up restricted keys and ensures they are maximally-restricted, regardless of any permissions we might implement in the future. Example: restrict,pty,command="nethack" ssh-ed25519 AAAAC3NzaC1lZDI1... Idea from Jann Horn; ok markus@ Upstream-ID: 04ceb9d448e46e67e13887a7ae5ea45b4f1719d0 --- auth-options.c | 87 ++++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 30 deletions(-) (limited to 'auth-options.c') diff --git a/auth-options.c b/auth-options.c index e387697d3..cb68802de 100644 --- a/auth-options.c +++ b/auth-options.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.c,v 1.68 2015/07/03 03:43:18 djm Exp $ */ +/* $OpenBSD: auth-options.c,v 1.69 2015/11/16 00:30:02 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -87,6 +87,36 @@ auth_clear_options(void) channel_clear_permitted_opens(); } +/* + * Match flag 'opt' in *optsp, and if allow_negate is set then also match + * 'no-opt'. Returns -1 if option not matched, 1 if option matches or 0 + * if negated option matches. + * If the option or negated option matches, then *optsp is updated to + * point to the first character after the option and, if 'msg' is not NULL + * then a message based on it added via auth_debug_add(). + */ +static int +match_flag(const char *opt, int allow_negate, char **optsp, const char *msg) +{ + size_t opt_len = strlen(opt); + char *opts = *optsp; + int negate = 0; + + if (allow_negate && strncasecmp(opts, "no-", 3) == 0) { + opts += 3; + negate = 1; + } + if (strncasecmp(opts, opt, opt_len) == 0) { + *optsp = opts + opt_len; + if (msg != NULL) { + auth_debug_add("%s %s.", msg, + negate ? "disabled" : "enabled"); + } + return negate ? 0 : 1; + } + return -1; +} + /* * return 1 if access is granted, 0 if not. * side effect: sets key option flags @@ -95,7 +125,7 @@ int auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) { const char *cp; - int i; + int i, r; /* reset options */ auth_clear_options(); @@ -104,45 +134,42 @@ auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) return 1; while (*opts && *opts != ' ' && *opts != '\t') { - cp = "cert-authority"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - key_is_cert_authority = 1; - opts += strlen(cp); + if ((r = match_flag("cert-authority", 0, &opts, NULL)) != -1) { + key_is_cert_authority = r; goto next_option; } - cp = "no-port-forwarding"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - auth_debug_add("Port forwarding disabled."); + if ((r = match_flag("restrict", 0, &opts, NULL)) != -1) { + auth_debug_add("Key is restricted."); no_port_forwarding_flag = 1; - opts += strlen(cp); + no_agent_forwarding_flag = 1; + no_x11_forwarding_flag = 1; + no_pty_flag = 1; + no_user_rc = 1; goto next_option; } - cp = "no-agent-forwarding"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - auth_debug_add("Agent forwarding disabled."); - no_agent_forwarding_flag = 1; - opts += strlen(cp); + if ((r = match_flag("port-forwarding", 1, &opts, + "Port forwarding")) != -1) { + no_port_forwarding_flag = r != 1; goto next_option; } - cp = "no-X11-forwarding"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - auth_debug_add("X11 forwarding disabled."); - no_x11_forwarding_flag = 1; - opts += strlen(cp); + if ((r = match_flag("agent-forwarding", 1, &opts, + "Agent forwarding")) != -1) { + no_agent_forwarding_flag = r != 1; goto next_option; } - cp = "no-pty"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - auth_debug_add("Pty allocation disabled."); - no_pty_flag = 1; - opts += strlen(cp); + if ((r = match_flag("x11-forwarding", 1, &opts, + "X11 forwarding")) != -1) { + no_x11_forwarding_flag = r != 1; goto next_option; } - cp = "no-user-rc"; - if (strncasecmp(opts, cp, strlen(cp)) == 0) { - auth_debug_add("User rc file execution disabled."); - no_user_rc = 1; - opts += strlen(cp); + if ((r = match_flag("pty", 1, &opts, + "PTY allocation")) != -1) { + no_pty_flag = r != 1; + goto next_option; + } + if ((r = match_flag("user-rc", 1, &opts, + "User rc execution")) != -1) { + no_user_rc = r != 1; goto next_option; } cp = "command=\""; -- cgit v1.2.3 From d59ce08811bf94111c2f442184cf7d1257ffae24 Mon Sep 17 00:00:00 2001 From: "mmcc@openbsd.org" Date: Thu, 10 Dec 2015 17:08:40 +0000 Subject: upstream commit Remove NULL-checks before free(). ok dtucker@ Upstream-ID: e3d3cb1ce900179906af36517b5eea0fb15e6ef8 --- auth-options.c | 26 +++++++++----------------- authfile.c | 5 ++--- cipher.c | 5 ++--- kex.c | 5 ++--- packet.c | 14 +++++--------- ssh-dss.c | 5 ++--- ssh-rsa.c | 5 ++--- ssh.c | 5 ++--- sshconnect2.c | 5 ++--- sshd.c | 5 ++--- sshkey.c | 17 ++++++----------- 11 files changed, 36 insertions(+), 61 deletions(-) (limited to 'auth-options.c') diff --git a/auth-options.c b/auth-options.c index cb68802de..edbaf80bb 100644 --- a/auth-options.c +++ b/auth-options.c @@ -1,4 +1,4 @@ -/* $OpenBSD: auth-options.c,v 1.69 2015/11/16 00:30:02 djm Exp $ */ +/* $OpenBSD: auth-options.c,v 1.70 2015/12/10 17:08:40 mmcc Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -75,14 +75,10 @@ auth_clear_options(void) free(ce->s); free(ce); } - if (forced_command) { - free(forced_command); - forced_command = NULL; - } - if (authorized_principals) { - free(authorized_principals); - authorized_principals = NULL; - } + free(forced_command); + forced_command = NULL; + free(authorized_principals); + authorized_principals = NULL; forced_tun_device = -1; channel_clear_permitted_opens(); } @@ -175,8 +171,7 @@ auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) cp = "command=\""; if (strncasecmp(opts, cp, strlen(cp)) == 0) { opts += strlen(cp); - if (forced_command != NULL) - free(forced_command); + free(forced_command); forced_command = xmalloc(strlen(opts) + 1); i = 0; while (*opts) { @@ -206,8 +201,7 @@ auth_parse_options(struct passwd *pw, char *opts, char *file, u_long linenum) cp = "principals=\""; if (strncasecmp(opts, cp, strlen(cp)) == 0) { opts += strlen(cp); - if (authorized_principals != NULL) - free(authorized_principals); + free(authorized_principals); authorized_principals = xmalloc(strlen(opts) + 1); i = 0; while (*opts) { @@ -593,8 +587,7 @@ parse_option_list(struct sshbuf *oblob, struct passwd *pw, free(*cert_forced_command); *cert_forced_command = NULL; } - if (name != NULL) - free(name); + free(name); sshbuf_free(data); sshbuf_free(c); return ret; @@ -638,8 +631,7 @@ auth_cert_options(struct sshkey *k, struct passwd *pw) no_user_rc |= cert_no_user_rc; /* CA-specified forced command supersedes key option */ if (cert_forced_command != NULL) { - if (forced_command != NULL) - free(forced_command); + free(forced_command); forced_command = cert_forced_command; } return 0; diff --git a/authfile.c b/authfile.c index 1907cb1cc..668df7d9e 100644 --- a/authfile.c +++ b/authfile.c @@ -1,4 +1,4 @@ -/* $OpenBSD: authfile.c,v 1.117 2015/09/13 14:39:16 tim Exp $ */ +/* $OpenBSD: authfile.c,v 1.118 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000, 2013 Markus Friedl. All rights reserved. * @@ -426,8 +426,7 @@ sshkey_load_cert(const char *filename, struct sshkey **keyp) r = 0; out: - if (file != NULL) - free(file); + free(file); if (pub != NULL) sshkey_free(pub); return r; diff --git a/cipher.c b/cipher.c index 02dae6f9f..13847e5bd 100644 --- a/cipher.c +++ b/cipher.c @@ -1,4 +1,4 @@ -/* $OpenBSD: cipher.c,v 1.100 2015/01/14 10:29:45 djm Exp $ */ +/* $OpenBSD: cipher.c,v 1.101 2015/12/10 17:08:40 mmcc Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -353,8 +353,7 @@ cipher_init(struct sshcipher_ctx *cc, const struct sshcipher *cipher, if (cipher->discard_len > 0) { if ((junk = malloc(cipher->discard_len)) == NULL || (discard = malloc(cipher->discard_len)) == NULL) { - if (junk != NULL) - free(junk); + free(junk); ret = SSH_ERR_ALLOC_FAIL; goto bad; } diff --git a/kex.c b/kex.c index c1371c432..8243164f4 100644 --- a/kex.c +++ b/kex.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kex.c,v 1.113 2015/12/04 16:41:28 markus Exp $ */ +/* $OpenBSD: kex.c,v 1.114 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * @@ -906,8 +906,7 @@ derive_key(struct ssh *ssh, int id, u_int need, u_char *hash, u_int hashlen, digest = NULL; r = 0; out: - if (digest) - free(digest); + free(digest); ssh_digest_free(hashctx); return r; } diff --git a/packet.c b/packet.c index 378906956..06e16536c 100644 --- a/packet.c +++ b/packet.c @@ -1,4 +1,4 @@ -/* $OpenBSD: packet.c,v 1.218 2015/12/04 16:41:28 markus Exp $ */ +/* $OpenBSD: packet.c,v 1.219 2015/12/10 17:08:40 mmcc Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -519,10 +519,8 @@ ssh_packet_close(struct ssh *ssh) error("%s: cipher_cleanup failed: %s", __func__, ssh_err(r)); if ((r = cipher_cleanup(&state->receive_context)) != 0) error("%s: cipher_cleanup failed: %s", __func__, ssh_err(r)); - if (ssh->remote_ipaddr) { - free(ssh->remote_ipaddr); - ssh->remote_ipaddr = NULL; - } + free(ssh->remote_ipaddr); + ssh->remote_ipaddr = NULL; free(ssh->state); ssh->state = NULL; } @@ -1784,8 +1782,7 @@ ssh_packet_read_poll_seqnr(struct ssh *ssh, u_char *typep, u_int32_t *seqnr_p) if ((r = sshpkt_get_u8(ssh, NULL)) != 0 || (r = sshpkt_get_string(ssh, &msg, NULL)) != 0 || (r = sshpkt_get_string(ssh, NULL, NULL)) != 0) { - if (msg) - free(msg); + free(msg); return r; } debug("Remote: %.900s", msg); @@ -2570,8 +2567,7 @@ newkeys_from_blob(struct sshbuf *m, struct ssh *ssh, int mode) newkey = NULL; r = 0; out: - if (newkey != NULL) - free(newkey); + free(newkey); if (b != NULL) sshbuf_free(b); return r; diff --git a/ssh-dss.c b/ssh-dss.c index 8ed19d849..254f2a39b 100644 --- a/ssh-dss.c +++ b/ssh-dss.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-dss.c,v 1.32 2014/06/24 01:13:21 djm Exp $ */ +/* $OpenBSD: ssh-dss.c,v 1.33 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * @@ -211,8 +211,7 @@ ssh_dss_verify(const struct sshkey *key, DSA_SIG_free(sig); if (b != NULL) sshbuf_free(b); - if (ktype != NULL) - free(ktype); + free(ktype); if (sigblob != NULL) { explicit_bzero(sigblob, len); free(sigblob); diff --git a/ssh-rsa.c b/ssh-rsa.c index 6b8589522..4eb00c87c 100644 --- a/ssh-rsa.c +++ b/ssh-rsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-rsa.c,v 1.56 2015/12/07 20:04:09 markus Exp $ */ +/* $OpenBSD: ssh-rsa.c,v 1.57 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000, 2003 Markus Friedl * @@ -226,8 +226,7 @@ ssh_rsa_verify(const struct sshkey *key, explicit_bzero(sigblob, len); free(sigblob); } - if (ktype != NULL) - free(ktype); + free(ktype); if (b != NULL) sshbuf_free(b); explicit_bzero(digest, sizeof(digest)); diff --git a/ssh.c b/ssh.c index 38e2b6674..37dcdc705 100644 --- a/ssh.c +++ b/ssh.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh.c,v 1.430 2015/11/19 08:23:27 djm Exp $ */ +/* $OpenBSD: ssh.c,v 1.431 2015/12/10 17:08:40 mmcc Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -911,8 +911,7 @@ main(int ac, char **av) subsystem_flag = 1; break; case 'S': - if (options.control_path != NULL) - free(options.control_path); + free(options.control_path); options.control_path = xstrdup(optarg); break; case 'b': diff --git a/sshconnect2.c b/sshconnect2.c index da1bd3847..3c5afe507 100644 --- a/sshconnect2.c +++ b/sshconnect2.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshconnect2.c,v 1.231 2015/12/04 16:41:28 markus Exp $ */ +/* $OpenBSD: sshconnect2.c,v 1.232 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2008 Damien Miller. All rights reserved. @@ -1257,8 +1257,7 @@ load_identity_file(Identity *id) explicit_bzero(passphrase, strlen(passphrase)); free(passphrase); } - if (comment) - free(comment); + free(comment); if (private != NULL || quit) break; } diff --git a/sshd.c b/sshd.c index 2f3f5b551..5d2e0a03c 100644 --- a/sshd.c +++ b/sshd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshd.c,v 1.461 2015/12/04 16:41:28 markus Exp $ */ +/* $OpenBSD: sshd.c,v 1.462 2015/12/10 17:08:40 mmcc Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1257,8 +1257,7 @@ server_accept_loop(int *sock_in, int *sock_out, int *newsock, int *config_s) for (;;) { if (received_sighup) sighup_restart(); - if (fdset != NULL) - free(fdset); + free(fdset); fdset = xcalloc(howmany(maxfd + 1, NFDBITS), sizeof(fd_mask)); diff --git a/sshkey.c b/sshkey.c index 587bf5b84..87abea1e0 100644 --- a/sshkey.c +++ b/sshkey.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sshkey.c,v 1.28 2015/12/04 16:41:28 markus Exp $ */ +/* $OpenBSD: sshkey.c,v 1.29 2015/12/10 17:08:40 mmcc Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * Copyright (c) 2008 Alexander von Gernler. All rights reserved. @@ -426,12 +426,10 @@ cert_free(struct sshkey_cert *cert) sshbuf_free(cert->critical); if (cert->extensions != NULL) sshbuf_free(cert->extensions); - if (cert->key_id != NULL) - free(cert->key_id); + free(cert->key_id); for (i = 0; i < cert->nprincipals; i++) free(cert->principals[i]); - if (cert->principals != NULL) - free(cert->principals); + free(cert->principals); if (cert->signature_key != NULL) sshkey_free(cert->signature_key); explicit_bzero(cert, sizeof(*cert)); @@ -2473,10 +2471,8 @@ sshkey_certify(struct sshkey *k, struct sshkey *ca) out: if (ret != 0) sshbuf_reset(cert); - if (sig_blob != NULL) - free(sig_blob); - if (ca_blob != NULL) - free(ca_blob); + free(sig_blob); + free(ca_blob); if (principals != NULL) sshbuf_free(principals); return ret; @@ -3764,8 +3760,7 @@ sshkey_parse_private_rsa1(struct sshbuf *blob, const char *passphrase, } out: explicit_bzero(&ciphercontext, sizeof(ciphercontext)); - if (comment != NULL) - free(comment); + free(comment); if (prv != NULL) sshkey_free(prv); if (copy != NULL) -- cgit v1.2.3