File better-handling-of-bad-public-keys-from-minions-bsc-.patch of Package salt
From 27d02cebeb215f3e93c0db9c52eea271ed7ad4ed Mon Sep 17 00:00:00 2001
From: "Daniel A. Wozniak" <dwozniak@saltstack.com>
Date: Mon, 2 Aug 2021 13:50:37 -0700
Subject: [PATCH] Better handling of bad public keys from minions
(bsc#1189040)
Add changelog for #57733
Fix pre-commit check
Add missing test
Fix test on older pythons
---
changelog/57733.fixed | 1 +
salt/crypt.py | 16 ++++++++++++---
salt/exceptions.py | 6 ++++++
salt/key.py | 35 ++++++++++++++++----------------
salt/transport/mixins/auth.py | 2 +-
tests/pytests/unit/test_crypt.py | 20 ++++++++++++++++++
6 files changed, 58 insertions(+), 22 deletions(-)
create mode 100644 changelog/57733.fixed
create mode 100644 tests/pytests/unit/test_crypt.py
diff --git a/changelog/57733.fixed b/changelog/57733.fixed
new file mode 100644
index 0000000000..0cd55b19a6
--- /dev/null
+++ b/changelog/57733.fixed
@@ -0,0 +1 @@
+Better handling of bad RSA public keys from minions
diff --git a/salt/crypt.py b/salt/crypt.py
index 6d45a0d053..be426c6ab3 100644
--- a/salt/crypt.py
+++ b/salt/crypt.py
@@ -73,7 +73,11 @@ import salt.utils.user
import salt.utils.verify
import salt.version
from salt.exceptions import (
- AuthenticationError, SaltClientError, SaltReqTimeoutError, MasterExit
+ AuthenticationError,
+ InvalidKeyError,
+ MasterExit,
+ SaltClientError,
+ SaltReqTimeoutError,
)
log = logging.getLogger(__name__)
@@ -210,10 +214,16 @@ def get_rsa_pub_key(path):
with salt.utils.files.fopen(path, 'rb') as f:
data = f.read().replace(b'RSA ', b'')
bio = BIO.MemoryBuffer(data)
- key = RSA.load_pub_key_bio(bio)
+ try:
+ key = RSA.load_pub_key_bio(bio)
+ except RSA.RSAError:
+ raise InvalidKeyError("Encountered bad RSA public key")
else:
with salt.utils.files.fopen(path) as f:
- key = RSA.importKey(f.read())
+ try:
+ key = RSA.importKey(f.read())
+ except (ValueError, IndexError, TypeError):
+ raise InvalidKeyError("Encountered bad RSA public key")
return key
diff --git a/salt/exceptions.py b/salt/exceptions.py
index 58cb8b9204..d570cb8df5 100644
--- a/salt/exceptions.py
+++ b/salt/exceptions.py
@@ -114,6 +114,12 @@ class AuthenticationError(SaltException):
'''
+class InvalidKeyError(SaltException):
+ """
+ Raised when we encounter an invalid RSA key.
+ """
+
+
class CommandNotFoundError(SaltException):
'''
Used in modules or grains when a required binary is not available
diff --git a/salt/key.py b/salt/key.py
index 6cc4c33a75..809ccc7bde 100644
--- a/salt/key.py
+++ b/salt/key.py
@@ -10,6 +10,7 @@ import os
import shutil
import fnmatch
import logging
+import sys
# Import salt libs
import salt.cache
@@ -625,30 +626,28 @@ class Key(object):
keydirs.append(self.REJ)
if include_denied:
keydirs.append(self.DEN)
+ invalid_keys = []
for keydir in keydirs:
for key in matches.get(keydir, []):
+ key_path = os.path.join(self.opts["pki_dir"], keydir, key)
+ try:
+ salt.crypt.get_rsa_pub_key(key_path)
+ except salt.exceptions.InvalidKeyError:
+ log.error("Invalid RSA public key: %s", key)
+ invalid_keys.append((keydir, key))
+ continue
try:
shutil.move(
- os.path.join(
- self.opts['pki_dir'],
- keydir,
- key),
- os.path.join(
- self.opts['pki_dir'],
- self.ACC,
- key)
- )
- eload = {'result': True,
- 'act': 'accept',
- 'id': key}
- self.event.fire_event(eload,
- salt.utils.event.tagify(prefix='key'))
+ key_path, os.path.join(self.opts["pki_dir"], self.ACC, key),
+ )
+ eload = {"result": True, "act": "accept", "id": key}
+ self.event.fire_event(eload, salt.utils.event.tagify(prefix="key"))
except (IOError, OSError):
pass
- return (
- self.name_match(match) if match is not None
- else self.dict_match(matches)
- )
+ for keydir, key in invalid_keys:
+ matches[keydir].remove(key)
+ sys.stderr.write("Unable to accept invalid key for {}.\n".format(key))
+ return self.name_match(match) if match is not None else self.dict_match(matches)
def accept_all(self):
'''
diff --git a/salt/transport/mixins/auth.py b/salt/transport/mixins/auth.py
index e7a0b96eee..ec736227ab 100644
--- a/salt/transport/mixins/auth.py
+++ b/salt/transport/mixins/auth.py
@@ -434,7 +434,7 @@ class AESReqServerMixin(object):
# and an empty request comes in
try:
pub = salt.crypt.get_rsa_pub_key(pubfn)
- except (ValueError, IndexError, TypeError) as err:
+ except salt.crypt.InvalidKeyError as err:
log.error('Corrupt public key "%s": %s', pubfn, err)
return {'enc': 'clear',
'load': {'ret': False}}
diff --git a/tests/pytests/unit/test_crypt.py b/tests/pytests/unit/test_crypt.py
new file mode 100644
index 0000000000..aa8f439b8c
--- /dev/null
+++ b/tests/pytests/unit/test_crypt.py
@@ -0,0 +1,20 @@
+"""
+tests.pytests.unit.test_crypt
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Unit tests for salt's crypt module
+"""
+import pytest
+import salt.crypt
+import salt.utils.files
+
+
+def test_get_rsa_pub_key_bad_key(tmp_path):
+ """
+ get_rsa_pub_key raises InvalidKeyError when encoutering a bad key
+ """
+ key_path = str(tmp_path / "key")
+ with salt.utils.files.fopen(key_path, "w") as fp:
+ fp.write("")
+ with pytest.raises(salt.crypt.InvalidKeyError):
+ salt.crypt.get_rsa_pub_key(key_path)
--
2.32.0
