Skip to content

Commit 44641a0

Browse files
authored
Develop (#58)
This pull request introduces several important improvements and bug fixes to the uPKI CA Server, focusing on robust Distinguished Name (DN) handling, enhanced certificate management APIs, and dependency updates. The changes ensure compatibility with both slash-separated and RFC 4514 comma-separated DN formats, fix certificate renewal bugs, and add comprehensive tests for these scenarios. Additionally, the pull request updates dependencies, removes the obsolete `setup.py`, and increments the project version. **Key changes:** ### DN Handling and Certificate Management * Refactored all certificate management methods in `Authority` (`revoke_certificate`, `unrevoke_certificate`, `renew_certificate`, `view_certificate`, and `delete_certificate`) to correctly extract and use the Common Name (CN) from both slash-separated and RFC 4514 comma-separated Distinguished Names, fixing bugs where certificates could not be found or renewed if the DN was not in the expected format. [[1]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773R590-R596) [[2]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773R653-R656) [[3]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773L699-R706) [[4]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773L709-R723) [[5]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773L775-R784) [[6]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773R876-R881) [[7]](diffhunk://#diff-55729ca46f7c8f84c983dc6ce4b678948d538c8771c7037a9db28c021844c773L826-L828) * Added a new `list_nodes` method to `Authority` to enumerate all certificates issued by the CA, including those not revoked or renewed, and expose their revocation status. ### Testing and Regression Coverage * Introduced comprehensive tests for DN parsing, certificate renewal, and the new `list_nodes` API, including regression tests for real-world bugs involving RFC 4514 DNs and renewal failures. [[1]](diffhunk://#diff-44ccbaf10c5a27481125e68ca0397fbf805936e44266cf4dedda6493515cbd4aR55-R67) [[2]](diffhunk://#diff-0c03354020754c0ffe5e1c4b723181f6bc057b2e6bb73af9a0a2071055085dccR1709-R1899) * Updated functional tests to handle variations in OpenSSL subject formatting across versions using regular expressions. [[1]](diffhunk://#diff-0c03354020754c0ffe5e1c4b723181f6bc057b2e6bb73af9a0a2071055085dccL198-R200) [[2]](diffhunk://#diff-0c03354020754c0ffe5e1c4b723181f6bc057b2e6bb73af9a0a2071055085dccL300-R308) [[3]](diffhunk://#diff-0c03354020754c0ffe5e1c4b723181f6bc057b2e6bb73af9a0a2071055085dccL532-R540) ### Dependency and Build System Updates * Updated Python dependencies to the latest compatible versions in `pyproject.toml`, including `cryptography`, `pyzmq`, `tinydb`, `pytest`, and `ruff`. * Set the Python version to 3.11.13 in `.python-version`. * Removed the obsolete `setup.py` file in favor of modern build tooling. ### Versioning * Bumped the project version to 0.1.5 in `upki_ca/__init__.py`. ### Minor * Minor import fix in tests to support new test cases.
2 parents 340f723 + 5f2bb0b commit 44641a0

11 files changed

Lines changed: 524 additions & 289 deletions

File tree

‎.python-version‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
3.11.13

‎poetry.lock‎

Lines changed: 194 additions & 197 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pyproject.toml‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,25 @@ license = {text = "MIT"}
99
readme = "README.md"
1010
requires-python = ">=3.11, <4.0"
1111
dependencies = [
12-
"cryptography (>=46.0.5,<47.0.0)",
12+
"cryptography (>=50.0.1,<51.0.0)",
1313
"pyyaml (>=6.0.3,<7.0.0)",
14-
"tinydb (>=4.8.2,<5.0.0)",
15-
"pyzmq (>=26.0.0,<27.0.0)"
14+
"tinydb (>=4.9.0,<5.0.0)",
15+
"pyzmq (>=27.0.0,<28.0.0)"
1616
]
1717

1818
[tool.poetry.group.lint]
1919
optional = true
2020

2121
[tool.poetry.group.lint.dependencies]
22-
ruff = ">=0.10.0,<1.0.0"
22+
ruff = ">=0.16.0,<1.0.0"
2323

2424
[tool.poetry.group.dev]
2525
optional = true
2626

2727
[tool.poetry.group.dev.dependencies]
28-
pytest = ">=8.0.0,<9.0.0"
29-
pytest-cov = ">=6.0.0,<7.0.0"
30-
pytest-asyncio = ">=0.25.0,<1.0.0"
28+
pytest = ">=9.0.0,<10.0.0"
29+
pytest-cov = ">=7.0.0,<8.0.0"
30+
pytest-asyncio = ">=1.0.0,<2.0.0"
3131

3232
[tool.pytest.ini_options]
3333
testpaths = ["tests"]

‎setup.py‎

Lines changed: 0 additions & 51 deletions
This file was deleted.

‎tests/test_00_common.py‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ def test_parse_dn_without_slashes(self):
5252
assert result["C"] == "FR"
5353
assert result["CN"] == "example.com"
5454

55+
def test_parse_dn_rfc4514_comma_separated(self):
56+
"""Test DN parsing for the comma-separated RFC 4514 format produced
57+
by cryptography's ``Name.rfc4514_string()`` and passed through as-is
58+
by the RA's inventory API (real bug found via a Phase 3 live smoke
59+
test: revoke/renew/view/unrevoke/delete all silently failed with
60+
"Certificate not found" for any real cert, since its DN always has
61+
more than just a CN and therefore never contains a "/")."""
62+
dn = "CN=example.com,O=Company"
63+
result = Common.parse_dn(dn)
64+
65+
assert result["CN"] == "example.com"
66+
assert result["O"] == "Company"
67+
5568
def test_build_dn(self):
5669
"""Test DN building."""
5770
components = {"C": "FR", "O": "Company", "CN": "example.com"}

‎tests/test_100_pki_functional.py‎

Lines changed: 198 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"""
1515

1616
import os
17+
import re
1718
import shutil
1819
import subprocess
1920
import sys
@@ -195,8 +196,8 @@ def test_generate_certificate_with_openssl(self):
195196
check=True,
196197
)
197198
assert "Certificate:" in result.stdout
198-
# Note: openssl outputs "CN = Test Entity" with spaces
199-
assert "CN =" in result.stdout and "Test Entity" in result.stdout
199+
# openssl subject format varies by version ("CN = X" vs "CN=X")
200+
assert re.search(r"CN\s*=\s*Test Entity", result.stdout)
200201

201202
# Verify certificate dates
202203
result = subprocess.run(
@@ -297,14 +298,14 @@ def test_validate_certificate_with_openssl(self):
297298
assert "Version:" in result.stdout
298299
assert "Serial Number:" in result.stdout
299300

300-
# Verify certificate subject - openssl outputs with spaces
301+
# Verify certificate subject - openssl subject format varies by version
301302
result = subprocess.run(
302303
["openssl", "x509", "-in", entity_cert, "-noout", "-subject"],
303304
capture_output=True,
304305
text=True,
305306
check=True,
306307
)
307-
assert "CN =" in result.stdout and "Test Entity" in result.stdout
308+
assert re.search(r"CN\s*=\s*Test Entity", result.stdout)
308309

309310
def test_certificate_chain_verification(self):
310311
"""
@@ -529,14 +530,14 @@ def test_certificate_creation_for_revocation(self):
529530
)
530531
assert "OK" in result.stdout
531532

