Replace Python Cryptography with OpenSSL CLI for certificate generation by devin-ai-integration[bot] · Pull Request #16 · Autonomy-Logic/openplc-runtime

devin-ai-integration · 2025-10-09T01:30:03Z

Replace Python Cryptography with OpenSSL CLI for certificate generation

Summary

This PR migrates the certificate generation system from the Python cryptography library to OpenSSL CLI commands, following the same approach implemented in OpenPLC_v3 PR #4. The change includes:

Complete rewrite of webserver/credentials.py to use subprocess calls to OpenSSL instead of the cryptography library
Enhanced security: Upgraded from RSA 2048-bit to 4096-bit keys with 36500-day (~100 years) certificate validity
Platform detection: Added logic to run HTTPS on Linux systems and HTTP on other platforms for compatibility
Dependency cleanup: Removed cryptography from requirements.txt
Maintained API compatibility: The CertGen class interface remains unchanged to ensure existing code continues to work

Review & Testing Checklist for Human

⚠️ HIGH PRIORITY - This touches security-critical certificate generation functionality:

Test certificate generation on Linux: Verify that certificates are generated correctly with 4096-bit RSA keys and proper Subject Alternative Names
Test HTTP mode on non-Linux platforms: Confirm that the application starts and runs on HTTP port 8080 on Windows/Mac systems
Verify certificate validation: Test that existing certificates are properly validated and expired certificates trigger regeneration
Security review: Examine the OpenSSL command construction for potential command injection vulnerabilities with hostname/IP parameters
End-to-end functionality: Test the complete web interface workflow including file uploads and PLC operations on both HTTP and HTTPS modes

Notes

This implements the same changes as OpenPLC_v3 PR #4 (commits 1b82973, b3a1e65)
OpenSSL must be available on all target systems (should be standard on Linux)
The platform detection uses platform.system() == "Linux" to determine HTTPS vs HTTP mode
Certificate validity increased from 365 days to ~100 years for reduced maintenance

Requested by: Thiago Alves (@thiagoralves)
Devin session: https://app.devin.ai/sessions/7734798dc74e4823ab03bc8402ba6cfa

@thiagoralves

This change migrates the certificate generation from the Python Cryptography library to OpenSSL CLI commands, following the same approach as PR #4 in the OpenPLC_v3 repository. Key changes: - Replaced credentials.py to use subprocess calls to OpenSSL instead of cryptography library - Upgraded from RSA 2048-bit to 4096-bit keys for enhanced security - Increased certificate validity from 365 days to 36500 days (~100 years) - Removed cryptography dependency from requirements.txt - Added platform detection to app.py: HTTPS on Linux, HTTP on other platforms - Maintained same CertGen class interface for backward compatibility Benefits: - OpenSSL is universally available on Linux systems - No complex Python library dependencies - Stronger security with 4096-bit keys - Cross-platform compatibility with HTTP fallback for non-Linux systems Implements the same changes as: - OpenPLC_v3 PR #4: Autonomy-Logic/OpenPLC_v3#4 - Commits: 1b82973, b3a1e65 Requested by: Thiago Alves (@thiagoralves) Devin run: https://app.devin.ai/sessions/7734798dc74e4823ab03bc8402ba6cfa Co-Authored-By: Thiago Alves <thiagoralves@gmail.com>

devin-ai-integration · 2025-10-09T01:30:05Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

⚙️ Control Options:

Disable automatic comment and CI monitoring

Copilot

Pull Request Overview

This PR migrates the certificate generation system from the Python cryptography library to OpenSSL CLI commands, enhancing security and platform compatibility.

Complete rewrite of certificate generation using OpenSSL subprocess calls instead of Python cryptography library
Enhanced security with RSA 4096-bit keys and ~100-year certificate validity
Added platform-specific HTTPS/HTTP handling based on operating system

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File	Description
webserver/credentials.py	Complete rewrite to use OpenSSL CLI commands for certificate generation and validation
webserver/app.py	Added platform detection logic and HTTP mode for non-Linux systems

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-09T02:29:38Z

+            "-days",
+            "36500",
+            "-subj",
+            f"/CN={self.hostname}",


The hostname is directly interpolated into the OpenSSL command without validation or escaping. This could allow command injection if the hostname contains shell metacharacters. Consider validating the hostname format or using shell escaping.

Copilot · 2025-10-09T02:29:38Z

+            "-subj",
+            f"/CN={self.hostname}",
+            "-addext",
+            f"subjectAltName={san_string}",


The SAN string contains unvalidated IP addresses and hostnames that could contain shell metacharacters. This poses a command injection risk. Consider validating IP addresses and hostnames before constructing the command.

Copilot

Pull Request Overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-09T16:17:08Z

+
+        san_list = [f"DNS:{self.hostname}"]
+        for ip in self.ip_addresses:
+            san_list.append(f"IP:{ip}")


IP addresses from self.ip_addresses are directly interpolated into the OpenSSL command without validation. This could allow command injection if malicious IP values are provided. Consider validating IP addresses or using a safer method to construct the SAN extension.

Copilot · 2025-10-09T16:17:08Z

+
+        # Check if the certificate is valid
+        if not cert_gen.is_certificate_valid(CERT_FILE):
+            # logger.error("Invalid certificate. Cannot start https application")
+            print("Invalid certificate. Cannot start https application") # TODO: remove this temporary print once logger is functional again
+            sys.exit(1)



The certificate validation logic is flawed. The code generates a certificate when files don't exist, but then immediately checks if the certificate is valid and exits if it's not. This could cause the application to exit even after successfully generating a new certificate. The validation check should be moved into an elif block or the logic should be restructured.

Suggested change

# Check if the certificate is valid

if not cert_gen.is_certificate_valid(CERT_FILE):

# logger.error("Invalid certificate. Cannot start https application")

print("Invalid certificate. Cannot start https application") # TODO: remove this temporary print once logger is functional again

sys.exit(1)

else:

# Check if the certificate is valid

if not cert_gen.is_certificate_valid(CERT_FILE):

# logger.error("Invalid certificate. Cannot start https application")

print("Invalid certificate. Cannot start https application") # TODO: remove this temporary print once logger is functional again

sys.exit(1)

thiagoralves requested a review from Copilot October 9, 2025 02:28

Copilot AI reviewed Oct 9, 2025

View reviewed changes

Fix certificate logic in main app. Removed http-only implementation

b75f1fe

thiagoralves requested a review from Copilot October 9, 2025 16:16

Copilot AI reviewed Oct 9, 2025

View reviewed changes

thiagoralves merged commit f768d21 into development Oct 9, 2025

thiagoralves deleted the devin/1728420279-replace-cryptography-with-openssl branch October 9, 2025 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Python Cryptography with OpenSSL CLI for certificate generation#16

Replace Python Cryptography with OpenSSL CLI for certificate generation#16
thiagoralves merged 2 commits into
developmentfrom
devin/1728420279-replace-cryptography-with-openssl

devin-ai-integration Bot commented Oct 9, 2025

Uh oh!

devin-ai-integration Bot commented Oct 9, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Oct 9, 2025

Uh oh!

Copilot AI Oct 9, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 9, 2025

Uh oh!

Copilot AI Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

devin-ai-integration Bot commented Oct 9, 2025

Replace Python Cryptography with OpenSSL CLI for certificate generation

Summary

Review & Testing Checklist for Human

Notes

Uh oh!

devin-ai-integration Bot commented Oct 9, 2025

🤖 Devin AI Engineer

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants