Copilot commented on code in PR #2133:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2133#discussion_r2990035665
##########
behave_framework/src/minifi_test_framework/core/minifi_test_context.py:
##########
@@ -16,32 +16,46 @@
#
from __future__ import annotations
-from typing import TYPE_CHECKING
+
+import os
+import docker
from behave.runner import Context
+from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey
+from cryptography.x509 import Certificate
from docker.models.networks import Network
-from minifi_test_framework.containers.container import Container
-from OpenSSL import crypto
+from minifi_test_framework.containers.container_protocol import
ContainerProtocol
+from minifi_test_framework.containers.minifi_protocol import MinifiProtocol
-if TYPE_CHECKING:
- from minifi_test_framework.containers.minifi_container import
MinifiContainer
DEFAULT_MINIFI_CONTAINER_NAME = "minifi-primary"
+class MinifiContainer(ContainerProtocol, MinifiProtocol):
+ pass
+
+
class MinifiTestContext(Context):
- containers: dict[str, Container]
+ containers: dict[str, ContainerProtocol]
scenario_id: str
network: Network
minifi_container_image: str
resource_dir: str | None
- root_ca_key: crypto.PKey
- root_ca_cert: crypto.X509
+ root_ca_key: Certificate
+ root_ca_cert: RSAPrivateKey
Review Comment:
`root_ca_key` / `root_ca_cert` type annotations are swapped.
`make_self_signed_cert()` returns (Certificate, RSAPrivateKey) and `hooks.py`
assigns `context.root_ca_cert, context.root_ca_key` accordingly, so
`root_ca_cert` should be `Certificate` and `root_ca_key` should be
`RSAPrivateKey` to match actual runtime types.
```suggestion
root_ca_key: RSAPrivateKey
root_ca_cert: Certificate
```
##########
behave_framework/src/minifi_test_framework/core/ssl_utils.py:
##########
@@ -13,149 +13,79 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import datetime
-import time
-import logging
-import random
-
-from M2Crypto import X509, EVP, RSA, ASN1
-from OpenSSL import crypto
-
-
-def gen_cert():
- """
- Generate TLS certificate request for testing
- """
-
- req, key = gen_req()
- pub_key = req.get_pubkey()
- subject = req.get_subject()
- cert = X509.X509()
- # noinspection PyTypeChecker
- cert.set_serial_number(1)
- cert.set_version(2)
- cert.set_subject(subject)
- t = int(time.time())
- now = ASN1.ASN1_UTCTIME()
- now.set_time(t)
- now_plus_year = ASN1.ASN1_UTCTIME()
- now_plus_year.set_time(t + 60 * 60 * 24 * 365)
- cert.set_not_before(now)
- cert.set_not_after(now_plus_year)
- issuer = X509.X509_Name()
- issuer.C = 'US'
- issuer.CN = 'minifi-listen'
- cert.set_issuer(issuer)
- cert.set_pubkey(pub_key)
- cert.sign(key, 'sha256')
-
- return cert, key
-
-
-def rsa_gen_key_callback():
- pass
-
-
-def gen_req():
- """
- Generate TLS certificate request for testing
- """
-
- logging.info('Generating test certificate request')
- key = EVP.PKey()
- req = X509.Request()
- rsa = RSA.gen_key(1024, 65537, rsa_gen_key_callback)
- key.assign_rsa(rsa)
- req.set_pubkey(key)
- name = req.get_subject()
- name.C = 'US'
- name.CN = 'minifi-listen'
- req.sign(key, 'sha256')
-
- return req, key
+from cryptography import x509
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey
+from cryptography.x509 import Certificate, ExtendedKeyUsage
+from cryptography.x509.oid import NameOID
-def make_self_signed_cert(common_name):
- ca_key = crypto.PKey()
- ca_key.generate_key(crypto.TYPE_RSA, 2048)
+def gen_cert() -> tuple[Certificate, RSAPrivateKey]:
+ key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
- ca_cert = crypto.X509()
- ca_cert.set_version(2)
- ca_cert.set_serial_number(random.randint(50000000, 100000000))
+ subject = issuer = x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME,
u"US"), x509.NameAttribute(NameOID.COMMON_NAME, u"minifi-listen"), ])
- ca_subj = ca_cert.get_subject()
- ca_subj.commonName = common_name
+ cert =
x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key()).serial_number(
+
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.UTC)).not_valid_after(
+ datetime.datetime.now(datetime.UTC) +
datetime.timedelta(days=365)).sign(key, hashes.SHA256())
- ca_cert.add_extensions([
- crypto.X509Extension(b"subjectKeyIdentifier", False, b"hash",
subject=ca_cert),
- ])
-
- ca_cert.add_extensions([
- crypto.X509Extension(b"authorityKeyIdentifier", False,
b"keyid:always", issuer=ca_cert),
- ])
-
- ca_cert.add_extensions([
- crypto.X509Extension(b"basicConstraints", False, b"CA:TRUE"),
- crypto.X509Extension(b"keyUsage", False, b"keyCertSign, cRLSign"),
- ])
-
- ca_cert.set_issuer(ca_subj)
- ca_cert.set_pubkey(ca_key)
+ return cert, key
- ca_cert.gmtime_adj_notBefore(0)
- ca_cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60)
- ca_cert.sign(ca_key, 'sha256')
+def make_self_signed_cert(common_name: str) -> tuple[Certificate,
RSAPrivateKey]:
+ key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
- return ca_cert, ca_key
+ subject = issuer = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME,
common_name), ])
+ cert =
x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key()).serial_number(
+
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.UTC)).not_valid_after(
+ datetime.datetime.now(datetime.UTC) +
datetime.timedelta(days=3650)).add_extension(
+ x509.SubjectKeyIdentifier.from_public_key(key.public_key()),
critical=False, ).add_extension(x509.BasicConstraints(ca=True,
path_length=None),
+
critical=True, ).sign(key, hashes.SHA256())
-def _make_cert(common_name, ca_cert, ca_key, extended_key_usage=None):
- key = crypto.PKey()
- key.generate_key(crypto.TYPE_RSA, 2048)
+ return cert, key
- cert = crypto.X509()
- cert.set_version(2)
- cert.set_serial_number(random.randint(50000000, 100000000))
- client_subj = cert.get_subject()
- client_subj.commonName = common_name
+def _make_cert(common_name: str, ca_cert: Certificate, ca_key: RSAPrivateKey,
extended_key_usage: ExtendedKeyUsage | None) -> tuple[
+ Certificate, RSAPrivateKey]:
+ key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
- cert.add_extensions([
- crypto.X509Extension(b"basicConstraints", False, b"CA:FALSE"),
- crypto.X509Extension(b"subjectKeyIdentifier", False, b"hash",
subject=cert),
- ])
+ subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, common_name),
])
- extensions = [crypto.X509Extension(b"authorityKeyIdentifier", False,
b"keyid:always", issuer=ca_cert),
- crypto.X509Extension(b"keyUsage", False,
b"digitalSignature")]
+ builder =
x509.CertificateBuilder().subject_name(subject).issuer_name(ca_cert.subject).public_key(key.public_key()).serial_number(
+
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.UTC)).not_valid_after(
+ datetime.datetime.now(datetime.UTC) +
datetime.timedelta(days=3650)).add_extension(x509.BasicConstraints(ca=False,
path_length=None),
+
critical=True, ).add_extension(
+ x509.SubjectKeyIdentifier.from_public_key(key.public_key()),
critical=False, ).add_extension(
+ x509.SubjectAlternativeName([x509.DNSName(common_name)]),
critical=False, )
if extended_key_usage:
- extensions.append(crypto.X509Extension(b"extendedKeyUsage", False,
extended_key_usage))
-
- cert.add_extensions([
- crypto.X509Extension(b"subjectAltName", False, b"DNS.1:" +
common_name.encode())
- ])
+ builder =
builder.add_extension(x509.ExtendedKeyUsage(extended_key_usage), critical=False)
- cert.add_extensions(extensions)
+ cert = builder.sign(ca_key, hashes.SHA256())
+ return cert, key
- cert.set_issuer(ca_cert.get_subject())
- cert.set_pubkey(key)
- cert.gmtime_adj_notBefore(0)
- cert.gmtime_adj_notAfter(10 * 365 * 24 * 60 * 60)
+def make_client_cert(common_name: str, ca_cert: Certificate, ca_key:
RSAPrivateKey) -> tuple[Certificate, RSAPrivateKey]:
+ return _make_cert(common_name, ca_cert, ca_key,
x509.ExtendedKeyUsage([x509.OID_CLIENT_AUTH]))
- cert.sign(ca_key, 'sha256')
- return cert, key
+def make_server_cert(common_name: str, ca_cert: Certificate, ca_key:
RSAPrivateKey) -> tuple[Certificate, RSAPrivateKey]:
+ return _make_cert(common_name, ca_cert, ca_key,
x509.ExtendedKeyUsage(([x509.OID_SERVER_AUTH])))
Review Comment:
`x509.OID_CLIENT_AUTH` / `x509.OID_SERVER_AUTH` are not part of the public
`cryptography.x509` API. Use
`cryptography.x509.oid.ExtendedKeyUsageOID.CLIENT_AUTH` / `.SERVER_AUTH` (and
pass those OIDs into `x509.ExtendedKeyUsage([...])`) to avoid runtime attribute
errors.
##########
extensions/standard-processors/tests/features/steps/minifi_c2_server_container.py:
##########
@@ -17,32 +17,35 @@
import jks
import os
-from OpenSSL import crypto
-from cryptography.hazmat.primitives.serialization import pkcs12,
BestAvailableEncryption, load_pem_private_key
+
+from cryptography.hazmat.primitives import serialization
from cryptography import x509
from pathlib import Path
-from minifi_test_framework.containers.container import Container
+from cryptography.hazmat.primitives._serialization import
BestAvailableEncryption
+from cryptography.hazmat.primitives.serialization import load_pem_private_key,
pkcs12
Review Comment:
Importing `BestAvailableEncryption` from
`cryptography.hazmat.primitives._serialization` relies on a private module.
Please import it from the public API
(`cryptography.hazmat.primitives.serialization`) to avoid breakage on
cryptography upgrades.
##########
behave_framework/src/minifi_test_framework/containers/minifi_win_container.py:
##########
@@ -0,0 +1,59 @@
+from typing import Dict
+
+from minifi_test_framework.core.minifi_test_context import MinifiTestContext
+from minifi_test_framework.minifi.minifi_flow_definition import
MinifiFlowDefinition
+from minifi_test_framework.containers.directory import Directory
Review Comment:
New source file is missing the standard Apache license header block at the
top of the file. Please add the ASF license header, consistent with other files
in this repository.
##########
behave_framework/src/minifi_test_framework/core/ssl_utils.py:
##########
@@ -13,149 +13,79 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import datetime
-import time
-import logging
-import random
-
-from M2Crypto import X509, EVP, RSA, ASN1
-from OpenSSL import crypto
-
-
-def gen_cert():
- """
- Generate TLS certificate request for testing
- """
-
- req, key = gen_req()
- pub_key = req.get_pubkey()
- subject = req.get_subject()
- cert = X509.X509()
- # noinspection PyTypeChecker
- cert.set_serial_number(1)
- cert.set_version(2)
- cert.set_subject(subject)
- t = int(time.time())
- now = ASN1.ASN1_UTCTIME()
- now.set_time(t)
- now_plus_year = ASN1.ASN1_UTCTIME()
- now_plus_year.set_time(t + 60 * 60 * 24 * 365)
- cert.set_not_before(now)
- cert.set_not_after(now_plus_year)
- issuer = X509.X509_Name()
- issuer.C = 'US'
- issuer.CN = 'minifi-listen'
- cert.set_issuer(issuer)
- cert.set_pubkey(pub_key)
- cert.sign(key, 'sha256')
-
- return cert, key
-
-
-def rsa_gen_key_callback():
- pass
-
-
-def gen_req():
- """
- Generate TLS certificate request for testing
- """
-
- logging.info('Generating test certificate request')
- key = EVP.PKey()
- req = X509.Request()
- rsa = RSA.gen_key(1024, 65537, rsa_gen_key_callback)
- key.assign_rsa(rsa)
- req.set_pubkey(key)
- name = req.get_subject()
- name.C = 'US'
- name.CN = 'minifi-listen'
- req.sign(key, 'sha256')
-
- return req, key
+from cryptography import x509
+from cryptography.hazmat.primitives import hashes
+from cryptography.hazmat.primitives import serialization
+from cryptography.hazmat.primitives.asymmetric import rsa
+from cryptography.hazmat.primitives.asymmetric.rsa import RSAPrivateKey
+from cryptography.x509 import Certificate, ExtendedKeyUsage
+from cryptography.x509.oid import NameOID
-def make_self_signed_cert(common_name):
- ca_key = crypto.PKey()
- ca_key.generate_key(crypto.TYPE_RSA, 2048)
+def gen_cert() -> tuple[Certificate, RSAPrivateKey]:
+ key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
- ca_cert = crypto.X509()
- ca_cert.set_version(2)
- ca_cert.set_serial_number(random.randint(50000000, 100000000))
+ subject = issuer = x509.Name([x509.NameAttribute(NameOID.COUNTRY_NAME,
u"US"), x509.NameAttribute(NameOID.COMMON_NAME, u"minifi-listen"), ])
- ca_subj = ca_cert.get_subject()
- ca_subj.commonName = common_name
+ cert =
x509.CertificateBuilder().subject_name(subject).issuer_name(issuer).public_key(key.public_key()).serial_number(
+
x509.random_serial_number()).not_valid_before(datetime.datetime.now(datetime.UTC)).not_valid_after(
+ datetime.datetime.now(datetime.UTC) +
datetime.timedelta(days=365)).sign(key, hashes.SHA256())
Review Comment:
`datetime.UTC` is only available in newer Python versions (not in 3.10).
Since the framework requires Python >=3.10, this will raise `AttributeError` on
3.10; use `datetime.timezone.utc` (or `datetime.datetime.utcnow()` with
explicit tz) instead.
##########
behave_framework/src/minifi_test_framework/containers/minifi_controller.py:
##########
@@ -0,0 +1,80 @@
+import logging
+from minifi_test_framework.core.helpers import retry_check
+
+
+class MinifiController:
Review Comment:
New source file is missing the standard Apache license header block at the
top of the file. Please add the ASF license header, consistent with other files
in this repository.
##########
behave_framework/src/minifi_test_framework/containers/minifi_protocol.py:
##########
@@ -0,0 +1,40 @@
+from typing import Protocol
+
+
+class MinifiProtocol(Protocol):
+ def set_property(self, key: str, value: str):
Review Comment:
New source file is missing the standard Apache license header block at the
top of the file. Please add the ASF license header, consistent with other files
in this repository.
##########
docker/installed/win.Dockerfile:
##########
@@ -0,0 +1,24 @@
+#escape=`
+
+FROM mcr.microsoft.com/windows/servercore:ltsc2022
+
+LABEL maintainer="Apache NiFi <[email protected]>"
+
Review Comment:
New Dockerfile is missing the standard Apache license header block at the
top (other Dockerfiles under `docker/installed/` include it). Please add the
ASF license header for consistency/compliance.
##########
behave_framework/src/minifi_test_framework/containers/container_protocol.py:
##########
@@ -0,0 +1,47 @@
+from typing import Protocol, List
+
+from minifi_test_framework.containers.directory import Directory
+from minifi_test_framework.containers.file import File
+from minifi_test_framework.containers.host_file import HostFile
Review Comment:
New source file is missing the standard Apache license header block at the
top of the file. Please add the ASF license header, consistent with other files
in this repository.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]