532-
# Verify the certificate structure
533+
# Verify the certificate structure - openssl subject format varies by version
533534
result = subprocess.run(
534535
["openssl", "x509", "-in", test_cert, "-noout", "-subject"],
535536
capture_output=True,
536537
text=True,
537538
check=True,
538539
)
539-
assert "CN =" in result.stdout and "Revoke Test" in result.stdout
540+
assert re.search(r"CN\s*=\s*Revoke Test", result.stdout)
540541

541542

542543
# Run tests if executed directly
@@ -1705,6 +1706,197 @@ def test_start_uses_provided_env_seed(self):
17051706
assert written_seed == expected_seed
17061707

17071708

1709+
class TestListNodes:
1710+
"""Tests for `Authority.list_nodes()` (backs the CA's `list_nodes` ZMQ task)."""
1711+
1712+
@pytest.fixture(autouse=True)
1713+
def setup_teardown(self):
1714+
"""Set up and tear down for each test."""
1715+
self.pki_path = "/tmp/test_pki_list_nodes"
1716+
1717+
if os.path.exists(self.pki_path):
1718+
shutil.rmtree(self.pki_path)
1719+
1720+
from upki_ca.ca.authority import Authority
1721+
1722+
Authority.reset_instance()
1723+
1724+
yield
1725+
1726+
if os.path.exists(self.pki_path):
1727+
shutil.rmtree(self.pki_path)
1728+
Authority.reset_instance()
1729+
1730+
def _authority(self):
1731+
from upki_ca.ca.authority import Authority
1732+
from upki_ca.storage.file_storage import FileStorage
1733+
1734+
storage = FileStorage(self.pki_path)
1735+
storage.initialize()
1736+
authority = Authority.get_instance()
1737+
authority.initialize(storage=storage, keychain=self.pki_path)
1738+
return authority
1739+
1740+
def test_list_nodes_empty_when_nothing_issued(self):
1741+
authority = self._authority()
1742+
assert authority.list_nodes() == []
1743+
1744+
def test_list_nodes_includes_certificates_issued_via_generate_certificate(self):
1745+
"""Certificates issued via generate_certificate must be listed even if
1746+
never revoked/renewed (they never touch node storage otherwise)."""
1747+
authority = self._authority()
1748+
cert = authority.generate_certificate("node1.example.com", "server")
1749+
1750+
nodes = authority.list_nodes()
1751+
assert len(nodes) == 1
1752+
assert nodes[0]["cn"] == "node1.example.com"
1753+
assert nodes[0]["dn"] == "/CN=node1.example.com"
1754+
assert nodes[0]["serial"] == cert.serial_number
1755+
assert "BEGIN CERTIFICATE" in nodes[0]["certificate"]
1756+
assert nodes[0]["revoked"] is False
1757+
1758+
def test_list_nodes_includes_certificates_issued_via_sign_csr(self):
1759+
authority = self._authority()
1760+
1761+
from cryptography import x509
1762+
from cryptography.hazmat.primitives import hashes, serialization
1763+
from cryptography.hazmat.primitives.asymmetric import rsa
1764+
from cryptography.x509.oid import NameOID
1765+
1766+
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
1767+
csr = (
1768+
x509.CertificateSigningRequestBuilder()
1769+
.subject_name(
1770+
x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "csr-node.example.com")])
1771+
)
1772+
.sign(private_key, hashes.SHA256())
1773+
)
1774+
csr_pem = csr.public_bytes(serialization.Encoding.PEM).decode("utf-8")
1775+
1776+
authority.sign_csr(csr_pem, "server")
1777+
1778+
nodes = authority.list_nodes()
1779+
assert len(nodes) == 1
1780+
assert nodes[0]["cn"] == "csr-node.example.com"
1781+
1782+
def test_list_nodes_reflects_revoked_status(self):
1783+
authority = self._authority()
1784+
authority.generate_certificate("revoke-me.example.com", "server")
1785+
1786+
authority.revoke_certificate("/CN=revoke-me.example.com", "keyCompromise")
1787+
1788+
nodes = authority.list_nodes()
1789+
assert len(nodes) == 1
1790+
assert nodes[0]["revoked"] is True
1791+
1792+
def test_list_nodes_multiple_certificates(self):
1793+
authority = self._authority()
1794+
authority.generate_certificate("node1.example.com", "server")
1795+
authority.generate_certificate("node2.example.com", "server")
1796+
1797+
nodes = authority.list_nodes()
1798+
assert {n["cn"] for n in nodes} == {"node1.example.com", "node2.example.com"}
1799+
1800+
1801+
class TestRenewCertificateRFC4514DN:
1802+
"""Tests for `Authority.renew_certificate()` with a comma-separated,
1803+
multi-attribute RFC 4514 DN (e.g. "CN=x,O=y", as produced by
1804+
`cryptography`'s `Name.rfc4514_string()` and passed through as-is by the
1805+
RA's `/api/v1/inventory/certificates/{serial}/renew` endpoint - see
1806+
`upki_ra/utils/cert_metadata.py`).
1807+
1808+
Regression coverage for two real bugs found via a live Phase 3 frontend
1809+
smoke test (renew always failed for any real certificate):
1810+
1. `Common.parse_dn()` only understood the CA's native slash-separated
1811+
format (`/CN=x/O=y`); a comma-separated multi-attribute DN with no
1812+
slash at all was parsed into a single garbage key/value pair with no
1813+
"CN" key, so the pre-renewal `storage.get_cert()` lookup always
1814+
raised "Certificate not found".
1815+
2. Even past that, `renew_certificate()` extracted the *old*
1816+
certificate's own CN via a dict keyed by `attr.oid._name`
1817+
(cryptography's *long* name, e.g. "commonName") then looked up the
1818+
short form "CN" in it - a key that never existed, so it always
1819+
raised "Old certificate has no Common Name".
1820+
"""
1821+
1822+
@pytest.fixture(autouse=True)
1823+
def setup_teardown(self):
1824+
"""Set up and tear down for each test."""
1825+
self.pki_path = "/tmp/test_pki_renew_rfc4514"
1826+
1827+
if os.path.exists(self.pki_path):
1828+
shutil.rmtree(self.pki_path)
1829+
1830+
from upki_ca.ca.authority import Authority
1831+
1832+
Authority.reset_instance()
1833+
1834+
yield
1835+
1836+
if os.path.exists(self.pki_path):
1837+
shutil.rmtree(self.pki_path)
1838+
Authority.reset_instance()
1839+
1840+
def _authority(self):
1841+
from upki_ca.ca.authority import Authority
1842+
from upki_ca.storage.file_storage import FileStorage
1843+
1844+
storage = FileStorage(self.pki_path)
1845+
storage.initialize()
1846+
authority = Authority.get_instance()
1847+
authority.initialize(storage=storage, keychain=self.pki_path)
1848+
return authority
1849+
1850+
def _sign_csr_with_cn_and_o(self, authority, cn: str, org: str) -> None:
1851+
from cryptography import x509
1852+
from cryptography.hazmat.primitives import hashes, serialization
1853+
from cryptography.hazmat.primitives.asymmetric import rsa
1854+
from cryptography.x509.oid import NameOID
1855+
1856+
private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
1857+
csr = (
1858+
x509.CertificateSigningRequestBuilder()
1859+
.subject_name(
1860+
x509.Name(
1861+
[
1862+
x509.NameAttribute(NameOID.COMMON_NAME, cn),
1863+
x509.NameAttribute(NameOID.ORGANIZATION_NAME, org),
1864+
]
1865+
)
1866+
)
1867+
.sign(private_key, hashes.SHA256())
1868+
)
1869+
csr_pem = csr.public_bytes(serialization.Encoding.PEM).decode("utf-8")
1870+
authority.sign_csr(csr_pem, "server")
1871+
1872+
def test_renew_succeeds_with_comma_separated_multi_attribute_dn(self):
1873+
authority = self._authority()
1874+
self._sign_csr_with_cn_and_o(authority, "renew-me.example.com", "Test Corp")
1875+
1876+
# Same shape/order as `cert.subject.rfc4514_string()` for a
1877+
# CN-then-O subject (RFC 4514 lists attributes most-specific-first).
1878+
rfc4514_dn = "CN=renew-me.example.com,O=Test Corp"
1879+
1880+
new_cert, new_serial = authority.renew_certificate(rfc4514_dn)
1881+
1882+
assert new_cert.subject_cn == "renew-me.example.com"
1883+
assert new_serial != 0
1884+
1885+
def test_renew_succeeds_with_organization_first_dn(self):
1886+
"""Same as above but with O before CN in the DN string - this is the
1887+
exact ordering that triggered bug #1 above (the pre-fix parser
1888+
consumed the whole rest of the string as O's value, so no "CN" key
1889+
was ever produced)."""
1890+
authority = self._authority()
1891+
self._sign_csr_with_cn_and_o(authority, "renew-me2.example.com", "Test Corp")
1892+
1893+
rfc4514_dn = "O=Test Corp,CN=renew-me2.example.com"
1894+
1895+
new_cert, _ = authority.renew_certificate(rfc4514_dn)
1896+
1897+
assert new_cert.subject_cn == "renew-me2.example.com"
1898+
1899+
17081900
# Run tests if executed directly
17091901
if __name__ == "__main__":
17101902
pytest.main([__file__, "-v"])

‎upki_ca/__init__.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313
- Profiles: Certificate profile management
1414
- ZMQ connectors: CA-RA communication
1515
16-
Version: 0.1.0
16+
Version: 0.1.5
1717
"""
1818

19-
__version__ = "0.1.0"
19+
__version__ = "0.1.5"
2020
__author__ = "uPKI Team"
2121
__license__ = "MIT"
2222

0 commit comments

Comments
 (0)