User Identity Negotiation

[pim_philipse]

I found code for this purpose in the 20081001 snapshot.
The only problem is that the server is unable to send a response.
So please consider this submission:

Code: Select all

DCUSERID.H:

  /** Informs the server whether a positive response was requested.
   *  @return OFTrue if  a response was requested
   */
  OFBool isPosResponseRequested()
  {
	  return m_posRspRequested!=0? OFTrue: OFFalse;
  }


DCUSERID.CC:

void 
UserIdentityNegotiationSubItemAC::setServerResponse(const char* rsp,
													const Uint16& rspLen)
{
  if (m_serverRsp != NULL)
  {
    delete[] m_serverRsp;
    m_serverRsp = NULL;
  }
  m_rspLength = rspLen;
  if ((rspLen == 0) || (rsp == NULL))
    return;

  m_serverRsp = new char[rspLen];
  memcpy(m_serverRsp, rsp, rspLen);
}

ASSOC.H:

/** Acknowledges a User Identity Negotiation request.
 *  @param params - [in/out] The association parameters to be filled
 *  @param response  - [in]  The response to return (Kerberos or SAML) (will be copied)
 *  @param length - [in] Length of response
 *  @return EC_Normal if response could be set, error otherwise
 */
OFCondition ASC_setIdentAC(
    T_ASC_Parameters * params,
    const char* response,
    const Uint16& length );

ASSOC.CC:

OFCondition ASC_setIdentAC(
    T_ASC_Parameters * params,
    const char* response,
    const Uint16& length )
{
  if (params == NULL)
    return ASC_NULLKEY;
  UserIdentityNegotiationSubItemAC* ac = params->DULparams.ackUserIdentNeg;
  if (ac == NULL)
    ac = new UserIdentityNegotiationSubItemAC();
  else
    ac->clear();
  if( response!=NULL )
	ac->setServerResponse(response, length);
  params->DULparams.ackUserIdentNeg = ac;
  return EC_Normal;
}

In my SCP application, I'm now able to say

Code: Select all

