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]

Reply via email to