Possible issue: incorrect decoding of DCE/RPC lookup request #555
Description
I'm currently having an issue where a Proneta scan against my device running this stack returns no IM0 record data.
In other words, this section is missing from Proneta:
After investigating this a bit, the particular implementation of pf_cmrdr_inquiry_read_all_reg_ind seems a bit bizarre to me. I'm not super familiar with the Profinet specification, so I might just be missing something here.
The following here is a Wireshark capture of a request made from Proneta to the IO-device. As highlighted here, Proneta chooses a somewhat random sequence number (in this case 370):
Taking a look at pf_cmrdr_inquiry_read_all_reg_ind, a switch is performed off of the p_rpc_req->sequence_nmb field:
switch (p_rpc_req->sequence_nmb)
{
case 0:
{
// Handle EPM interface
// ...
}
break;
case 1:
{
// Handle PNIO
// ....According to the specification, the sequence number is used to uniquely identify an RPC call. From my understanding, the exact value has no further purpose than to distinguish multiple RPC calls.
To me it seems like the sequence number should not be checked against the fixed values of 0 and 1. Looking at the packet, I can't determine any other way to handle the lookup requests properly than to keep the p_session_info_t alive instead of killing it after the first request. That way a sequence number can just be handled internally and incremented for the session.
I ending up adding the fix for this in, I'm happy to make a pull request.
typedef struct pf_session_info {
+ uint32_t lookup_sequence_nmb;
}This sequence number is set to 0 when the session is created and incremented once every time a packet is handled.
static int pf_cmrdr_inquiry_read_all_reg_ind (
pnet_t * net,
pf_session_info_t * p_sess,
const pf_rpc_header_t * p_rpc_req,
const pf_rpc_lookup_req_t * p_lookup_req,
pf_rpc_lookup_rsp_t * p_lookup_rsp,
pnet_result_t * p_read_status)
{
int ret = -1;
- switch(p_rpc_req->sequence_nmb)
+ switch(p_sess->lookup_sequence_nmb)
{
case 0:
{
/* Check if handle is all NULL */
if(
memcmp(
&p_lookup_req->rpc_handle,
&null_rpc_handle,
sizeof(null_rpc_handle)) == 0)
{
/* Send the EPM interface information */
LOG_INFO (
PF_RPC_LOG,
"EPM(%d): PLC is requesting EPM interface information.\n",
__LINE__);
ret = pf_cmrdr_add_epmv4_entry(
net,
p_rpc_req,
p_lookup_req,
p_lookup_rsp,
p_read_status);
}
}
break;
case 1:
{
/* Check if handle is NOT NULL */
if(
memcmp(
&p_lookup_req->rpc_handle,
&null_rpc_handle,
sizeof(null_rpc_handle)) != 0)
{
/* Send the PNIO interface information */
LOG_INFO (
PF_RPC_LOG,
"EPM(%d): PLC is requesting PNIO interface information via EPM.\n",
__LINE__);
ret = pf_cmrdr_add_pnio_entry(
net,
p_rpc_req,
p_lookup_req,
p_lookup_rsp,
p_read_status);
}
}
break;
default:
+ p_sess->kill_session = true;
}
return ret;
}// pf_cmrpc_ce_packet
/* Handle Endpoint Map request */
ret = pf_cmrpc_lookup_ind (
net,
p_sess,
&rpc_req,
req_pos,
max_rsp_len,
p_sess->out_buffer,
&p_sess->out_buf_len);
*p_close_socket = p_sess->from_me;
+ p_sess->lookup_sequence_nmb++;
/* Close session after each EPM request
If future more advanced EPM usage is required, implement a
timeout for closing the session */
- p_sess->kill_session = true;Per the above comment, this approach also requires a timeout for closing the session. Any ideas on this are welcome.