RDK-61158: L3 CRL mTLS + cross-signed PKI test infrastructure - #153
Merged
shibu-kv merged 24 commits intoJul 30, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CRL-aware mutual-TLS (mTLS) and cross-signed PKI test infrastructure to the docker-device-mgt-service-test environment, enabling revocation-driven test scenarios via a dedicated mTLS endpoint plus a control plane for CRL updates/resets.
Changes:
- Introduces a new CRL-enforced mTLS server (50061) and a CRL control HTTP server (50062) in
mock-xconf. - Extends certificate generation/installation scripts to generate CRL-capable PKI material (including required CRLs for OpenSSL 3 CRL_CHECK_ALL) and distribute/install the needed trust anchors.
- Updates compose + tests to enable and validate the additional services/ports.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test_docker.py |
Updates mockxconf port expectations and Node process count for the new CRL servers. |
native-platform/certs.sh |
Copies CRL/xsign client assets from shared volume and installs CRL/XS trust anchors into the system trust store. |
mock-xconf/entrypoint.sh |
Conditionally starts the CRL mTLS server (which also starts the control server). |
mock-xconf/Dockerfile |
Adds CRL server/control scripts and exposes ports 50061/50062. |
mock-xconf/crl-mtls-server.js |
New HTTPS mTLS server with CRL enforcement and hot reload via setSecureContext(). |
mock-xconf/crl-control.js |
New HTTP control plane to revoke/reset and regenerate CRLs, triggering live TLS context reload. |
mock-xconf/certs.sh |
Generates a dedicated CRL PKI hierarchy + cross-signed test PKI and required CRLs, exporting to shared volume. |
compose.yaml |
Publishes ports 50061/50062 and enables ENABLE_CRL_L3=true in relevant services. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+65
to
+66
| assert node_process_count == 10, f"Expected 10 Node.js processes, but found {node_process_count}!" | ||
| print(f"\u2705 All 10 Node.js processes are running in {container.name}") |
Comment on lines
+231
to
+233
| touch "${CRL_ROOT_DIR}/index.txt" | ||
| printf "01\n" > "${CRL_ROOT_DIR}/crlnumber" | ||
| cat > "${CRL_ROOT_DIR}/openssl.cnf" << ROOTCAEOF |
| # Test-XS-NewRoot is needed so curl can verify xsign bridged cert chains. | ||
| _TRUST_DIR="/usr/share/ca-certificates" | ||
|
|
||
| # Extract Test-CRL-Root from the ICA chain (first cert block) |
Comment on lines
+65
to
+66
| assert node_process_count == 10, f"Expected 10 Node.js processes, but found {node_process_count}!" | ||
| print(f"\u2705 All 10 Node.js processes are running in {container.name}") |
Comment on lines
+13
to
+14
| # RDK-61158: Added ports 50061 (CRL mTLS) and 50062 (CRL control) | ||
| MOCKXCONF_EXPECTED_PORTS = [50050, 50051, 50052, 50053, 50054, 50055, 50056, 50057, 50058, 50059, 50060, 50061, 50062] # Example IPv6 ports for mockxconf |
Comment on lines
+151
to
+155
| # CA database files required by openssl ca | ||
| touch "${CRL_ICA_DIR}/index.txt" | ||
| printf "01\n" > "${CRL_ICA_DIR}/serial" | ||
| printf "01\n" > "${CRL_ICA_DIR}/crlnumber" | ||
|
|
Comment on lines
+18
to
+21
| - "50061:50061" # RDK-61158: CRL mTLS server | ||
| - "50062:50062" # RDK-61158: CRL control endpoint | ||
| - "50063:50063" # RDK-61158: OCSP responder | ||
| - "50064:50064" # RDK-61158: OCSP stapling server |
Comment on lines
+298
to
+305
| # Extract Test-CRL-Root from the ICA chain (first cert block) | ||
| awk '/-----BEGIN CERTIFICATE-----/{n++} n==2{print}' \ | ||
| /opt/certs/crl/crl-ica-chain.pem > "${_TRUST_DIR}/test-crl-root.pem" 2>/dev/null || true | ||
| # Fallback: the chain has ICA first, root second; use openssl to get the root | ||
| if [ ! -s "${_TRUST_DIR}/test-crl-root.pem" ]; then | ||
| # Just copy the whole chain as the trust anchor; curl will accept it | ||
| cp /opt/certs/crl/crl-ica-chain.pem "${_TRUST_DIR}/test-crl-root.pem" | ||
| fi |
Comment on lines
+50
to
+66
| assert node_process_count == 9, f"Expected 9 Node.js processes, but found {node_process_count}!" | ||
| print(f"✅ All 9 Node.js processes are running in {container.name}") | ||
| assert node_process_count == 10, f"Expected 10 Node.js processes, but found {node_process_count}!" | ||
| print(f"\u2705 All 10 Node.js processes are running in {container.name}") |
Comment on lines
+13
to
+14
| # RDK-61158: Added ports 50061 (CRL mTLS) and 50062 (CRL control) | ||
| MOCKXCONF_EXPECTED_PORTS = [50050, 50051, 50052, 50053, 50054, 50055, 50056, 50057, 50058, 50059, 50060, 50061, 50062] # Example IPv6 ports for mockxconf |
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
June 24, 2026 06:56
a18fd87 to
e90a1ef
Compare
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
June 26, 2026 17:55
e90a1ef to
e78e985
Compare
Comment on lines
+18
to
+21
| - "50061:50061" # CRL mTLS server | ||
| - "50062:50062" # CRL control endpoint | ||
| - "50063:50063" # OCSP responder | ||
| - "50064:50064" # OCSP stapling server |
Comment on lines
+50
to
+66
| assert node_process_count == 9, f"Expected 9 Node.js processes, but found {node_process_count}!" | ||
| print(f"✅ All 9 Node.js processes are running in {container.name}") | ||
| assert node_process_count == 10, f"Expected 10 Node.js processes, but found {node_process_count}!" | ||
| print(f"\u2705 All 10 Node.js processes are running in {container.name}") |
Comment on lines
+256
to
+265
| _TRUST_DIR="/usr/share/ca-certificates" | ||
| cp "${SHARED_CERTS_DIR}/crl-client/Test-CRL-Root.pem" "${_TRUST_DIR}/test-crl-root.pem" | ||
| grep -qxF "test-crl-root.pem" /etc/ca-certificates.conf 2>/dev/null || \ | ||
| echo "test-crl-root.pem" >> /etc/ca-certificates.conf | ||
|
|
||
| if [ -f "${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" ]; then | ||
| cp "${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" "${_TRUST_DIR}/test-xs-newroot.pem" | ||
| grep -qxF "test-xs-newroot.pem" /etc/ca-certificates.conf 2>/dev/null || \ | ||
| echo "test-xs-newroot.pem" >> /etc/ca-certificates.conf | ||
| fi |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+62
to
+66
| node_process_count = int(output.strip()) # Convert output to integer | ||
| print(f"Found {node_process_count} Node.js processes running in {container.name}") | ||
|
|
||
| assert node_process_count == 9, f"Expected 9 Node.js processes, but found {node_process_count}!" | ||
| print(f"✅ All 9 Node.js processes are running in {container.name}") | ||
| assert node_process_count == 10, f"Expected 10 Node.js processes, but found {node_process_count}!" | ||
| print(f"\u2705 All 10 Node.js processes are running in {container.name}") |
Comment on lines
+18
to
+21
| - "50061:50061" # CRL mTLS server | ||
| - "50062:50062" # CRL control endpoint | ||
| - "50063:50063" # OCSP responder | ||
| - "50064:50064" # OCSP stapling server |
Comment on lines
+277
to
+280
| while [ ! -f "${SHARED_CERTS_DIR}/xs-client/client-expxs.p12" ]; do | ||
| sleep 1 | ||
| echo "[certs] [CRL-L3] Waiting for ${SHARED_CERTS_DIR}/xs-client/client-expxs.p12..." | ||
| done |
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
July 1, 2026 10:08
eef69ec to
11ad9a2
Compare
| '-out', CRL_FILE, | ||
| '-crldays', '365', | ||
| '-batch', | ||
| ], { stdio: 'pipe' }); |
| '-revoke', certFile, | ||
| '-crl_reason', 'keyCompromise', | ||
| '-batch', | ||
| ], { stdio: 'pipe' }); |
| */ | ||
| function fetchOcspResponse() { | ||
| return new Promise((resolve) => { | ||
| const derFile = `/tmp/ocsp-staple-${Date.now()}.der`; |
Comment on lines
+144
to
+147
| } catch (e) { | ||
| console.error(`[ocsp-stapling-server] Failed to read OCSP DER file: ${e.message}`); | ||
| resolve(null); | ||
| } |
| - "50059:50059" # Crash metadata endpoint | ||
| - "50060:50060" # S3 presigned URL endpoint | ||
| - "50061:50061" # CRL mTLS server | ||
| - "50062:50062" # CRL control endpoint |
… and wire indexes - Remove docs/l3/ (standalone L3 doc structure) - Add docs/certificates/crl-mtls.md, cross-signed-pki.md, ocsp-stapling.md as flat peer pages alongside pkcs11.md and xpki-certifier.md - Wire new pages into docs/README.md and docs/certificates/README.md indexes - Add ENABLE_CRL_L3, CRL_L3_WAIT_TIMEOUT_SEC env vars and ports 50061-50064 to docs/certificates/configuration.md - Updated for fe857f5: 50062 bound to 127.0.0.1, server sends leaf+ICA chain
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
test_docker.py:76
cat /proc/net/tcp6is executed once per expected port. This makes the test slower and adds avoidable Docker exec overhead.
You can execute it once, decode once, then assert all expected ports against the same snapshot.
for port in expected_ports:
hex_port = format(port, '04x').upper() # Convert port number to uppercase hex (e.g., 50050 -> 'C382')
exit_code, output = container.exec_run("cat /proc/net/tcp6")
print(f"Checking port {port} (Hex: {hex_port}) inside mockxconf...")
assert exit_code == 0, f"Failed to check open ports in {container.name}!"
assert f":{hex_port}" in output.decode(), f"Port {port} is NOT open in {container.name}!"
mock-xconf/crl-control.js:142
readBody()destroys the request socket when the body exceedsMAX_BODY_BYTES, but the caller then tries to return a JSON413response. Destroying the socket here can prevent the 413 from being delivered (client will see a hangup) and can also lead to write errors onres.
Instead of req.destroy(), stop accumulating the body and allow the socket to be drained while rejecting so the handler can respond with 413.
size += Buffer.byteLength(chunk);
if (size > MAX_BODY_BYTES) {
const err = new Error('request body too large');
err.code = 'PAYLOAD_TOO_LARGE';
req.destroy();
native-platform/certs.sh:270
- The wait loop only gates on
crl-client.p12, but the script immediately copies several other CRL client artifacts afterward. Withset -e, if those files appear slightly later than the P12, the script can fail flakily.
Consider waiting for all required CRL client artifacts before proceeding (including Test-CRL-Root.pem, which is used later for trust-store install).
waited=0
while [ ! -f "${SHARED_CERTS_DIR}/crl-client/crl-client.p12" ]; do
if [ "${waited}" -ge "${CRL_L3_WAIT_TIMEOUT_SEC}" ]; then
echo "[certs] [CRL-L3] ERROR: Timed out after ${CRL_L3_WAIT_TIMEOUT_SEC}s waiting for ${SHARED_CERTS_DIR}/crl-client/crl-client.p12" >&2
exit 1
native-platform/certs.sh:301
- Similarly, this wait loop gates only on
xs-client/NewRoot.pem, but the script then copies three P12 bundles. If the producer writesNewRoot.pembefore all bundles are fully written, this can fail underset -e.
Consider waiting for the full set of XS artifacts you immediately consume.
waited=0
while [ ! -f "${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" ]; do
if [ "${waited}" -ge "${CRL_L3_WAIT_TIMEOUT_SEC}" ]; then
echo "[certs] [CRL-L3] ERROR: Timed out after ${CRL_L3_WAIT_TIMEOUT_SEC}s waiting for ${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" >&2
exit 1
docs/certificates/crl-mtls.md:9
- This overview says the CRL L3 flow "only matters when
ENABLE_MTLS=true", but the actual wiring (mock-xconfcerts.sh+entrypoint.sh, and native-platformcerts.sh) gates L3 PKI/server startup onENABLE_CRL_L3alone.
To avoid confusion, either (a) update the wording to reflect that L3 is gated solely by ENABLE_CRL_L3, or (b) enforce ENABLE_MTLS=true in the scripts before starting L3.
by `ENABLE_CRL_L3` (default `false`) and, like the baseline mTLS flow, only
matters when `ENABLE_MTLS=true`. When the flag is unset, none of the servers or
PKI below are created.
shibu-kv
previously approved these changes
Jul 29, 2026
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
July 30, 2026 12:11
1f91f4e to
9c8c2ab
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
native-platform/certs.sh:310
NewRoot.pemis waited on, but its copy into/opt/certs/xs/is conditional while later code unconditionally copies/opt/certs/xs/NewRoot.peminto the system trust store. If the conditional is ever skipped (or the copy fails), the trust-store step will fail with a missing file. Since the loop guarantees the file exists, copy it unconditionally here.
if [ -f "${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" ]; then
cp "${SHARED_CERTS_DIR}/xs-client/NewRoot.pem" /opt/certs/xs/NewRoot.pem
chmod 644 /opt/certs/xs/NewRoot.pem
fi
mock-xconf/crl-control.js:134
- When the request body exceeds
MAX_BODY_BYTES,readBody()callsreq.destroy()before the handler sends the 413 response. Destroying the request/socket can prevent the client from receiving the JSON error and typically surfaces as a connection reset. Prefer pausing/stopping the stream and letting the handler return a proper 413.
if (size > MAX_BODY_BYTES) {
const err = new Error('request body too large');
err.code = 'PAYLOAD_TOO_LARGE';
req.destroy();
reject(err);
test_docker.py:72
- In the IPv6 port assertion loop,
/proc/net/tcp6is fetched once per port, which adds unnecessary exec overhead and can slow/flakify CI. Read it once, then assert all expected ports against the same snapshot.
for port in expected_ports:
hex_port = format(port, '04x').upper() # Convert port number to uppercase hex (e.g., 50050 -> 'C382')
exit_code, output = container.exec_run("cat /proc/net/tcp6")
docs/certificates/crl-mtls.md:101
- This bullet implies the control endpoint is not reachable beyond the Docker host. Binding the published port to
127.0.0.1limits host access, but the service is still reachable from other containers on the Compose network because the container listens on its internal interface. Reword to avoid overstating network isolation.
- **Network isolation.** `compose.yaml` binds the 50062 control port to
`127.0.0.1` only, so it is not reachable from the wider network.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
test_docker.py:72
- The port check is currently grepping the entire
/proc/net/tcp6output for:{hex_port}. That substring can also appear in the remote address column, which can produce false positives (test passes even if the port is not actually listening). Restrict the command output to the local_address column so the assertion only checks listening sockets’ local ports.
for port in expected_ports:
hex_port = format(port, '04x').upper() # Convert port number to uppercase hex (e.g., 50050 -> 'C382')
exit_code, output = container.exec_run("cat /proc/net/tcp6")
native-platform/certs.sh:328
- This block calls
/usr/sbin/update-ca-certificates --freshwithout first verifying the binary exists/executable. Earlier in the script that check is done only when mock-xconf is resolvable, so withENABLE_CRL_L3=truethe container can now fail withupdate-ca-certificates: not foundin environments where the tool is missing.
chmod 644 "${SYSTEM_TRUST_STORE}/test-xs-newroot.pem"
grep -qxF "test-xs-newroot.pem" /etc/ca-certificates.conf 2>/dev/null || \
echo "test-xs-newroot.pem" >> /etc/ca-certificates.conf
/usr/sbin/update-ca-certificates --fresh
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
July 30, 2026 12:39
be89c7a to
091f1a5
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
mock-xconf/crl-control.js:142
readBody()destroys the request stream when the body exceedsMAX_BODY_BYTES. In Node this typically closes the underlying socket, so the handler may never be able to send the intended413JSON response (client will see a reset/empty response). Also, because the Promise is already rejected, subsequentdataevents would still append tobodyunless you guard against it.
if (size > MAX_BODY_BYTES) {
const err = new Error('request body too large');
err.code = 'PAYLOAD_TOO_LARGE';
req.destroy();
reject(err);
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
July 30, 2026 16:58
091f1a5 to
9f23641
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
native-platform/certs.sh:329
- The CRL-L3 block calls
/usr/sbin/update-ca-certificates --freshwithout the executable check that exists earlier in this script. Ifmock-xconfisn’t resolvable (so the earlier check is skipped) andENABLE_CRL_L3=true, this path can fail with a generic “command not found” instead of the clearer error used above. Consider adding the same guard here for consistent, actionable failures.
/usr/sbin/update-ca-certificates --fresh
echo "[certs] [CRL-L3] Test-CRL-Root and Test-XS-NewRoot installed in system trust store"
… (Coverity SIGMA fix)
Lasya-Prakarsha-D-V
force-pushed
the
topic/RDK-61158_old
branch
from
July 30, 2026 17:31
9f23641 to
0709982
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
mock-xconf/ocsp-stapling-server.js:97
- lastRefreshTime is written but never read, so it adds state without affecting behavior. Either use it (e.g., for staleness diagnostics) or remove it to keep the cache logic minimal.
let cachedOcspResponse = null; // Buffer (DER-encoded) or null
let lastRefreshTime = 0;
mock-xconf/ocsp-stapling-server.js:172
- After removing lastRefreshTime, this assignment becomes dead code and should be dropped.
if (response) {
cachedOcspResponse = response;
lastRefreshTime = Date.now();
}
mock-xconf/ocsp-stapling-server.js:221
- After removing lastRefreshTime, this assignment becomes dead code and should be dropped.
if (response) {
cachedOcspResponse = response;
lastRefreshTime = Date.now();
}
docs/certificates/crl-mtls.md:9
- The overview says this feature “only matters when ENABLE_MTLS=true”, but the implementation is gated by ENABLE_CRL_L3 alone (mock-xconf/certs.sh generates L3 PKI regardless of ENABLE_MTLS, and mock-xconf/entrypoint.sh starts the L3 servers based only on ENABLE_CRL_L3). This doc should match the actual gating to avoid confusing configuration/debugging.
by `ENABLE_CRL_L3` (default `false`) and, like the baseline mTLS flow, only
matters when `ENABLE_MTLS=true`. When the flag is unset, none of the servers or
PKI below are created.
test_docker.py:23
- This comment states the OCSP responder is “reached solely by the 50064 stapling server”, but the responder binds to 0.0.0.0 inside the container (see mock-xconf/entrypoint.sh) so it is still reachable from other containers on the Docker network; the main restriction is that it is not published to the host. Rewording avoids overstating the isolation guarantee.
# 50063 (openssl OCSP responder) is intentionally NOT listed: the openssl
# responder binds IPv4-only, so it never appears in /proc/net/tcp6, and it is an
# internal-only dependency reached solely by the 50064 stapling server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.