Fixes following two problems:

1. XrdSecProtocolgsi::Encrypt returns wrong size of encrypted message if initialization vector is being used (the size of initialization vector which is being appended at the beginning of the output buffer is not taken into account)

2. When initialization vector is being generated and set the sessionKey is being passed its own iv pointer, as a result the initialization vector is first deleted and only then used to set IV. In more details:

• in here

  xrootd/src/XrdSecgsi/XrdSecProtocolgsi.cc

  Lines 1090 to 1091 in 7f2476e

  	iv = sessionKey->RefreshIV(liv);
  	sessionKey->SetIV(liv, iv);

  we are generating and setting the IV

• however in here:

  xrootd/src/XrdCrypto/XrdCryptosslCipher.cc

  Lines 984 to 1012 in 7f2476e

  	char *XrdCryptosslCipher::RefreshIV(int &l)
  	{
  	// Regenerate IV and return it
  	// Generate a new IV
  	GenerateIV();
  	// Set output
  	l = lIV;
  	return fIV;
  	}
  	//____________________________________________________________________________
  	void XrdCryptosslCipher::GenerateIV()
  	{
  	// Generate IV
  	// Cleanup existing one, if any
  	if (fIV) {
  	delete[] fIV;
  	fIV = 0;
  	lIV = 0;
  	}
  	// Generate a new one, using crypt-like chars
  	fIV = XrdSutRndm::GetBuffer(EVP_MAX_IV_LENGTH, 3);
  	if (fIV)
  	lIV = EVP_MAX_IV_LENGTH;
  	}

  we can see that XrdCryptosslCipher::RefreshIV already sets the internal pointer (fIV) to the new initialization vector and fIV is what is being returned

• subsequently we call XrdCryptosslCipher::SetIV with what has been returned from the previous call, hence fIV, in here:

  xrootd/src/XrdCrypto/XrdCryptosslCipher.cc

  Lines 964 to 981 in 7f2476e

  	void XrdCryptosslCipher::SetIV(int l, const char *iv)
  	{
  	// Set IV from l bytes at iv. If !iv, sets the IV length.
  	if (fIV) {
  	delete[] fIV;
  	fIV = 0;
  	lIV = 0;
  	}
  	if (l > 0) {
  	if (iv) {
  	fIV = new char[l];
  	if (fIV) memcpy(fIV,iv,l);
  	}
  	lIV = l;
  	}
  	}

  we can see the fIV will be first deleted and only then set to a new value, however since we called the routine with fIV itself the new value is fIV and we are using a dangling pointer to initialize fIV

• output from valgrind:

