Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rebased version of PR#1493 #1561

Merged
merged 5 commits into from
Nov 29, 2021
Merged

Rebased version of PR#1493 #1561

merged 5 commits into from
Nov 29, 2021

Conversation

gganis
Copy link
Member

@gganis gganis commented Nov 24, 2021

Adding support for pure cert/key authentication.
Client controls this mode via XrdSecGSICREATEPROXY:

  1. If XrdSecGSICREATEPROXY=1 (default), a proxy is auto-generated from the cert/key pair if one is not found.
  2. If XrdSecGSICREATEPROXY=0, a proxy is used if present. Otherwise, the cert/key pair is used if present (no proxy).
    This is mostly meant, on the server side, for pass-less authentication, possible when the key file is pass-less.
    NB1: if the key-file is pass-less and XrdSecGSICREATEPROXY = 1 (default) authentication still works with the usual protocol, i.e. creating a proxy and using that for the handshake. Setting XrdSecGSICREATEPROXY = 0 avoids those additional steps.
    NB2: Forward / backward compatibility is obtained by enabling the cert/pair mechanism only for versions supporting it

Client enforces this mode by setting XrdSecGISNOPROXY=1.
Key file must be pass-less.
Servers check the number of certificates in received chain.
Pass-less authentication can still happen but requires the creation of the
proxy.
@simonmichal
Copy link
Contributor

Hi @gganis,
thanks a lot for the rebased PR :-)

I gave it try by running a server and client built with this PR and setting XrdSecGSICREATEPROXY=0 and I run into a double-free error, here's the output from asan:

[simonm@idefix XrdCl]$ ./xrdfs slc7-test ls /tmp
=================================================================
==32039==ERROR: AddressSanitizer: attempting double-free on 0x616000015080 in thread T1:
    #0 0x7fbd64749508 in __interceptor_free (/lib64/libasan.so.4+0xde508)
    #1 0x7fbd62791246 in _IO_new_fclose (/lib64/libc.so.6+0x6e246)
    #2 0x7fbd64748b80 in __interceptor_fclose (/lib64/libasan.so.4+0xddb80)
    #3 0x7fbd63602641 in XrdCryptosslX509ParseFile(char const*, XrdCryptoX509Chain*, char const*) /home/simonm/xrootd-pr1561/src/XrdCrypto/XrdCryptosslAux.cc:469
    #4 0x7fbd58769af0 in XrdSecProtocolgsi::QueryProxy(bool, XrdSutCache*, char const*, XrdCryptoFactory*, long, ProxyIn_t*, ProxyOut_t*) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:5126
    #5 0x7fbd5878be69 in XrdSecProtocolgsi::ClientDoInit(XrdSutBuffer*, XrdSutBuffer**, XrdOucString&) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:3075
    #6 0x7fbd5878c70d in XrdSecProtocolgsi::ParseClientInput(XrdSutBuffer*, XrdSutBuffer**, XrdOucString&) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:2942
    #7 0x7fbd5878d1e1 in XrdSecProtocolgsi::getCredentials(XrdSecBuffer*, XrdOucErrInfo*) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:1507
    #8 0x7fbd6401efa4 in XrdCl::XRootDTransport::GetCredentials(XrdSecBuffer*&, XrdCl::HandShakeData*, XrdCl::XRootDChannelInfo*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:2506
    #9 0x7fbd64029d12 in XrdCl::XRootDTransport::DoAuthentication(XrdCl::HandShakeData*, XrdCl::XRootDChannelInfo*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:2259
    #10 0x7fbd6402eb8b in XrdCl::XRootDTransport::HandShakeMain(XrdCl::HandShakeData*, XrdCl::AnyObject&) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:516
    #11 0x7fbd6402fc69 in XrdCl::XRootDTransport::HandShake(XrdCl::HandShakeData*, XrdCl::AnyObject&) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:416
    #12 0x7fbd6422b2f5 in XrdCl::AsyncSocketHandler::HandleHandShake(std::unique_ptr<XrdCl::Message, std::default_delete<XrdCl::Message> >) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:539
    #13 0x7fbd6422d38c in XrdCl::AsyncSocketHandler::OnReadWhileHandshaking() /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:527
    #14 0x7fbd642333fc in XrdCl::AsyncSocketHandler::Event(unsigned char, XrdCl::Socket*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:227
    #15 0x7fbd63fc510d in Event /home/simonm/xrootd-pr1561/src/XrdCl/XrdClPollerBuiltIn.cc:83
    #16 0x7fbd635cc497 in XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysIOEvents.cc:695
    #17 0x7fbd635d00ec in XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) /home/simonm/xrootd-pr1561/src/./XrdSys/XrdSysIOEventsPollE.icc:277
    #18 0x7fbd635d06a6 in XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) /home/simonm/xrootd-pr1561/src/./XrdSys/XrdSysIOEventsPollE.icc:232
    #19 0x7fbd635c608f in XrdSys::IOEvents::BootStrap::Start(void*) /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysIOEvents.cc:133
    #20 0x7fbd635dece9 in XrdSysThread_Xeq /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysPthread.cc:86
    #21 0x7fbd62af8ea4 in start_thread (/lib64/libpthread.so.0+0x7ea4)
    #22 0x7fbd62821b0c in clone (/lib64/libc.so.6+0xfeb0c)