if( userRQ->isPosResponseRequested() )
{
...
  ASC_setIdentAC(assoc->params, "ticket goes here", 17 );

and my SCU can retrieve the response with

Code: Select all

UserIdentityNegotiationSubItemAC *userAC;
ASC_getUserIdentAC(mAssoc->params,&userAC);
if( userAC!=NULL )
{
	char* resultBuf;
	Uint16 resultLen;
	userAC->getServerResponse(resultBuf,resultLen);
...

Have fun!

Pim.

[Michael Onken]

Hi Pim,

so actually somebody stumbled over this new feature When I implemented this, I only tested the feature with a client implementation and checked the debug output of the server (in that case storescp): If you look at storescu in the last snapshot(s), there are some new options in this SCU (the only one so far) for User Identity Negotiation support:

Code: Select all

  user identity negotiation:
    -usr  --user               [u]ser name: string
                               authenticate using user name u
    -pwd  --password           [p]assword: string (only with --user)
                               authenticate using password p
    -epw  --empty-password     send empty password (only with --user)
    -kt   --kerberos           [f]ilename: string
                               read kerberos ticket from file f
          --saml               [f]ilename: string
                               read SAML request from file f      

Because I did not have the time to implement that in a senseful way for storescp, I just missed to code the server side API Thanks for the hint - I will check and integrate your contribution as soon as time permits

Please note that I did not have any other implementation to test with, so one could say this feature is a bit experimental at the moment. If you can confirm User Identity Negotiation works fine with other implementation, we would be happy to hear that Maybe dcm4chee does in it's PACS implementation (as far as I know, User Identity Negotiation was planned), that would be good opportunity to test storescu...

Regards,
Michael

[pim_philipse]

Hi Michael,

dcm4chee can send the types 1 and 2.
In the following example it appears that their dcmsnd is satisfied with the transaction, and the SCP accepts the user name as well, so this looks promising.
If I hack the SCP to send a non-empty response on receiving a type 2 request, dcmsnd reports
UserIdentity[serverResponse(17)]
which is consistent with the amount that I put in the response.

Here is the complete association request (with empty response):

D:\dcm4che-2.0.15\bin>dcmsnd -username pim -passcode secret -uidnegrsp OnlineXS h:\scripts\00000012.dcm
Scanning files to send
.
Scanned 1 files in 0.094s (=94ms/file)
07:52:05,441 INFO - Association(1) initiated Socket[addr=/127.0.0.1,port=104,localport=4151]
07:52:05,456 INFO - OnlineXS(1) << A-ASSOCIATE-RQ[
calledAET = OnlineXS
callingAET = DCMSND
applicationContext = 1.2.840.10008.3.1.1.1 - DICOM Application Context Name
implClassUID = 1.2.40.0.13.1.1
implVersionName = dcm4che-2.0
maxPDULength = 16384
maxOpsInvoked/maxOpsPerformed = 0/0
UserIdentity[type = 2, username = pim, passcode = ******]
PresentationContext[id = 1, as = 1.2.840.10008.5.1.4.1.1.20 - Nuclear Medicine Image Storage
ts = 1.2.840.10008.1.2 - Implicit VR Little Endian
]
Role Selection(0):
Extended Negotiation(0):
Common Extended Negotiation(0):
]
07:52:05,472 INFO - OnlineXS(1) >> A-ASSOCIATE-AC[
calledAET = OnlineXS
callingAET = DCMSND
applicationContext = 1.2.840.10008.3.1.1.1 - DICOM Application Context Name
implClassUID = 1.2.528.1.1008.0.203
implVersionName = ROGAN_DICOM_203
maxPDULength = 65520
maxOpsInvoked/maxOpsPerformed = 2/1
UserIdentity[serverResponse(0)]
PresentationContext[id = 1, result = 0 - acceptance
ts = 1.2.840.10008.1.2 - Implicit VR Little Endian
]
Role Selection(0):
Extended Negotiation(0):
Common Extended Negotiation(0):
]

[Michael Onken]

Hi Pim,

great, thank you for posting...and sharing your experiences

Have a nice weekend,
Michael

[pim_philipse]

We did some analysis on the "experimental" code from the snapshot, and came up with the following suggestions for improvement:

1: memory leak in destructor, second field is not deleted

Code: Select all

// Destructor, frees memory of primary and secondary field
UserIdentityNegotiationSubItemRQ::~UserIdentityNegotiationSubItemRQ()
{
  if ((m_primField != NULL) && (m_primFieldLength > 0))
  {
    delete[] m_primField; m_primField = NULL;
  }
  m_primFieldLength = 0;
  if ((m_secField != NULL) && (m_secFieldLength > 0))
  {
    delete[] m_secField; m_secField = NULL;
  }
  m_secFieldLength = 0;
}

2: potential leak in assignment operators. Scenario:
UserIdentityNegotiationSubItemRQ rq1, rq2;
rq1.setPrimField("test1");
rq2.setPrimField("test2");
rq1 = rq2;
In this case, the primary field of rq1 is not deleted by operator=.
Suggested resolution: call clear() first.

Code: Select all

UserIdentityNegotiationSubItemRQ& UserIdentityNegotiationSubItemRQ::operator= (const UserIdentityNegotiationSubItemRQ& rhs)
{
  this->clear();
  m_userIdentityType = rhs.m_userIdentityType;
...
// Assignment operator
UserIdentityNegotiationSubItemAC& UserIdentityNegotiationSubItemAC::operator= (const UserIdentityNegotiationSubItemAC& rhs)
{
  this->clear();
  if (rhs.m_rspLength > 0)
...

Good luck,

Pim

[Michael Onken]

Thanks Pim, I will take your findings into account. If new code is available, I will send you a short hint to your email adress.

Regards,
MIchael

[Michael Onken]

Hi,

I integrated all posted fixes into the User Identity code. The new code is also part of the new snapshot which can be downloaded here as source code.

Thank you for your contribution

Regards,
Michael

[Yves Neumann]

Hello DCMTK-team,

I also 'stumbled' over this features. I am trying to extend my implementation of scp and scu with user identity negotiation and am reviewing what is available in the dcmtk (my code is based on the dcmtk).

I was comparing the implementation to the DICOM standard and noticed that there might be a flaw in the implementation of the 'UserIdentityNegotiationSubItemAC' (dcmnet/dcuderid.h). The class is derrived from 'UserIdentityNegotiationSubItem' and, therefore, defines the 'item type' with 0x58. Reading the standard it should be 0x59 for the 'A-ASSOCIATE-AC' and 0x58 for the 'A-ASSOCIATE-RQ' (UserIdentityNegotiationSubItemRQ).

I have not tested it yet (I am in an early stage of using this feature) but a fast check in the DCM4CHEE implementation seems to verify the issue and my understanding of the standard definition.

Regards, Yves

[J. Riesmeier]

Thank to for the report. I've just added this issue to our internal to-do/bug list with priority "high".

[Michael Onken]

This has finally* been fixed in DCMTK commit #54c8d8 (to show up on the public git in a few days).

*Considering the time, mildly contradicting its "high priority" status