Skip to content
This repository has been archived by the owner on Apr 19, 2023. It is now read-only.
Permalink
Browse files Browse the repository at this point in the history
Block injection attack in ssh public key handling (#673)
Also needed to fix the setup a bit to address a version incompatibility
  • Loading branch information
jingw committed Mar 28, 2022
1 parent 30755ea commit d930879
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
7 changes: 7 additions & 0 deletions grouper/public_key.py
Expand Up @@ -72,6 +72,13 @@ def add_public_key(session, user, public_key_str):
except sshpubkeys.InvalidKeyException as e:
raise PublicKeyParseError(str(e))

# Allowing newlines can lead to injection attacks depending on how the key is
# consumed, such as if it's dumped in an authorized_keys file with a `command`
# restriction.
# Note parsing the key is insufficient to block this.
if "\r" in public_key_str or "\n" in public_key_str:
raise PublicKeyParseError("Public key cannot have newlines")

try:
get_plugin_proxy().will_add_public_key(pubkey)
except PluginRejectedPublicKey as e:
Expand Down
9 changes: 9 additions & 0 deletions tests/constants.py
Expand Up @@ -16,6 +16,15 @@

SSH_KEY_BAD = "ssh-rsa AAAblahblahkey some-comment"

SSH_KEY_BAD_MULTILINE = SSH_KEY_1 + "\r" + SSH_KEY_2

SSH2_KEY_BAD = """\
---- BEGIN SSH2 PUBLIC KEY ----
foobar: this is a chance to hide bad things
AAAAC3NzaC1lZDI1NTE5AAAAIJ5O/AXibtVhySDYn60ATXftAU1oCe4BQubFYoV2juEb
---- END SSH2 PUBLIC KEY ----
"""

SSH_KEY_RSA_1024 = (
"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQDhKgSsCJR9UyQa/Gsheb5F56hg23CVnCLpmpyC2JMqVEptG9CL83Oft"
"pOPvEb/785Act4En1VFvwMwTj25VurbG3XI984csiNdWPlM1ke4lHK2PQepSYyZVYn+hhXhzSILNDixhBYeDVv4GOfJM1"
Expand Down
12 changes: 11 additions & 1 deletion tests/public_key_test.py
Expand Up @@ -11,7 +11,7 @@
get_public_keys_of_user,
PublicKeyParseError,
)
from tests.constants import SSH_KEY_1, SSH_KEY_BAD
from tests.constants import SSH2_KEY_BAD, SSH_KEY_1, SSH_KEY_BAD, SSH_KEY_BAD_MULTILINE
from tests.fixtures import session, users # noqa: F401


Expand Down Expand Up @@ -41,6 +41,16 @@ def test_bad_key(session, users): # noqa: F811
assert get_public_keys_of_user(session, user.id) == []


@pytest.mark.parametrize("key", [SSH_KEY_BAD_MULTILINE, SSH2_KEY_BAD])
def test_multiline_key(key, session, users): # noqa: F811
user = users["cbguder@a.co"]

with pytest.raises(PublicKeyParseError, match="Public key cannot have newlines"):
add_public_key(session, user, key)

assert get_public_keys_of_user(session, user.id) == []


@patch("grouper.public_key.get_plugin_proxy")
def test_rejected_key(get_plugin_proxy, session, users): # noqa: F811
get_plugin_proxy.return_value = PluginProxy([PublicKeyPlugin()])
Expand Down

0 comments on commit d930879

Please sign in to comment.