Clean up un-needed re-init crypto and test fix

This commit is contained in:
Daniel A. Wozniak 2024-05-25 01:27:41 -07:00 committed by Daniel Wozniak
parent 6fe58ffd30
commit b32f09b655
13 changed files with 553 additions and 461 deletions

View file

@ -53,7 +53,10 @@ class ReqServerChannel:
def __init__(self, opts, transport):
self.opts = opts
self.transport = transport
self.event = None
self.event = salt.utils.event.get_master_event(
self.opts, self.opts["sock_dir"], listen=False
)
self.master_key = salt.crypt.MasterKeys(self.opts)
def pre_fork(self, process_manager):
"""

View file

@ -35,14 +35,6 @@ from salt.exceptions import (
)
from salt.template import compile_template
try:
import Cryptodome.Random
except ImportError:
try:
import Crypto.Random # nosec
except ImportError:
pass # pycrypto < 2.1
log = logging.getLogger(__name__)
@ -2288,8 +2280,6 @@ def create_multiprocessing(parallel_data, queue=None):
This function will be called from another process when running a map in
parallel mode. The result from the create is always a json object.
"""
salt.utils.crypt.reinit_crypto()
parallel_data["opts"]["output"] = "json"
cloud = Cloud(parallel_data["opts"])
try:
@ -2318,8 +2308,6 @@ def destroy_multiprocessing(parallel_data, queue=None):
This function will be called from another process when running a map in
parallel mode. The result from the destroy is always a json object.
"""
salt.utils.crypt.reinit_crypto()
parallel_data["opts"]["output"] = "json"
clouds = salt.loader.clouds(parallel_data["opts"])
@ -2350,8 +2338,6 @@ def run_parallel_map_providers_query(data, queue=None):
This function will be called from another process when building the
providers map.
"""
salt.utils.crypt.reinit_crypto()
cloud = Cloud(data["opts"])
try:
with salt.utils.context.func_globals_inject(

View file

@ -622,8 +622,6 @@ class AsyncAuth:
self.get_keys()
self.io_loop = io_loop or salt.ext.tornado.ioloop.IOLoop.current()
salt.utils.crypt.reinit_crypto()
key = self.__key(self.opts)
# TODO: if we already have creds for this key, lets just re-use
if key in AsyncAuth.creds_map:

View file

@ -37,7 +37,6 @@ import salt.serializers.msgpack
import salt.state
import salt.utils.args
import salt.utils.atomicfile
import salt.utils.crypt
import salt.utils.event
import salt.utils.files
import salt.utils.gitfs
@ -1156,7 +1155,6 @@ class MWorker(salt.utils.process.SignalHandlingProcess):
)
self.clear_funcs.connect()
self.aes_funcs = AESFuncs(self.opts)
salt.utils.crypt.reinit_crypto()
self.__bind()

View file

@ -39,7 +39,6 @@ import salt.syspaths
import salt.transport
import salt.utils.args
import salt.utils.context
import salt.utils.crypt
import salt.utils.data
import salt.utils.dictdiffer
import salt.utils.dictupdate
@ -1811,7 +1810,6 @@ class Minion(MinionBase):
name=name,
args=(instance, self.opts, data, self.connected, creds_map),
)
process.register_after_fork_method(salt.utils.crypt.reinit_crypto)
else:
process = threading.Thread(
target=self._target,

View file

@ -17,7 +17,6 @@ import os
import subprocess
import sys
import salt.utils.crypt
import salt.utils.files
import salt.utils.fsutils
import salt.utils.path
@ -579,10 +578,9 @@ if __name__ == "__main__":
# Double-fork stuff
try:
if os.fork() > 0:
salt.utils.crypt.reinit_crypto()
sys.exit(0)
else:
salt.utils.crypt.reinit_crypto()
pass
except OSError as ex:
sys.exit(1)
@ -592,7 +590,6 @@ if __name__ == "__main__":
try:
pid = os.fork()
if pid > 0:
salt.utils.crypt.reinit_crypto()
with salt.utils.files.fopen(
os.path.join(pidfile, EnvLoader.PID_FILE), "w"
) as fp_:
@ -601,5 +598,4 @@ if __name__ == "__main__":
except OSError as ex:
sys.exit(1)
salt.utils.crypt.reinit_crypto()
main(dbfile, pidfile, mode)

View file

@ -12,35 +12,6 @@ from salt.exceptions import SaltInvocationError
log = logging.getLogger(__name__)
try:
import M2Crypto # pylint: disable=unused-import
Random = None
HAS_M2CRYPTO = True
except ImportError:
HAS_M2CRYPTO = False
if not HAS_M2CRYPTO:
try:
from Cryptodome import Random
HAS_CRYPTODOME = True
except ImportError:
HAS_CRYPTODOME = False
else:
HAS_CRYPTODOME = False
if not HAS_M2CRYPTO and not HAS_CRYPTODOME:
try:
from Crypto import Random # nosec
HAS_CRYPTO = True
except ImportError:
HAS_CRYPTO = False
else:
HAS_CRYPTO = False
def decrypt(
data, rend, translate_newlines=False, renderers=None, opts=None, valid_rend=None
):
@ -117,20 +88,6 @@ def decrypt(
return rend_func(data, translate_newlines=translate_newlines)
def reinit_crypto():
"""
When a fork arises, pycrypto needs to reinit
From its doc::
Caveat: For the random number generator to work correctly,
you must call Random.atfork() in both the parent and
child processes after using os.fork()
"""
if HAS_CRYPTODOME or HAS_CRYPTO:
Random.atfork()
def pem_finger(path=None, key=None, sum_type="sha256"):
"""
Pass in either a raw pem string, or the path on disk to the location of a

View file

@ -996,6 +996,9 @@ def salt_syndic_master_factory(
config_overrides = {
"log_level_logfile": "quiet",
"fips_mode": FIPS_TESTRUN,
"publish_signing_algorithm": (
"PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1"
),
}
ext_pillar = []
if salt.utils.platform.is_windows():
@ -1112,6 +1115,9 @@ def salt_master_factory(
config_overrides = {
"log_level_logfile": "quiet",
"fips_mode": FIPS_TESTRUN,
"publish_signing_algorithm": (
"PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1"
),
}
ext_pillar = []
if salt.utils.platform.is_windows():
@ -1221,6 +1227,8 @@ def salt_minion_factory(salt_master_factory):
"file_roots": salt_master_factory.config["file_roots"].copy(),
"pillar_roots": salt_master_factory.config["pillar_roots"].copy(),
"fips_mode": FIPS_TESTRUN,
"rsa_encryption_algorithm": "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1",
"rsa_signing_algorithm": "PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1",
}
virtualenv_binary = get_virtualenv_binary_path()
@ -1253,6 +1261,8 @@ def salt_sub_minion_factory(salt_master_factory):
"file_roots": salt_master_factory.config["file_roots"].copy(),
"pillar_roots": salt_master_factory.config["pillar_roots"].copy(),
"fips_mode": FIPS_TESTRUN,
"rsa_encryption_algorithm": "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1",
"rsa_signing_algorithm": "PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1",
}
virtualenv_binary = get_virtualenv_binary_path()

View file

@ -191,6 +191,9 @@ def salt_master_factory(
config_overrides = {
"pytest-master": {"log": {"level": "DEBUG"}},
"fips_mode": FIPS_TESTRUN,
"publish_signing_algorithm": (
"PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA224"
),
}
ext_pillar = []
if salt.utils.platform.is_windows():
@ -321,6 +324,8 @@ def salt_minion_factory(salt_master_factory, salt_minion_id, sdb_etcd_port, vaul
"file_roots": salt_master_factory.config["file_roots"].copy(),
"pillar_roots": salt_master_factory.config["pillar_roots"].copy(),
"fips_mode": FIPS_TESTRUN,
"rsa_encryption_algorithm": "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1",
"rsa_signing_algorithm": "PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1",
}
virtualenv_binary = get_virtualenv_binary_path()
@ -352,6 +357,8 @@ def salt_sub_minion_factory(salt_master_factory, salt_sub_minion_id):
"file_roots": salt_master_factory.config["file_roots"].copy(),
"pillar_roots": salt_master_factory.config["pillar_roots"].copy(),
"fips_mode": FIPS_TESTRUN,
"rsa_encryption_algorithm": "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1",
"rsa_signing_algorithm": "PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1",
}
virtualenv_binary = get_virtualenv_binary_path()

View file

@ -3,6 +3,7 @@ import os
import pytest
import salt.config
from tests.conftest import FIPS_TESTRUN
@pytest.fixture
@ -10,6 +11,7 @@ def minion_opts(tmp_path):
"""
Default minion configuration with relative temporary paths to not require root permissions.
"""
print(f"WTF {FIPS_TESTRUN}")
root_dir = tmp_path / "minion"
opts = salt.config.DEFAULT_MINION_OPTS.copy()
opts["__role"] = "minion"
@ -23,6 +25,9 @@ def minion_opts(tmp_path):
opts[name] = str(dirpath)
opts["log_file"] = "logs/minion.log"
opts["conf_file"] = os.path.join(opts["conf_dir"], "minion")
opts["fips_mode"] = FIPS_TESTRUN
opts["encryption_algorithm"] = "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1"
opts["signing_algorithm"] = "PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1"
return opts
@ -41,6 +46,10 @@ def master_opts(tmp_path):
opts[name] = str(dirpath)
opts["log_file"] = "logs/master.log"
opts["conf_file"] = os.path.join(opts["conf_dir"], "master")
opts["fips_mode"] = FIPS_TESTRUN
opts["publish_signing_algorithm"] = (
"PKCS1v15-SHA224" if FIPS_TESTRUN else "PKCS1v15-SHA1"
)
return opts

View file

@ -2,6 +2,7 @@ import hashlib
import hmac
import os
import cryptography.exceptions
import pytest
from cryptography.hazmat.backends.openssl import backend
from cryptography.hazmat.primitives import serialization
@ -29,6 +30,45 @@ else:
from . import AES, PKCS1_OAEP
OPENSSL_ENCRYPTED_KEY = """
-----BEGIN RSA PRIVATE KEY-----
Proc-Type: 4,ENCRYPTED
DEK-Info: AES-256-CBC,12E2FE8CA93672B629477073867DE7E4
3wgKQlgjT3G+8rIQt97AXMDByh5u9JMZPYOB9/wg3iC3qoJXfoFAsCNUjODJBnkI
j9Zgj/bOCaSM3UshMQXmYY+2Rfi1SVQPnETlqEH/plMZS38tB8mN5pBdthgGTw6c
rhpj/S23eZT5d+z5ODeVYlWCVhx8CtE8OQEzOQk8dxLWbVHhvgC3uJGWOPR3P7VM
BlxH5LxWRCrC8vzbnwwaJnX8BTQ7fc4qeGwHlXBjpnxQhxO27pEj08NQ0/lfKh1b
seX5uiCjuQhHFKNGTuA16rQIe6BkYRHIWCDhySftl/lqSfLQif0OAXaHEdL2EdIS
ySD12RYyNUDotEzYFF+qzJ5OAtraqvc8kYror7oN52bHKCjJyrp4+DWA4/N7FjTV
+FqUyJNKqw1DAQAxlZlq+GgNyi8+g/Zs2TKTc/ZXaPLjtWYOQEQkYaNBoaD3ydY3
c7x+uQtJLVW9BF9FSl9A7BItpqZQWKiHiGtUdhYOkemlR+zMatjBe/eTq8LrnEDa
IyI+rRo1PSDAz3n1pdEAzGAOeqwT+j7YG9O8+dybMY5FcAtiiPX21nIpmP+Rtx4X
GqzHsT7nM6QG4O8GPKuK6TniG+Q0doNWomwuau/cjQgL4C+yFiX3kIPpHz9kA/aS
NL1SJqSsvc3D/KlRbHXaJZJyhmzDuEbQynkaAdvejiajlJWAwA3BZWw1RUK7Wn8m
XcNPJL3g02uKq8SUDgVQl/cx4QawuWri2Xh8/xakNYWzNU2feoWBmV+gN2qDSxyz
Qi+xu3CzdJVrPs71lW0rEAIQvU3K3Umava9M4CUF6R7N9+Zv+m1EuMQs0IGt8VCY
Wo1KY5PAb/V718d1C3I6kXvLSDXG8xqyEleilPLhKCRGPK+2g0nGYu562EV1i6by
gr+PLnFJTfgHEzwIfsqfNoR8ReQ6AJKJoniQr4vqex9xtifuhes0odpqmUB4/B2C
UfY/SpJR6tzdrGndpB/vb1vjHumHklHHWrLONtz70BhR8Zaisc7SCmL5bFgWqzMC
MJKPulRRGQCPAzy5OI/ZULY8+dzlva1MyoCYlWjeUtcUAy+9dyA8GZv75ez9g71b
10nNINDcvGG7zWShSYrAKrvLlsoE7eZ+flG+XhI2CfiC9/zHBzy/slbaH9H+1tlO
VWKiw6iBb2TEvBk4Wpk2nUFlWKtkkBVAlgbShbE2K8pTHrJeIRv5J897k693NFZE
DjVVJirzMv/OiZTami0qBQ4nDtUZpH8FsFZ8DtREkhROsDmrjq9PGkOVaxEyF/ke
avJT34gp4OoNWj7/Rd1YNbGiWjMEB3zi9y1Q6Ufiod9ZlK3RQb4tNrpzDn/msdJo
pIkuByWjXjF4YQRKtAoeCn+CWiY7L/Qi8X7jmX27JLILlZPtTJ+aNp3eCr6ZX0dW
y0uhN89sgMewlvDA/pduL/VJRHUBZC2eD8FbD7p6K+yRKhdciS9A8F6aIhx615s6
mngRBfwzh8ST6yQgLwCgle/XaRYTWJKjzAe3lkaIBBhHpeuq1UMAjunoS8JnLNiy
xQJ0PznzY57sYKpIiClwMjfpnX47nTU2DFWuPEXvBtG1xMjacGPbVrUslesY5bii
-----END RSA PRIVATE KEY-----
"""
@pytest.fixture
def openssl_encrypted_key():
return OPENSSL_ENCRYPTED_KEY
@pytest.fixture
def passphrase():
return "pass1234"
@ -284,3 +324,51 @@ def test_sign_message_with_passphrase(signature, signing_algorithm):
def test_verify_signature():
with patch("salt.utils.files.fopen", mock_open(read_data=PUBKEY_DATA.encode())):
assert salt.crypt.verify_signature("/keydir/keyname.pub", MSG, SIG)
def test_loading_encrypted_openssl_format(openssl_encrypted_key, passphrase, tmp_path):
path = tmp_path / "key"
path.write_text(openssl_encrypted_key)
if FIPS_TESTRUN:
with pytest.raises(ValueError):
salt.crypt.get_rsa_key(path, passphrase)
else:
try:
salt.crypt.get_rsa_key(path, passphrase)
# BaseException to catch errors bubbling up from the cryptogrphy's
# rust layer.
except BaseException as exc: # pylint: disable=broad-except
pytest.fail(f"Unexpected exception: {exc}")
@pytest.mark.skipif(not FIPS_TESTRUN, reason="Only valid when in FIPS mode")
def test_fips_bad_signing_algo(private_key, passphrase):
key = salt.crypt.PrivateKey(private_key, passphrase)
with pytest.raises(cryptography.exceptions.UnsupportedAlgorithm):
key.sign("meh", salt.crypt.PKCS1v15_SHA1)
@pytest.mark.skipif(not FIPS_TESTRUN, reason="Only valid when in FIPS mode")
def test_fips_bad_signing_algo_verification(private_key, passphrase):
lpriv = LegacyPrivateKey(private_key.encode(), passphrase.encode())
data = b"meh"
signature = lpriv.sign(data)
pubkey = salt.crypt.PublicKey(private_key.replace(".pem", ".pub"))
# cryptogrpahy silently returns False on unsuppoted algorithm
assert pubkey.verify(signature, salt.crypt.PKCS1v15_SHA1) is False
@pytest.mark.skipif(not FIPS_TESTRUN, reason="Only valid when in FIPS mode")
def test_fips_bad_encryption_algo(private_key, passphrase):
key = salt.crypt.PublicKey(private_key.replace(".pem", ".pub"))
with pytest.raises(cryptography.exceptions.UnsupportedAlgorithm):
key.encrypt("meh", salt.crypt.OAEP_SHA1)
@pytest.mark.skipif(not FIPS_TESTRUN, reason="Only valid when in FIPS mode")
def test_fips_bad_decryption_algo(private_key, passphrase):
pubkey = LegacyPublicKey(private_key.replace(".pem", ".pub"))
data = pubkey.encrypt("meh")
key = salt.crypt.PrivateKey(private_key, passphrase)
with pytest.raises(cryptography.exceptions.UnsupportedAlgorithm):
key.decrypt(data)

File diff suppressed because it is too large Load diff

View file

@ -5,29 +5,6 @@ Unit tests for salt.utils.crypt.py
import pytest
import salt.utils.crypt
from tests.support.mock import patch
try:
import M2Crypto # pylint: disable=unused-import
HAS_M2CRYPTO = True
except ImportError:
HAS_M2CRYPTO = False
try:
from Cryptodome import Random as CryptodomeRandom
HAS_CYPTODOME = True
except ImportError:
HAS_CYPTODOME = False
try:
from Crypto import Random as CryptoRandom # nosec
HAS_CRYPTO = True
except ImportError:
HAS_CRYPTO = False
@pytest.fixture
@ -45,28 +22,6 @@ def pub_key_data():
]
def test_random():
# make sure the right library is used for random
if HAS_M2CRYPTO:
assert None is salt.utils.crypt.Random
elif HAS_CYPTODOME:
assert CryptodomeRandom is salt.utils.crypt.Random
elif HAS_CRYPTO:
assert CryptoRandom is salt.utils.crypt.Random
def test_reinit_crypto():
# make sure reinit crypto does not crash
salt.utils.crypt.reinit_crypto()
# make sure reinit does not crash when no crypt is found
with patch("salt.utils.crypt.HAS_M2CRYPTO", False):
with patch("salt.utils.crypt.HAS_CRYPTODOME", False):
with patch("salt.utils.crypt.HAS_CRYPTO", False):
with patch("salt.utils.crypt.Random", None):
salt.utils.crypt.reinit_crypto()
@pytest.mark.parametrize("line_ending", ["\n", "\r\n"])
def test_pem_finger_file_line_endings(tmp_path, pub_key_data, line_ending):
key_file = tmp_path / "master_crlf.pub"