Print

Print


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 https://github.com/xrootd/xrootd/blob/7f2476ee739f38b34815a55acb1227aff1d4685f/src/XrdSecgsi/XrdSecProtocolgsi.cc#L1090-L1091 we are generating and setting the IV

- however in here: https://github.com/xrootd/xrootd/blob/7f2476ee739f38b34815a55acb1227aff1d4685f/src/XrdCrypto/XrdCryptosslCipher.cc#L984-L1012 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: https://github.com/xrootd/xrootd/blob/7f2476ee739f38b34815a55acb1227aff1d4685f/src/XrdCrypto/XrdCryptosslCipher.cc#L964-L981 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 or view it on GitHub:
https://github.com/xrootd/xrootd/pull/1019

########################################################################
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