Skip to content

Improve NETCONF session correctness, XML hardening, and compatibility docs#83

Merged
ydnath merged 8 commits intoJuniper:masterfrom
peterjhill:apr-2026
Apr 25, 2026
Merged

Improve NETCONF session correctness, XML hardening, and compatibility docs#83
ydnath merged 8 commits intoJuniper:masterfrom
peterjhill:apr-2026

Conversation

@peterjhill
Copy link
Copy Markdown
Contributor

@peterjhill peterjhill commented Apr 24, 2026

Summary

This PR improves protocol correctness, parser safety, session cleanup, and developer-facing documentation in netconf-java without introducing a broad API rewrite.

The focus is on making the existing client more robust under real production failure modes while keeping the current JSch-based transport model intact.

What changed

  • Hardened NETCONF XML parsing against XXE / DTD-based attacks

    • tightened parser configuration in the relevant XML parsing paths
    • added tests to prevent regression
  • Fixed NETCONF session framing and reply correlation

    • preserve or inject message-id on outbound RPCs
    • validate reply message-id on inbound rpc-reply
    • correctly handle both NETCONF 1.0 end-of-message framing and NETCONF 1.1 chunked framing
    • added regression coverage for sequential same-session RPC alignment
  • Fixed session/resource cleanup during connection failures

    • disconnect partially initialized SSH resources when NETCONF session setup fails
    • reduce the chance of leaking broken transport state after failed connect()
  • Fixed shell exec helper correctness

    • ensure exec channels are actually connected
    • ensure the running-shell helper sets the requested command
    • make helper cleanup/timeout behavior more predictable
  • Fixed nested XML path construction

    • corrected path-building behavior in XML helper logic
    • added regression coverage
  • Clarified session concurrency expectations

    • documented that a single NetconfSession is intended for sequential RPC use
    • explicitly do not claim support for multiple concurrent in-flight RPCs on one session
  • Added compatibility documentation

    • introduced docs/compatibility.md
    • documents current support for core NETCONF RFCs, capabilities, NMDA-related behavior, vendor-specific helpers, and known gaps/caveats
  • Updated test dependency for security

    • upgraded assertj-core to 3.27.7 to address CVE-2026-24400
  • shared NETCONF base-capability negotiation with framing derived from the negotiated base

  • capability-gated candidate, validate, and confirmed-commit operations

  • Device.getNegotiatedCapabilities() and NetconfSession.getNegotiatedCapabilities()

  • structured RpcErrorException / ValidateException handling, including namespaced rpc-error parsing

  • sanitized integration runner/docs path cleanup

  • Bumped project version to 2.2.1.0

Verification

  • mvn test
  • ./gradlew test
  • ./gradlew integrationTest
  • ./run-integration-tests.sh

Notes / non-goals

This PR improves RFC 6241 / 6242 behavior materially, but it does not claim full standards compliance yet.

Known follow-up areas include:

  • notification / subscription support (RFC 5277)
  • clearer transport abstraction if we later want pluggable transports while preserving FIPS-friendly deployment options

Peter Hill added 6 commits April 24, 2026 11:01
Configure secure XML parser settings for device sessions and parsed NETCONF elements to disable external entities, DTD loading, and external schema access.

Also reject DOCTYPE declarations in rpc-reply parsing and cover the behavior with a focused reply parsing test.
Switch session framing based on the peer hello, decode chunked NETCONF 1.1 replies, and return framed payloads from streaming RPC reads instead of raw transport markers.

Also preserve caller-supplied rpc message-ids, generate missing message-ids for raw rpc envelopes, validate rpc-reply correlation, and ensure commitThisConfiguration always unlocks the candidate datastore on failure.
Make XML.addPath build each segment under the previously created node instead of attaching every segment directly to the original active element.

Add a regression test for multi-segment paths so nested hierarchies are preserved correctly.
- clean up partial SSH state when NETCONF session creation fails
- connect and release exec channels deterministically
- enforce commandTimeout for shell helper reads
- document the recommended Device lifecycle in the README
- verify back-to-back executeRPC calls stay aligned with NETCONF 1.0 framing
- verify back-to-back executeRPC calls stay aligned with NETCONF 1.1 chunked framing
- document the issue scope as sequential session reuse rather than concurrent RPC support
- clarify that a NetconfSession is intended for sequential RPC use
- state that concurrent in-flight RPCs on the same session are not guaranteed safe
- recommend separate sessions when applications need concurrency
Add a maintainer-facing compatibility matrix covering NETCONF RFC support, capability caveats, NMDA coverage, notifications gaps, and Junos-specific helpers.

Also align Maven and Gradle on assertj-core 3.27.7 to address CVE-2026-24400 in AssertJ's XML pretty-printing path while keeping the existing XMLUnit-based test style.

Bump  version to 2.2.1.0
Teach sessions to negotiate the shared NETCONF base capability and gate operations against what the server actually advertises.

Add structured RpcErrorException and ValidateException flows, harden rpc-error parsing for namespaced replies, and update commit/load/validate behavior to preserve server detail.

Back the change with unit and integration coverage, Javadocs for the new public API, and a sanitized integration runner/docs path story.
@peterjhill
Copy link
Copy Markdown
Contributor Author

Issue coverage note for reviewers:

This PR appears to address the following existing issues:

Fully addresses #81 by negotiating the shared NETCONF base capability instead of assuming NETCONF 1.1, which restores proper NETCONF 1.0 behavior when that is the common denominator.
Fully addresses the sequential session-reuse failure described in #68 by tightening RPC/reply alignment, preserving message-id correlation, and adding tests for multiple RPCs on the same session.
Partially addresses #14 by improving RFC 6241 / 6242 behavior substantially, including framing negotiation and capability-aware operation handling, but does not claim complete protocol coverage in all cases yet.
I wanted to call that out explicitly so issue impact is easier to track from this PR.

@peterjhill
Copy link
Copy Markdown
Contributor Author

Also, I purposely did not squash commits. Each one is meant to contain specific fixes. Or a smaller focused set of fixes.

@ydnath ydnath merged commit ee712d5 into Juniper:master Apr 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants