[
https://issues.apache.org/jira/browse/CAMEL-24429?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrea Cosentino updated CAMEL-24429:
-------------------------------------
Description:
ResponseMDN holds the AS2 security material in mutable instance fields and
overwrites them per request:
{code:java}
private AS2SignatureAlgorithm signingAlgorithm;
private Certificate[] signingCertificateChain;
private PrivateKey signingPrivateKey;
private PrivateKey decryptingPrivateKey;
private Certificate[] validateSigningCertificateChain;
private boolean keysAreDynamic = false;
private final Lock lock = new ReentrantLock();
...
if (this.keysAreDynamic) {
// Dynamically load path-specific security material from the HttpContext,
// which was populated by AS2ConsumerConfigInterceptor.
this.signingAlgorithm = (AS2SignatureAlgorithm)
context.getAttribute(AS2ServerConnection.AS2_SIGNING_ALGORITHM);
this.signingCertificateChain = (Certificate[])
context.getAttribute(...AS2_SIGNING_CERTIFICATE_CHAIN);
this.signingPrivateKey = (PrivateKey)
context.getAttribute(...AS2_SIGNING_PRIVATE_KEY);
this.decryptingPrivateKey = (PrivateKey)
context.getAttribute(...AS2_DECRYPTING_PRIVATE_KEY);
this.validateSigningCertificateChain = (Certificate[])
context.getAttribute(...AS2_VALIDATE_SIGNING_CERTIFICATE_CHAIN);
}
{code}
Three things combine here:
* The instance is registered once on the shared HttpProcessor, so a single
ResponseMDN serves every request.
* The assignment block above is not inside the class's lock.
* The assigned values persist after the request that set them, so they are
still in place when the next request begins.
A deployment hosting more than one partner on different paths, each with its
own keys, can therefore have one partner's MDN signed with another partner's
private key, or validated against the wrong chain. An MDN is the
non-repudiation record for the interchange, so signing it with the wrong key
undermines exactly the property it exists to carry.
Proposal: keep the dynamically loaded algorithm, keys and chains in local
variables for the duration of process(), or carry them through the
HttpCoreContext, rather than in instance fields. A regression test should drive
two requests with different key material through one connection and assert each
MDN is signed with its own path's key.
The context-reuse issue that was originally filed alongside this one has been
split out to CAMEL-24435 - the two share the theme of per-request state
outliving its request, but they are separate defects with separate fixes.
was:
Two places keep per-request state where it can be observed by a later or
concurrent request.
*ResponseMDN* holds the security material in mutable instance fields and
overwrites them per request:
{code:java}
private AS2SignatureAlgorithm signingAlgorithm;
private Certificate[] signingCertificateChain;
private PrivateKey signingPrivateKey;
private PrivateKey decryptingPrivateKey;
private Certificate[] validateSigningCertificateChain;
private boolean keysAreDynamic = false;
...
if (this.keysAreDynamic) {
this.signingAlgorithm = (AS2SignatureAlgorithm)
context.getAttribute(AS2ServerConnection.AS2_SIGNING_ALGORITHM);
this.signingCertificateChain = (Certificate[])
context.getAttribute(...AS2_SIGNING_CERTIFICATE_CHAIN);
this.signingPrivateKey = (PrivateKey)
context.getAttribute(...AS2_SIGNING_PRIVATE_KEY);
this.decryptingPrivateKey = (PrivateKey)
context.getAttribute(...AS2_DECRYPTING_PRIVATE_KEY);
this.validateSigningCertificateChain = (Certificate[])
context.getAttribute(...AS2_VALIDATE_SIGNING_CERTIFICATE_CHAIN);
}
{code}
The instance is registered once on the shared HttpProcessor, so it is shared
across requests. The assignment block is not inside the class's lock, and the
values persist after the request that set them. A deployment hosting two
partners on different paths with different keys can therefore have one
request's MDN signed with the other partner's key.
*AS2ServerConnection* creates the HttpContext once outside the request loop:
{code:java}
final HttpContext context = HttpCoreContext.create();
try {
while (!Thread.interrupted()) {
this.httpService.handleRequest(this.serverConnection, context);
...
String recipientAddress =
coreContext.getAttribute(AS2AsynchronousMDNManager.RECIPIENT_ADDRESS,
String.class);
{code}
RECIPIENT_ADDRESS and ASYNCHRONOUS_MDN set while handling one request are still
present on the next iteration, so a later request that does not request an
asynchronous MDN can trigger a resend to the previous request's address.
Proposal: keep the dynamically loaded keys and chains in local variables (or in
the HttpCoreContext) for the duration of process() rather than in instance
fields, and remove RECIPIENT_ADDRESS/ASYNCHRONOUS_MDN from the context after
the send block - or create a fresh context per request. A test that drives two
requests with different key material through one connection would cover both.
Summary: camel-as2 - ResponseMDN keeps per-request signing keys in
shared mutable instance fields (was: camel-as2 - per-request security material
outlives the request in ResponseMDN and the listener context)
> camel-as2 - ResponseMDN keeps per-request signing keys in shared mutable
> instance fields
> ----------------------------------------------------------------------------------------
>
> Key: CAMEL-24429
> URL: https://issues.apache.org/jira/browse/CAMEL-24429
> Project: Camel
> Issue Type: Bug
> Components: camel-as2
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.23.0
>
>
> ResponseMDN holds the AS2 security material in mutable instance fields and
> overwrites them per request:
> {code:java}
> private AS2SignatureAlgorithm signingAlgorithm;
> private Certificate[] signingCertificateChain;
> private PrivateKey signingPrivateKey;
> private PrivateKey decryptingPrivateKey;
> private Certificate[] validateSigningCertificateChain;
> private boolean keysAreDynamic = false;
> private final Lock lock = new ReentrantLock();
> ...
> if (this.keysAreDynamic) {
> // Dynamically load path-specific security material from the HttpContext,
> // which was populated by AS2ConsumerConfigInterceptor.
> this.signingAlgorithm = (AS2SignatureAlgorithm)
> context.getAttribute(AS2ServerConnection.AS2_SIGNING_ALGORITHM);
> this.signingCertificateChain = (Certificate[])
> context.getAttribute(...AS2_SIGNING_CERTIFICATE_CHAIN);
> this.signingPrivateKey = (PrivateKey)
> context.getAttribute(...AS2_SIGNING_PRIVATE_KEY);
> this.decryptingPrivateKey = (PrivateKey)
> context.getAttribute(...AS2_DECRYPTING_PRIVATE_KEY);
> this.validateSigningCertificateChain = (Certificate[])
> context.getAttribute(...AS2_VALIDATE_SIGNING_CERTIFICATE_CHAIN);
> }
> {code}
> Three things combine here:
> * The instance is registered once on the shared HttpProcessor, so a single
> ResponseMDN serves every request.
> * The assignment block above is not inside the class's lock.
> * The assigned values persist after the request that set them, so they are
> still in place when the next request begins.
> A deployment hosting more than one partner on different paths, each with its
> own keys, can therefore have one partner's MDN signed with another partner's
> private key, or validated against the wrong chain. An MDN is the
> non-repudiation record for the interchange, so signing it with the wrong key
> undermines exactly the property it exists to carry.
> Proposal: keep the dynamically loaded algorithm, keys and chains in local
> variables for the duration of process(), or carry them through the
> HttpCoreContext, rather than in instance fields. A regression test should
> drive two requests with different key material through one connection and
> assert each MDN is signed with its own path's key.
> The context-reuse issue that was originally filed alongside this one has been
> split out to CAMEL-24435 - the two share the theme of per-request state
> outliving its request, but they are separate defects with separate fixes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
