rpms/pcs
7
0
Fork 0
Code Issues Pull Requests Releases Activity
79e064f384
pcs/bz1927404-01-replace-pyOpenSSL-with-python-crypt.patch
Miroslav Lisik 79e064f384 Resolves: rhbz#1927404 
- Replace pyOpenSSL with python-cryptography
2021-03-04 11:50:09 +01:00

703 lines
21 KiB
Diff
Raw Blame History

From 68157f21fe8051ebd7eace11012738d8d91a1812 Mon Sep 17 00:00:00 2001
From: Tomas Jelinek <tojeline@redhat.com>
Date: Tue, 2 Mar 2021 14:47:27 +0100
Subject: [PATCH 1/2] squash bz1927404: replace pyOpenSSL with
python-cryptography
python-cryptography requires new mypy
improve TLS certificate verification
Library methods are now used instead of running openssl processes. That
enabled support for Elliptic Curve certificates.
cleanup dependencies in spec file
---
.gitlab-ci.yml | 6 +-
README.md | 2 +-
mypy.ini | 7 +-
pcs.spec.in | 5 +-
pcs/common/ssl.py | 108 +++++++++++++++++-------------
pcs/daemon/ssl.py | 48 ++-----------
pcs/pcsd.py | 19 +++---
pcs/utils.py | 32 ---------
pcs_test/tier0/daemon/test_ssl.py | 64 +++++++++---------
pcsd/pcs.rb | 39 ++++-------
pcsd/remote.rb | 2 +-
requirements.txt | 2 +-
test/centos8/Dockerfile | 2 +-
test/fedora31/Dockerfile | 2 +-
test/fedora32/Dockerfile | 2 +-
15 files changed, 135 insertions(+), 205 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4cb4c14b..72668787 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -46,12 +46,12 @@ pylint:
script:
- "dnf install -y
python3
+ python3-cryptography
python3-dateutil
python3-distro
python3-lxml
python3-pip
python3-pycurl
- python3-pyOpenSSL
python3-pyparsing
findutils
make
@@ -66,12 +66,12 @@ mypy:
script:
- "dnf install -y
python3
+ python3-cryptography
python3-dateutil
python3-distro
python3-lxml
python3-pip
python3-pycurl
- python3-pyOpenSSL
python3-pyparsing
git
make
@@ -111,12 +111,12 @@ python_tier0_tests:
- "dnf install -y
make
python3
+ python3-cryptography
python3-dateutil
python3-distro
python3-lxml
python3-pip
python3-pycurl
- python3-pyOpenSSL
python3-pyparsing
which
"
diff --git a/README.md b/README.md
index fe6eeed6..a0c01c02 100644
--- a/README.md
+++ b/README.md
@@ -30,7 +30,7 @@ These are the runtime dependencies of pcs and pcsd:
* python3-lxml
* python3-pycurl
* python3-setuptools
-* python3-pyOpenSSL (python3-openssl)
+* python3-cryptography
* python3-pyparsing
* python3-tornado 6.1.0+
* python dataclasses (`pip install dataclasses`; required only for python 3.6,
diff --git a/mypy.ini b/mypy.ini
index 6d3d2ff9..e3198530 100644
--- a/mypy.ini
+++ b/mypy.ini
@@ -48,6 +48,10 @@ disallow_untyped_defs = True
# this is a temporary solution for legacy code
disallow_untyped_defs = False
+[mypy-pcs.common.ssl]
+disallow_untyped_defs = True
+disallow_untyped_calls = True
+
[mypy-pcs.common.types]
disallow_untyped_defs = True
disallow_untyped_calls = True
@@ -122,9 +126,6 @@ ignore_missing_imports = True
[mypy-distro]
ignore_missing_imports = True
-[mypy-OpenSSL]
-ignore_missing_imports = True
-
[mypy-pyagentx.*]
ignore_errors = True
diff --git a/pcs.spec.in b/pcs.spec.in
index db66c5b0..610fad50 100644
--- a/pcs.spec.in
+++ b/pcs.spec.in
@@ -139,6 +139,7 @@ BuildRequires: python3-setuptools_scm
BuildRequires: python3-devel
# for tier0 tests
+BuildRequires: python3-cryptography
BuildRequires: python3-pyparsing
# gcc for compiling custom rubygems
@@ -171,6 +172,7 @@ Requires: platform-python
Requires: platform-python-setuptools
%endif
+Requires: python3-cryptography
Requires: python3-lxml
Requires: python3-pycurl
Requires: python3-pyparsing
@@ -190,9 +192,6 @@ Requires: ruby >= 2.2.0
Requires: rubygems
# for killall
Requires: psmisc
-# for working with certificates (validation etc.)
-Requires: openssl
-Requires: python3-pyOpenSSL
# cluster stack and related packages
Requires: pacemaker >= 2.0.0
Requires: corosync >= 3.0
diff --git a/pcs/common/ssl.py b/pcs/common/ssl.py
index 852fea80..74ddd4ec 100644
--- a/pcs/common/ssl.py
+++ b/pcs/common/ssl.py
@@ -1,45 +1,63 @@
-import time
-from OpenSSL import crypto
-
-
-def cert_date_format(timestamp):
- return str.encode(time.strftime("%Y%m%d%H%M%SZ", time.gmtime(timestamp)))
-
-
-def generate_key(length=3072):
- key = crypto.PKey()
- key.generate_key(crypto.TYPE_RSA, length)
- return key
-
-
-def generate_cert(key, server_name):
- now = time.time()
- cert = crypto.X509()
-
- subject = cert.get_subject()
- subject.countryName = "US"
- subject.stateOrProvinceName = "MN"
- subject.localityName = "Minneapolis"
- subject.organizationName = "pcsd"
- subject.organizationalUnitName = "pcsd"
- subject.commonName = server_name
-
- cert.set_version(2)
- cert.set_serial_number(int(now * 1000))
- cert.set_notBefore(cert_date_format(now))
- cert.set_notAfter(
- cert_date_format(now + 60 * 60 * 24 * 365 * 10)
- ) # 10 years
- cert.set_issuer(subject)
- cert.set_pubkey(key)
- cert.sign(key, "sha256")
-
- return cert
-
-
-def dump_cert(certificate):
- return crypto.dump_certificate(crypto.FILETYPE_PEM, certificate)
-
-
-def dump_key(key):
- return crypto.dump_privatekey(crypto.FILETYPE_PEM, key)
+import datetime
+import ssl
+from typing import List
+
+from cryptography import x509
+from cryptography.x509.oid import NameOID
+from cryptography.hazmat.backends import default_backend
+from cryptography.hazmat.primitives import hashes, serialization
+from cryptography.hazmat.primitives.asymmetric import rsa
+
+
+def check_cert_key(cert_path: str, key_path: str) -> List[str]:
+ errors = []
+ try:
+ ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
+ ssl_context.load_cert_chain(cert_path, key_path)
+ except ssl.SSLError as e:
+ errors.append(f"SSL certificate does not match the key: {e}")
+ except EnvironmentError as e:
+ errors.append(f"Unable to load SSL certificate and/or key: {e}")
+ return errors
+
+
+def generate_key(length: int = 3072) -> rsa.RSAPrivateKeyWithSerialization:
+ return rsa.generate_private_key(
+ public_exponent=65537, key_size=length, backend=default_backend()
+ )
+
+
+def generate_cert(key: rsa.RSAPrivateKey, server_name: str) -> x509.Certificate:
+ now = datetime.datetime.utcnow()
+ subject = x509.Name(
+ [
+ x509.NameAttribute(NameOID.COUNTRY_NAME, "US"),
+ x509.NameAttribute(NameOID.STATE_OR_PROVINCE_NAME, "MN"),
+ x509.NameAttribute(NameOID.LOCALITY_NAME, "Minneapolis"),
+ x509.NameAttribute(NameOID.ORGANIZATION_NAME, "pcsd"),
+ x509.NameAttribute(NameOID.ORGANIZATIONAL_UNIT_NAME, "pcsd"),
+ x509.NameAttribute(NameOID.COMMON_NAME, server_name),
+ ]
+ )
+ return (
+ x509.CertificateBuilder()
+ .subject_name(subject)
+ .issuer_name(subject)
+ .public_key(key.public_key())
+ .serial_number(int(now.timestamp() * 1000))
+ .not_valid_before(now)
+ .not_valid_after(now + datetime.timedelta(days=3650))
+ .sign(key, hashes.SHA256(), default_backend())
+ )
+
+
+def dump_cert(certificate: x509.Certificate) -> bytes:
+ return certificate.public_bytes(serialization.Encoding.PEM)
+
+
+def dump_key(key: rsa.RSAPrivateKeyWithSerialization) -> bytes:
+ return key.private_bytes(
+ serialization.Encoding.PEM,
+ serialization.PrivateFormat.TraditionalOpenSSL,
+ serialization.NoEncryption(),
+ )
diff --git a/pcs/daemon/ssl.py b/pcs/daemon/ssl.py
index 40cca314..43865631 100644
--- a/pcs/daemon/ssl.py
+++ b/pcs/daemon/ssl.py
@@ -1,9 +1,8 @@
import os
import ssl
-from OpenSSL import crypto, SSL
-
from pcs.common.ssl import (
+ check_cert_key,
dump_cert,
dump_key,
generate_cert,
@@ -11,53 +10,15 @@ from pcs.common.ssl import (
)
-def check_cert_key(cert_path, key_path):
- errors = []
-
- def load(load_ssl_file, label, path):
- try:
- with open(path) as ssl_file:
- return load_ssl_file(crypto.FILETYPE_PEM, ssl_file.read())
- except EnvironmentError as e:
- errors.append(f"Unable to read SSL {label} '{path}': '{e}'")
- except crypto.Error as e:
- msg = ""
- if e.args and e.args[0] and e.args[0][0]:
- msg = f": '{':'.join(e.args[0][0])}'"
- errors.append(f"Invalid SSL {label} '{path}'{msg}")
-
- cert = load(crypto.load_certificate, "certificate", cert_path)
- key = load(crypto.load_privatekey, "key", key_path)
-
- if errors:
- return errors
-
- try:
- context = SSL.Context(SSL.TLSv1_METHOD)
- context.use_privatekey(key)
- context.use_certificate(cert)
- except SSL.Error as e:
- errors.append(f"Unable to load SSL certificate and/or key: {e}")
- # If we cannot load the files, do not confuse users with other error
- # messages.
- return errors
- try:
- context.check_privatekey()
- except (crypto.Error, SSL.Error) as e:
- errors.append(f"SSL certificate does not match the key: {e}")
-
- return errors
-
-
-def open_ssl_file_to_rewrite(path):
+def _open_ssl_file_to_rewrite(path):
return os.fdopen(os.open(path, os.O_CREAT | os.O_WRONLY, 0o600), "wb")
def regenerate_cert_key(server_name, cert_path, key_path, key_length=None):
key = generate_key(key_length) if key_length else generate_key()
- with open_ssl_file_to_rewrite(cert_path) as cert_file:
+ with _open_ssl_file_to_rewrite(cert_path) as cert_file:
cert_file.write(dump_cert(generate_cert(key, server_name)))
- with open_ssl_file_to_rewrite(key_path) as key_file:
+ with _open_ssl_file_to_rewrite(key_path) as key_file:
key_file.write(dump_key(key))
@@ -102,7 +63,6 @@ class PcsdSSL:
self.__ck_pair = CertKeyPair(cert_location, key_location)
def create_context(self) -> ssl.SSLContext:
- # pylint: disable=no-member
ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
ssl_context.set_ciphers(self.__ssl_ciphers)
ssl_context.options = self.__ssl_options
diff --git a/pcs/pcsd.py b/pcs/pcsd.py
index f3e6bca3..d5ddb443 100644
--- a/pcs/pcsd.py
+++ b/pcs/pcsd.py
@@ -5,6 +5,7 @@ import sys
from pcs import settings
from pcs import utils
from pcs.cli.common.errors import CmdLineInputError
+import pcs.common.ssl
def pcsd_certkey(lib, argv, modifiers):
@@ -21,13 +22,13 @@ def pcsd_certkey(lib, argv, modifiers):
keyfile = argv[1]
try:
- with open(certfile, "r") as myfile:
+ with open(certfile, "rb") as myfile:
cert = myfile.read()
- with open(keyfile, "r") as myfile:
+ with open(keyfile, "rb") as myfile:
key = myfile.read()
except IOError as e:
utils.err(e)
- errors = utils.verify_cert_key_pair(cert, key)
+ errors = pcs.common.ssl.check_cert_key(certfile, keyfile)
if errors:
for err in errors:
utils.err(err, False)
@@ -43,12 +44,12 @@ def pcsd_certkey(lib, argv, modifiers):
try:
try:
- os.chmod(settings.pcsd_cert_location, 0o700)
+ os.chmod(settings.pcsd_cert_location, 0o600)
except OSError: # If the file doesn't exist, we don't care
pass
try:
- os.chmod(settings.pcsd_key_location, 0o700)
+ os.chmod(settings.pcsd_key_location, 0o600)
except OSError: # If the file doesn't exist, we don't care
pass
@@ -56,9 +57,9 @@ def pcsd_certkey(lib, argv, modifiers):
os.open(
settings.pcsd_cert_location,
os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
- 0o700,
+ 0o600,
),
- "w",
+ "wb",
) as myfile:
myfile.write(cert)
@@ -66,9 +67,9 @@ def pcsd_certkey(lib, argv, modifiers):
os.open(
settings.pcsd_key_location,
os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
- 0o700,
+ 0o600,
),
- "w",
+ "wb",
) as myfile:
myfile.write(key)
diff --git a/pcs/utils.py b/pcs/utils.py
index 97a04787..59d1b66e 100644
--- a/pcs/utils.py
+++ b/pcs/utils.py
@@ -2105,38 +2105,6 @@ def is_iso8601_date(var):
return retVal == 0
-def verify_cert_key_pair(cert, key):
- """
- Commandline options: no options
- """
- errors = []
- cert_modulus = ""
- key_modulus = ""
-
- output, retval = run(
- ["/usr/bin/openssl", "x509", "-modulus", "-noout"],
- string_for_stdin=cert,
- )
- if retval != 0:
- errors.append("Invalid certificate: {0}".format(output.strip()))
- else:
- cert_modulus = output.strip()
-
- output, retval = run(
- ["/usr/bin/openssl", "rsa", "-modulus", "-noout"], string_for_stdin=key
- )
- if retval != 0:
- errors.append("Invalid key: {0}".format(output.strip()))
- else:
- key_modulus = output.strip()
-
- if not errors and cert_modulus and key_modulus:
- if cert_modulus != key_modulus:
- errors.append("Certificate does not match the key")
-
- return errors
-
-
def err(errorText, exit_after_error=True):
sys.stderr.write("Error: %s\n" % errorText)
if exit_after_error:
diff --git a/pcs_test/tier0/daemon/test_ssl.py b/pcs_test/tier0/daemon/test_ssl.py
index e80f7a30..2b2edd36 100644
--- a/pcs_test/tier0/daemon/test_ssl.py
+++ b/pcs_test/tier0/daemon/test_ssl.py
@@ -1,8 +1,7 @@
import os
+import ssl
from unittest import mock, TestCase
-from OpenSSL import SSL
-
from pcs_test.tools.misc import get_tmp_dir
from pcs.daemon.ssl import PcsdSSL, CertKeyPair, SSLCertKeyException
@@ -19,19 +18,6 @@ class SslFilesMixin:
self.ssl_dir = get_tmp_dir("tier0_daemon_ssl")
self.cert_path = os.path.join(self.ssl_dir.name, "daemon.cert")
self.key_path = os.path.join(self.ssl_dir.name, "daemon.key")
- # various versions of OpenSSL / PyOpenSSL emit different messages
- self.DAMAGED_SSL_FILES_ERRORS_1 = (
- f"Invalid SSL certificate '{self.cert_path}':"
- " 'PEM routines:PEM_read_bio:no start line'",
- f"Invalid SSL key '{self.key_path}':"
- " 'PEM routines:PEM_read_bio:no start line'",
- )
- self.DAMAGED_SSL_FILES_ERRORS_2 = (
- f"Invalid SSL certificate '{self.cert_path}':"
- " 'PEM routines:get_name:no start line'",
- f"Invalid SSL key '{self.key_path}':"
- " 'PEM routines:get_name:no start line'",
- )
def tearDown(self):
# pylint cannot possibly know this is being mixed into TestCase classes
@@ -56,21 +42,31 @@ class Pair(SslFilesMixin, TestCase):
def test_error_if_files_with_bad_content(self):
self.damage_ssl_files()
- self.assertTrue(
- self.pair.check()
- in [
- list(self.DAMAGED_SSL_FILES_ERRORS_1),
- list(self.DAMAGED_SSL_FILES_ERRORS_2),
- ]
+ errors = self.pair.check()
+ self.assertEqual(len(errors), 1)
+ self.assertRegex(
+ errors[0],
+ r"^SSL certificate does not match the key: "
+ r"\[SSL\] PEM lib \(_ssl\.c:\d+\)",
)
- @mock.patch("pcs.daemon.ssl.SSL.Context.use_privatekey")
- def test_error_if_short_key(self, mock_use_key):
- mock_use_key.side_effect = SSL.Error("reason")
+ @mock.patch("pcs.daemon.ssl.ssl.SSLContext.load_cert_chain")
+ def test_error_if_short_key(self, mock_load_cert_chain):
+ mock_load_cert_chain.side_effect = ssl.SSLError(
+ # These are the real args of the exception.
+ 336245135,
+ "[SSL: EE_KEY_TOO_SMALL] ee key too small (_ssl.c:3542)",
+ )
+ # 512 cannot be used as we would get an error from FIPS and 1024 is
+ # long enough. So a mock must be used.
self.pair.regenerate(SERVER_NAME, 1024)
errors = self.pair.check()
self.assertEqual(
- errors, ["Unable to load SSL certificate and/or key: reason"]
+ errors,
+ [
+ "SSL certificate does not match the key: "
+ "[SSL: EE_KEY_TOO_SMALL] ee key too small (_ssl.c:3542)",
+ ],
)
def test_error_if_cert_does_not_match_key(self):
@@ -83,8 +79,10 @@ class Pair(SslFilesMixin, TestCase):
errors = self.pair.check()
self.assertEqual(len(errors), 1)
- self.assertTrue(
- errors[0].startswith("SSL certificate does not match the key:")
+ self.assertRegex(
+ errors[0],
+ r"SSL certificate does not match the key: "
+ r"\[X509: KEY_VALUES_MISMATCH\] key values mismatch \(_ssl\.c:\d+\)",
)
@@ -102,12 +100,12 @@ class PcsdSSLTest(SslFilesMixin, TestCase):
self.damage_ssl_files()
with self.assertRaises(SSLCertKeyException) as ctx_manager:
self.pcsd_ssl.guarantee_valid_certs()
- self.assertTrue(
- ctx_manager.exception.args
- in [
- self.DAMAGED_SSL_FILES_ERRORS_1,
- self.DAMAGED_SSL_FILES_ERRORS_2,
- ]
+ errors = ctx_manager.exception.args
+ self.assertEqual(len(errors), 1)
+ self.assertRegex(
+ errors[0],
+ r"SSL certificate does not match the key: "
+ r"\[SSL\] PEM lib \(_ssl\.c:\d+\)",
)
def test_context_uses_given_options(self):
diff --git a/pcsd/pcs.rb b/pcsd/pcs.rb
index bce8e39e..89c26f33 100644
--- a/pcsd/pcs.rb
+++ b/pcsd/pcs.rb
@@ -12,6 +12,7 @@ require 'fileutils'
require 'backports/latest'
require 'base64'
require 'ethon'
+require 'openssl'
require 'config.rb'
require 'cfgsync.rb'
@@ -1170,39 +1171,23 @@ def read_file_lock(path, binary=false)
end
end
-def verify_cert_key_pair(cert, key)
+def verify_cert_key_pair(cert_data, key_data)
errors = []
- cert_modulus = nil
- key_modulus = nil
- stdout, stderr, retval = run_cmd_options(
- PCSAuth.getSuperuserAuth(),
- {
- 'stdin' => cert,
- },
- '/usr/bin/openssl', 'x509', '-modulus', '-noout'
- )
- if retval != 0
- errors << "Invalid certificate: #{stderr.join}"
- else
- cert_modulus = stdout.join.strip
+ begin
+ cert = OpenSSL::X509::Certificate.new(cert_data)
+ rescue OpenSSL::X509::CertificateError => e
+ errors << "Invalid certificate: #{e}"
end
- stdout, stderr, retval = run_cmd_options(
- PCSAuth.getSuperuserAuth(),
- {
- 'stdin' => key,
- },
- '/usr/bin/openssl', 'rsa', '-modulus', '-noout'
- )
- if retval != 0
- errors << "Invalid key: #{stderr.join}"
- else
- key_modulus = stdout.join.strip
+ begin
+ key = OpenSSL::PKey.read(key_data)
+ rescue OpenSSL::PKey::PKeyError => e
+ errors << "Invalid key: #{e}"
end
- if errors.empty? and cert_modulus and key_modulus
- if cert_modulus != key_modulus
+ if errors.empty?
+ if not cert.check_private_key(key)
errors << 'Certificate does not match the key'
end
end
diff --git a/pcsd/remote.rb b/pcsd/remote.rb
index 3361b3f6..c43e3116 100644
--- a/pcsd/remote.rb
+++ b/pcsd/remote.rb
@@ -694,7 +694,7 @@ def set_certs(params, request, auth_user)
if !ssl_cert.empty? and !ssl_key.empty?
ssl_errors = verify_cert_key_pair(ssl_cert, ssl_key)
if ssl_errors and !ssl_errors.empty?
- return [400, ssl_errors.join]
+ return [400, ssl_errors.join('; ')]
end
begin
write_file_lock(CRT_FILE, 0600, ssl_cert)
diff --git a/requirements.txt b/requirements.txt
index eb42ce40..2f62b1c3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,7 +2,7 @@
astroid==2.4.2
pylint==2.6.0
tornado>=6.1.0
-mypy==0.790
+mypy==0.812
dacite
# temporarily stick to previous version until it's convinient to reformat code
black==20.8b1
diff --git a/test/centos8/Dockerfile b/test/centos8/Dockerfile
index 910d7652..00c17ffe 100644
--- a/test/centos8/Dockerfile
+++ b/test/centos8/Dockerfile
@@ -7,11 +7,11 @@ RUN dnf install -y \
--enablerepo=PowerTools \
# python
python3 \
+ python3-cryptography \
python3-lxml \
python3-mock \
python3-pip \
python3-pycurl \
- python3-pyOpenSSL \
python3-pyparsing \
# ruby
ruby \
diff --git a/test/fedora31/Dockerfile b/test/fedora31/Dockerfile
index cc94bee2..8d0a0672 100644
--- a/test/fedora31/Dockerfile
+++ b/test/fedora31/Dockerfile
@@ -5,11 +5,11 @@ ARG src_path
RUN dnf install -y \
# python
python3 \
+ python3-cryptography \
python3-lxml \
python3-mock \
python3-pip \
python3-pycurl \
- python3-pyOpenSSL \
python3-pyparsing \
# ruby
ruby \
diff --git a/test/fedora32/Dockerfile b/test/fedora32/Dockerfile
index 82bdff74..750ff979 100644
--- a/test/fedora32/Dockerfile
+++ b/test/fedora32/Dockerfile
@@ -5,12 +5,12 @@ ARG src_path
RUN dnf install -y \
# python
python3 \
+ python3-cryptography \
python3-distro \
python3-lxml \
python3-mock \
python3-pip \
python3-pycurl \
- python3-pyOpenSSL \
python3-pyparsing \
# ruby
ruby \
--
2.26.2
Reference in New Issue View Git Blame Copy Permalink