From 991600a5a43f7419ae359655f5b0cdc408e6f926 Mon Sep 17 00:00:00 2001 From: Brian Bockelman Date: Tue, 23 Jun 2020 23:13:34 -0500 Subject: [PATCH] Switch DN extraction to use the existing library. OpenSSL's raw functions don't understand proxy certificates, resulting in garbage sent to the gridmap file. We can use the existing XrdCrypto library instead. --- src/XrdCrypto/XrdCryptosslAux.cc | 28 +++++++++++++++ src/XrdCrypto/XrdCryptosslAux.hh | 2 ++ src/XrdHttp/XrdHttpProtocol.cc | 61 +++++++++++--------------------- 3 files changed, 50 insertions(+), 41 deletions(-) diff --git a/src/XrdCrypto/XrdCryptosslAux.cc b/src/XrdCrypto/XrdCryptosslAux.cc index 04e65c574b1..cc307becd4d 100644 --- a/src/XrdCrypto/XrdCryptosslAux.cc +++ b/src/XrdCrypto/XrdCryptosslAux.cc @@ -376,6 +376,34 @@ int XrdCryptosslX509ChainToFile(XrdCryptoX509Chain *ch, const char *fn) return 0; } +//____________________________________________________________________________ +int XrdCryptosslX509ParseStack(STACK_OF(X509*) st_x509, XrdCryptoX509Chain *chain) +{ + EPNAME("X509ParseStack"); + + int nci = 0; + // Make sure we got a chain where to add the certificates + if (!chain) { + DEBUG("chain undefined: can do nothing"); + return nci; + } + + for (int i=0; i < sk_X509_num(st_x509); i++) { + X509 *cert = sk_X509_value(st_x509, i); + XrdCryptoX509 *c = new XrdCryptosslX509(cert); + if (c) { + chain->PushBack(c); + } else { + DEBUG("could not create certificate: memory exhausted?"); + chain->Reorder(); + return nci; + } + nci ++; + } + chain->Reorder(); + return nci; +} + //____________________________________________________________________________ int XrdCryptosslX509ParseFile(const char *fname, XrdCryptoX509Chain *chain) diff --git a/src/XrdCrypto/XrdCryptosslAux.hh b/src/XrdCrypto/XrdCryptosslAux.hh index b9d7daf8a7c..2668acf8a6b 100644 --- a/src/XrdCrypto/XrdCryptosslAux.hh +++ b/src/XrdCrypto/XrdCryptosslAux.hh @@ -60,6 +60,8 @@ int XrdCryptosslX509ChainToFile(XrdCryptoX509Chain *c, const char *fn); int XrdCryptosslX509ParseFile(const char *fname, XrdCryptoX509Chain *c); // certificates from bucket parsing int XrdCryptosslX509ParseBucket(XrdSutBucket *b, XrdCryptoX509Chain *c); +// certificates from STACK_OF(X509*) +int XrdCryptosslX509ParseStack(STACK_OF(X509*) st_x509, XrdCryptoX509Chain *c); // // Function to convert from ASN1 time format into UTC since Epoch (Jan 1, 1970) time_t XrdCryptosslASN1toUTC(const ASN1_TIME *tsn1); diff --git a/src/XrdHttp/XrdHttpProtocol.cc b/src/XrdHttp/XrdHttpProtocol.cc index d8172607a9e..7c1a802f78b 100644 --- a/src/XrdHttp/XrdHttpProtocol.cc +++ b/src/XrdHttp/XrdHttpProtocol.cc @@ -32,6 +32,8 @@ #include "XrdOuc/XrdOucGMap.hh" #include "XrdSys/XrdSysTimer.hh" #include "XrdOuc/XrdOucPinLoader.hh" +#include "XrdCrypto/XrdCryptoX509Chain.hh" +#include "XrdCrypto/XrdCryptosslAux.hh" #include "XrdHttpTrace.hh" #include "XrdHttpProtocol.hh" @@ -284,30 +286,29 @@ XrdProtocol *XrdHttpProtocol::Match(XrdLink *lp) { int XrdHttpProtocol::GetVOMSData(XrdLink *lp) { TRACEI(DEBUG, " Extracting auth info."); - X509 *peer_cert; - // No external plugin, hence we fill our XrdSec with what we can do here - peer_cert = SSL_get_peer_certificate(ssl); - TRACEI(DEBUG, " SSL_get_peer_certificate returned :" << peer_cert); + STACK_OF(X509) *peer_chain = SSL_get_peer_cert_chain(ssl); + TRACEI(DEBUG, " SSL_get_peer_certificate returned :" << peer_chain); ERR_print_errors(sslbio_err); - if (peer_cert) { + if (peer_chain) { char bufname[256]; - + // Add the original DN to the moninfo. Not sure if it makes sense to parametrize this or not. if (SecEntity.moninfo) free(SecEntity.moninfo); - - // The original DN is not so univoque. In the case of a proxy this can be either - // the issuer name or the subject name - // Here we try to use the one that is known to the user mapping - - - + + int mape; + + XrdCryptoX509Chain chain; + int count = XrdCryptosslX509ParseStack(peer_chain, &chain); + if (!count) { + TRACEI(DEBUG, " No certificates found in peer chain."); + } + const char * dn = chain.EECname(); + SecEntity.moninfo = strdup(dn); + + TRACEI(DEBUG, " Subject name is : '" << SecEntity.moninfo << "'"); if (servGMap) { - int mape; - - SecEntity.moninfo = X509_NAME_oneline(X509_get_issuer_name(peer_cert), NULL, 0); - TRACEI(DEBUG, " Issuer name is : '" << SecEntity.moninfo << "'"); mape = servGMap->dn2user(SecEntity.moninfo, bufname, sizeof(bufname), 0); if ( !mape && SecEntity.moninfo[0] ) { @@ -316,31 +317,11 @@ int XrdHttpProtocol::GetVOMSData(XrdLink *lp) { SecEntity.name = strdup(bufname); } else { - TRACEI(ALL, " Mapping name: '" << SecEntity.moninfo << "' Failed. err: " << mape); - - // Mapping the issuer name failed, let's try with the subject name - if (SecEntity.moninfo) free(SecEntity.moninfo); - SecEntity.moninfo = X509_NAME_oneline(X509_get_subject_name(peer_cert), NULL, 0); - TRACEI(DEBUG, " Subject name is : '" << SecEntity.moninfo << "'"); - - mape = servGMap->dn2user(SecEntity.moninfo, bufname, sizeof(bufname), 0); - if ( !mape ) { - TRACEI(DEBUG, " Mapping name: " << SecEntity.moninfo << " --> " << bufname); - if (SecEntity.name) free(SecEntity.name); - SecEntity.name = strdup(bufname); - } - else { - TRACEI(ALL, " Mapping name: " << SecEntity.moninfo << " Failed. err: " << mape); - } + TRACEI(ALL, " Mapping name: " << SecEntity.moninfo << " Failed. err: " << mape); } } - else { - - SecEntity.moninfo = X509_NAME_oneline(X509_get_subject_name(peer_cert), NULL, 0); - TRACEI(DEBUG, " Subject name is : '" << SecEntity.moninfo << "'"); - } - + if (!SecEntity.name) { // Here we have the user DN, and try to extract an useful user name from it if (SecEntity.name) free(SecEntity.name); @@ -398,8 +379,6 @@ int XrdHttpProtocol::GetVOMSData(XrdLink *lp) { } else return 0; // Don't fail if no cert - if (peer_cert) X509_free(peer_cert); - // Invoke our instance of the Security exctractor plugin