==9836== Invalid read of size 8
==9836==    at 0x4C2E060: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
==9836==    by 0xDFF7A29: XrdCryptosslCipher::SetIV(int, char const*) (XrdCryptosslCipher.cc:977)
==9836==    by 0xDBA82F7: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1091)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::Socket==9836== Invalid read of size 8
==9836==    at 0x4C2E060: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
==9836==    by 0xDFF7A29: XrdCryptosslCipher::SetIV(int, char const*) (XrdCryptosslCipher.cc:977)
==9836==    by 0xDBA82F7: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1091)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::SocketCallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)
==9836==    by 0x5393988: XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) (XrdSysIOEventsPollE.icc:225)
==9836==    by 0x53903AC: XrdSys::IOEvents::BootStrap::Start(void*) (XrdSysIOEvents.cc:131)
==9836==  Address 0x8d99760 is 0 bytes inside a block of size 16 free'd
==9836==    at 0x4C2B61D: operator delete[](void*) (vg_replace_malloc.c:621)
==9836==    by 0xDFF79EF: XrdCryptosslCipher::SetIV(int, char const*) (XrdCryptosslCipher.cc:969)
==9836==    by 0xDBA82F7: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1091)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::SocketCallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)
==9836==    by 0x5393988: XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) (XrdSysIOEventsPollE.icc:225)
==9836==    by 0x53903AC: XrdSys::IOEvents::BootStrap::Start(void*) (XrdSysIOEvents.cc:131)
==9836==  Block was alloc'd at
==9836==    at 0x4C2A888: operator new[](unsigned long) (vg_replace_malloc.c:423)
==9836==    by 0x53CB713: XrdSutRndm::GetBuffer(int, int) (XrdSutRndm.cc:201)
==9836==    by 0xDFF94CF: XrdCryptosslCipher::GenerateIV() (XrdCryptosslCipher.cc:1009)
==9836==    by 0xDFF9720: XrdCryptosslCipher::RefreshIV(int&) (XrdCryptosslCipher.cc:989)
==9836==    by 0xDBA82DD: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1090)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::SocketCallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)CallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)
==9836==    by 0x5393988: XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) (XrdSysIOEventsPollE.icc:225)
==9836==    by 0x53903AC: XrdSys::IOEvents::BootStrap::Start(void*) (XrdSysIOEvents.cc:131)
==9836==  Address 0x8d99760 is 0 bytes inside a block of size 16 free'd
==9836==    at 0x4C2B61D: operator delete[](void*) (vg_replace_malloc.c:621)
==9836==    by 0xDFF79EF: XrdCryptosslCipher::SetIV(int, char const*) (XrdCryptosslCipher.cc:969)
==9836==    by 0xDBA82F7: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1091)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::SocketCallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)
==9836==    by 0x5393988: XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) (XrdSysIOEventsPollE.icc:225)
==9836==    by 0x53903AC: XrdSys::IOEvents::BootStrap::Start(void*) (XrdSysIOEvents.cc:131)
==9836==  Block was alloc'd at
==9836==    at 0x4C2A888: operator new[](unsigned long) (vg_replace_malloc.c:423)
==9836==    by 0x53CB713: XrdSutRndm::GetBuffer(int, int) (XrdSutRndm.cc:201)
==9836==    by 0xDFF94CF: XrdCryptosslCipher::GenerateIV() (XrdCryptosslCipher.cc:1009)
==9836==    by 0xDFF9720: XrdCryptosslCipher::RefreshIV(int&) (XrdCryptosslCipher.cc:989)
==9836==    by 0xDBA82DD: XrdSecProtocolgsi::Encrypt(char const*, int, XrdSecBuffer**) (XrdSecProtocolgsi.cc:1090)
==9836==    by 0xE2114D6: XrdSecProtect::Secure(SecurityRequest*&, ClientRequest&, char const*) (XrdSecProtect.cc:284)
==9836==    by 0x4E97012: XrdCl::XRootDTransport::GetSignature(XrdCl::Message*, XrdCl::Message*&, XrdCl::AnyObject&) (XrdClXRootDTransport.cc:1276)
==9836==    by 0x4EF9DA6: XrdCl::AsyncSocketHandler::GetSignature(XrdCl::Message*, XrdCl::Message*&) (XrdClAsyncSocketHandler.cc:969)
==9836==    by 0x4EFA782: XrdCl::AsyncSocketHandler::OnWrite() (XrdClAsyncSocketHandler.cc:384)
==9836==    by 0x4E8CE96: (anonymous namespace)::SocketCallBack::Event(XrdSys::IOEvents::Channel*, void*, int) (XrdClPollerBuiltIn.cc:82)
==9836==    by 0x539265C: XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) (XrdSysIOEvents.cc:693)
==9836==    by 0x53937A8: XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) (XrdSysIOEventsPollE.icc:270)

You can view, comment on, or merge this pull request online at:

  https://github.com/xrootd/xrootd/pull/1019

Commit Summary

• [XrdSecgsi] XrdSecProtocolgsi::Encrypt - set IV correctly and report correct size.

File Changes

• M src/XrdSecgsi/XrdSecProtocolgsi.cc (7)

Patch Links:

• https://github.com/xrootd/xrootd/pull/1019.patch
• https://github.com/xrootd/xrootd/pull/1019.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Use REPLY-ALL to reply to list

To unsubscribe from the XROOTD-DEV list, click the following link:
https://listserv.slac.stanford.edu/cgi-bin/wa?SUBED1=XROOTD-DEV&A=1