0x616000015080 is located 0 bytes inside of 568-byte region [0x616000015080,0x6160000152b8)
freed by thread T1 here:
==32039==AddressSanitizer CHECK failed: ../../../../libsanitizer/asan/asan_descriptions.cc:174 "((id)) != (0)" (0x0, 0x0)
    #0 0x7fbd64754952  (/lib64/libasan.so.4+0xe9952)
    #1 0x7fbd647732e5 in __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) (/lib64/libasan.so.4+0x1082e5)
    #2 0x7fbd6469819c  (/lib64/libasan.so.4+0x2d19c)
    #3 0x7fbd64699603  (/lib64/libasan.so.4+0x2e603)
    #4 0x7fbd6469a2cc  (/lib64/libasan.so.4+0x2f2cc)
    #5 0x7fbd647517db  (/lib64/libasan.so.4+0xe67db)
    #6 0x7fbd647494d2 in __interceptor_free (/lib64/libasan.so.4+0xde4d2)
    #7 0x7fbd62791246 in _IO_new_fclose (/lib64/libc.so.6+0x6e246)
    #8 0x7fbd64748b80 in __interceptor_fclose (/lib64/libasan.so.4+0xddb80)
    #9 0x7fbd63602641 in XrdCryptosslX509ParseFile(char const*, XrdCryptoX509Chain*, char const*) /home/simonm/xrootd-pr1561/src/XrdCrypto/XrdCryptosslAux.cc:469
    #10 0x7fbd58769af0 in XrdSecProtocolgsi::QueryProxy(bool, XrdSutCache*, char const*, XrdCryptoFactory*, long, ProxyIn_t*, ProxyOut_t*) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:5126
    #11 0x7fbd5878be69 in XrdSecProtocolgsi::ClientDoInit(XrdSutBuffer*, XrdSutBuffer**, XrdOucString&) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:3075
    #12 0x7fbd5878c70d in XrdSecProtocolgsi::ParseClientInput(XrdSutBuffer*, XrdSutBuffer**, XrdOucString&) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:2942
    #13 0x7fbd5878d1e1 in XrdSecProtocolgsi::getCredentials(XrdSecBuffer*, XrdOucErrInfo*) /home/simonm/xrootd-pr1561/src/XrdSecgsi/XrdSecProtocolgsi.cc:1507
    #14 0x7fbd6401efa4 in XrdCl::XRootDTransport::GetCredentials(XrdSecBuffer*&, XrdCl::HandShakeData*, XrdCl::XRootDChannelInfo*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:2506
    #15 0x7fbd64029d12 in XrdCl::XRootDTransport::DoAuthentication(XrdCl::HandShakeData*, XrdCl::XRootDChannelInfo*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:2259
    #16 0x7fbd6402eb8b in XrdCl::XRootDTransport::HandShakeMain(XrdCl::HandShakeData*, XrdCl::AnyObject&) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:516
    #17 0x7fbd6402fc69 in XrdCl::XRootDTransport::HandShake(XrdCl::HandShakeData*, XrdCl::AnyObject&) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClXRootDTransport.cc:416
    #18 0x7fbd6422b2f5 in XrdCl::AsyncSocketHandler::HandleHandShake(std::unique_ptr<XrdCl::Message, std::default_delete<XrdCl::Message> >) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:539
    #19 0x7fbd6422d38c in XrdCl::AsyncSocketHandler::OnReadWhileHandshaking() /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:527
    #20 0x7fbd642333fc in XrdCl::AsyncSocketHandler::Event(unsigned char, XrdCl::Socket*) /home/simonm/xrootd-pr1561/src/XrdCl/XrdClAsyncSocketHandler.cc:227
    #21 0x7fbd63fc510d in Event /home/simonm/xrootd-pr1561/src/XrdCl/XrdClPollerBuiltIn.cc:83
    #22 0x7fbd635cc497 in XrdSys::IOEvents::Poller::CbkXeq(XrdSys::IOEvents::Channel*, int, int, char const*) /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysIOEvents.cc:695
    #23 0x7fbd635d00ec in XrdSys::IOEvents::PollE::Dispatch(XrdSys::IOEvents::Channel*, unsigned int) /home/simonm/xrootd-pr1561/src/./XrdSys/XrdSysIOEventsPollE.icc:277
    #24 0x7fbd635d06a6 in XrdSys::IOEvents::PollE::Begin(XrdSysSemaphore*, int&, char const**) /home/simonm/xrootd-pr1561/src/./XrdSys/XrdSysIOEventsPollE.icc:232
    #25 0x7fbd635c608f in XrdSys::IOEvents::BootStrap::Start(void*) /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysIOEvents.cc:133
    #26 0x7fbd635dece9 in XrdSysThread_Xeq /home/simonm/xrootd-pr1561/src/XrdSys/XrdSysPthread.cc:86
    #27 0x7fbd62af8ea4 in start_thread (/lib64/libpthread.so.0+0x7ea4)
    #28 0x7fbd62821b0c in clone (/lib64/libc.so.6+0xfeb0c)

@gganis
Copy link
Member Author

gganis commented Nov 25, 2021

Hi @simonmichal,
Thanks, indeed when the key file needed to be open inside ParseFile it was closing the cert file.
It should be fixed by the last patch.

@simonmichal
Copy link
Contributor

@gganis : thanks a lot for the quick fix! I can confirm that now it works as advertised :-)

@ellert
Copy link
Contributor

ellert commented Nov 26, 2021

Until the old pre-openssl3 code is removed, any change to code that exists in two version must be changed twice.
This PR changes the file:
src/XrdCrypto/XrdCryptosslAux.cc
It must therefore also introduce the same changes to the file:
src/XrdCrypto/openssl3/XrdCryptosslAux.cc
In order for the two implementations to be equivalent.
The openssl3 version of the code is compilable also with older openssl versions, and should at some point become the only version, but until this happens, changes must be implemented twice.

@simonmichal
Copy link
Contributor

@ellert : noted, I'll take care of this later on this week!

@simonmichal simonmichal merged commit 5cde9c9 into xrootd:master Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants