diff --git a/src/XrdClHttp b/src/XrdClHttp index b10ec4b84be..bfd1ff9bbd4 160000 --- a/src/XrdClHttp +++ b/src/XrdClHttp @@ -1 +1 @@ -Subproject commit b10ec4b84be17ff46f6093030509c897e4b19855 +Subproject commit bfd1ff9bbd4f44adcada0f73cc23fc8f540a7434 diff --git a/src/XrdCrypto/XrdCryptoFactory.cc b/src/XrdCrypto/XrdCryptoFactory.cc index 03f4941f555..9b35efad7cd 100644 --- a/src/XrdCrypto/XrdCryptoFactory.cc +++ b/src/XrdCrypto/XrdCryptoFactory.cc @@ -314,6 +314,16 @@ XrdCryptoX509ParseFile_t XrdCryptoFactory::X509ParseFile() return 0; } +//______________________________________________________________________________ +XrdCryptoX509ParseStack_t XrdCryptoFactory::X509ParseStack() +{ + // Return an instance of an implementation of a function + // to parse a file supposed to contain for X509 certificates. + + ABSTRACTMETHOD("XrdCryptoFactory::X509ParseStack"); + return 0; +} + //______________________________________________________________________________ XrdCryptoX509ParseBucket_t XrdCryptoFactory::X509ParseBucket() { diff --git a/src/XrdCrypto/XrdCryptoFactory.hh b/src/XrdCrypto/XrdCryptoFactory.hh index 9ee1aabc267..314652b506d 100644 --- a/src/XrdCrypto/XrdCryptoFactory.hh +++ b/src/XrdCrypto/XrdCryptoFactory.hh @@ -79,6 +79,11 @@ typedef int (*XrdCryptoX509ChainToFile_t)(XrdCryptoX509Chain *, const char *); // certificates from file parsing typedef int (*XrdCryptoX509ParseFile_t)(const char *fname, XrdCryptoX509Chain *); + +// certificates from STACK_OF(X509*) +typedef int (*XrdCryptoX509ParseStack_t)(void* ssl_conn, + XrdCryptoX509Chain *c); + // certificates from bucket parsing typedef int (*XrdCryptoX509ParseBucket_t)(XrdSutBucket *, XrdCryptoX509Chain *); @@ -173,6 +178,7 @@ public: virtual XrdCryptoX509VerifyCert_t X509VerifyCert(); virtual XrdCryptoX509VerifyChain_t X509VerifyChain(); virtual XrdCryptoX509ParseFile_t X509ParseFile(); + virtual XrdCryptoX509ParseStack_t X509ParseStack(); virtual XrdCryptoX509ParseBucket_t X509ParseBucket(); virtual XrdCryptoX509ExportChain_t X509ExportChain(); virtual XrdCryptoX509ChainToFile_t X509ChainToFile(); diff --git a/src/XrdCrypto/XrdCryptoX509Chain.cc b/src/XrdCrypto/XrdCryptoX509Chain.cc index 44bdf0b54e8..e840fc46dd7 100644 --- a/src/XrdCrypto/XrdCryptoX509Chain.cc +++ b/src/XrdCrypto/XrdCryptoX509Chain.cc @@ -308,6 +308,8 @@ void XrdCryptoX509Chain::PushBack(XrdCryptoX509 *c) end->SetNext(nc); end = nc; size++; + } else if (c) { + delete c; } // Search for the effective CA (the last one, in case of subCAs) diff --git a/src/XrdCrypto/XrdCryptosslAux.cc b/src/XrdCrypto/XrdCryptosslAux.cc index 04e65c574b1..85fc59c5097 100644 --- a/src/XrdCrypto/XrdCryptosslAux.cc +++ b/src/XrdCrypto/XrdCryptosslAux.cc @@ -43,6 +43,7 @@ #include "XrdCrypto/XrdCryptosslX509.hh" #include "XrdCrypto/XrdCryptosslTrace.hh" #include +#include // Error code from verification set by verify callback function static int gErrVerifyChain = 0; @@ -376,6 +377,63 @@ int XrdCryptosslX509ChainToFile(XrdCryptoX509Chain *ch, const char *fn) return 0; } +//____________________________________________________________________________ +int XrdCryptosslX509ParseStack(void* ssl_conn, XrdCryptoX509Chain *chain) +{ + EPNAME("X509ParseStack"); + SSL* ssl = (SSL*) ssl_conn; + + int nci = 0; + // Make sure we got a chain where to add the certificates + if (!chain) { + DEBUG("chain undefined: can do nothing"); + return nci; + } + + STACK_OF(X509) *st_x509 = SSL_get_peer_cert_chain(ssl); + // NOTE: SSL_get_peer_certificate increments the refcount; + // we must free it or pass along ownership. + X509 *peer_cert = SSL_get_peer_certificate(ssl); + + if (peer_cert) { + XrdCryptoX509 *c = new XrdCryptosslX509(peer_cert); + if (c) { + chain->PushBack(c); + nci ++; + } else { + X509_free(peer_cert); + } + } + if (!st_x509) { + 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) { + // The SSL_get_peer_chain method does not increment the + // refcount; the XrdCryptoX509 object assumes it owns + // the X509* but also does not increment the refcount. + // Hence, we increment manually. +#if OPENSSL_VERSION_NUMBER < 0x010100000L + CRYPTO_add(&(cert->references), 1, CRYPTO_LOCK_X509); +#else + X509_up_ref(cert); +#endif + chain->PushBack(c); + } else { + X509_free(cert); + 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..4531f81e7c0 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(void* ssl_conn, 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/XrdCrypto/XrdCryptosslFactory.cc b/src/XrdCrypto/XrdCryptosslFactory.cc index cc2337389a8..c321086237c 100644 --- a/src/XrdCrypto/XrdCryptosslFactory.cc +++ b/src/XrdCrypto/XrdCryptosslFactory.cc @@ -477,6 +477,15 @@ XrdCryptoX509ParseFile_t XrdCryptosslFactory::X509ParseFile() return &XrdCryptosslX509ParseFile; } +//______________________________________________________________________________ +XrdCryptoX509ParseStack_t XrdCryptosslFactory::X509ParseStack() +{ + // Return an instance of an implementation of a function + // to parse a file supposed to contain for X509 certificates. + + return &XrdCryptosslX509ParseStack; +} + //______________________________________________________________________________ XrdCryptoX509ParseBucket_t XrdCryptosslFactory::X509ParseBucket() { diff --git a/src/XrdCrypto/XrdCryptosslFactory.hh b/src/XrdCrypto/XrdCryptosslFactory.hh index c64568de3a9..e6871827977 100644 --- a/src/XrdCrypto/XrdCryptosslFactory.hh +++ b/src/XrdCrypto/XrdCryptosslFactory.hh @@ -95,6 +95,7 @@ public: XrdCryptoX509VerifyCert_t X509VerifyCert(); XrdCryptoX509VerifyChain_t X509VerifyChain(); XrdCryptoX509ParseFile_t X509ParseFile(); + XrdCryptoX509ParseStack_t X509ParseStack(); XrdCryptoX509ParseBucket_t X509ParseBucket(); XrdCryptoX509ExportChain_t X509ExportChain(); XrdCryptoX509ChainToFile_t X509ChainToFile(); diff --git a/src/XrdHttp/XrdHttpProtocol.cc b/src/XrdHttp/XrdHttpProtocol.cc index d8172607a9e..e7f7f4d4897 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" @@ -283,124 +285,98 @@ XrdProtocol *XrdHttpProtocol::Match(XrdLink *lp) { int XrdHttpProtocol::GetVOMSData(XrdLink *lp) { TRACEI(DEBUG, " Extracting auth info."); + char bufname[256]; + int mape; + XrdCryptoX509Chain chain; - X509 *peer_cert; + if (!myCryptoFactory->X509ParseStack()((void*) ssl, &chain)) { + TRACEI(DEBUG, "No certificates found in peer chain."); + return 0; + } - // 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); - ERR_print_errors(sslbio_err); + const char * dn = chain.EECname(); - if (peer_cert) { - 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 - - - - 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] ) { - 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); - - // 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); - } - } - + if (!dn) { + chain.Cleanup(); + return 0; + } + + if (SecEntity.moninfo) { + free(SecEntity.moninfo); + } + + SecEntity.moninfo = strdup(dn); + TRACEI(DEBUG, " Subject name is : '" << SecEntity.moninfo << "'"); + + // X509Chain doesn't assume it owns the underlying certs unless + // you explicitly invoke the Cleanup method; the destructor will + // otherwise leak these. + chain.Cleanup(); + + if (servGMap) { + mape = servGMap->dn2user(SecEntity.moninfo, bufname, sizeof(bufname), 0); + if ( !mape && SecEntity.moninfo[0] ) { + TRACEI(DEBUG, " Mapping name: '" << SecEntity.moninfo << "' --> " << bufname); + if (SecEntity.name) free(SecEntity.name); + SecEntity.name = strdup(bufname); } else { - - SecEntity.moninfo = X509_NAME_oneline(X509_get_subject_name(peer_cert), NULL, 0); - TRACEI(DEBUG, " Subject name is : '" << SecEntity.moninfo << "'"); + TRACEI(ALL, " Mapping name: " << SecEntity.moninfo << " Failed. err: " << mape); } - - 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); - SecEntity.name = 0; - // To set the name we pick the first CN of the certificate subject - // and hope that it makes some sense, it usually does - char *lnpos = strstr(SecEntity.moninfo, "/CN="); - char bufname2[9]; - - - if (lnpos) { - lnpos += 4; - char *lnpos2 = index(lnpos, '/'); - if (lnpos2) { - int l = ( lnpos2-lnpos < (int)sizeof(bufname) ? lnpos2-lnpos : (int)sizeof(bufname)-1 ); - strncpy(bufname, lnpos, l); - bufname[l] = '\0'; - - // Here we have the string in the buffer. Take the last 8 non-space characters - size_t j = 8; - strcpy(bufname2, "unknown-\0"); // note it's 9 chars - for (int i = (int)strlen(bufname)-1; i >= 0; i--) { - if (isalnum(bufname[i])) { - j--; - bufname2[j] = bufname[i]; - if (j == 0) break; - } - + + } + + 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); + SecEntity.name = 0; + // To set the name we pick the first CN of the certificate subject + // and hope that it makes some sense, it usually does + char *lnpos = strstr(SecEntity.moninfo, "/CN="); + char bufname2[9]; + + + if (lnpos) { + lnpos += 4; + char *lnpos2 = index(lnpos, '/'); + if (lnpos2) { + int l = ( lnpos2-lnpos < (int)sizeof(bufname) ? lnpos2-lnpos : (int)sizeof(bufname)-1 ); + strncpy(bufname, lnpos, l); + bufname[l] = '\0'; + + // Here we have the string in the buffer. Take the last 8 non-space characters + size_t j = 8; + strcpy(bufname2, "unknown-\0"); // note it's 9 chars + for (int i = (int)strlen(bufname)-1; i >= 0; i--) { + if (isalnum(bufname[i])) { + j--; + bufname2[j] = bufname[i]; + if (j == 0) break; } - - SecEntity.name = strdup(bufname); - TRACEI(DEBUG, " Setting link name: '" << bufname2+j << "'"); - lp->setID(bufname2+j, 0); - } - } - } - - - // If we could not find anything good, take the last 8 non-space characters of the main subject - if (!SecEntity.name) { - size_t j = 8; - SecEntity.name = strdup("unknown-\0"); // note it's 9 chars - for (int i = (int)strlen(SecEntity.moninfo)-1; i >= 0; i--) { - if (isalnum(SecEntity.moninfo[i])) { - j--; - SecEntity.name[j] = SecEntity.moninfo[i]; - if (j == 0) break; - + } + + SecEntity.name = strdup(bufname); + TRACEI(DEBUG, " Setting link name: '" << bufname2+j << "'"); + lp->setID(bufname2+j, 0); } } - - - } - else return 0; // Don't fail if no cert - if (peer_cert) X509_free(peer_cert); + // If we could not find anything good, take the last 8 non-space characters of the main subject + if (!SecEntity.name) { + size_t j = 8; + SecEntity.name = strdup("unknown-\0"); // note it's 9 chars + for (int i = (int)strlen(SecEntity.moninfo)-1; i >= 0; i--) { + if (isalnum(SecEntity.moninfo[i])) { + j--; + SecEntity.name[j] = SecEntity.moninfo[i]; + if (j == 0) break; + } + } + } // Invoke our instance of the Security exctractor plugin // This will fill the XrdSec thing with VOMS info, if VOMS is @@ -413,16 +389,16 @@ int XrdHttpProtocol::GetVOMSData(XrdLink *lp) { char *savestr = 0; if (servGMap && SecEntity.name) savestr = strdup(SecEntity.name); - + int r = secxtractor->GetSecData(lp, SecEntity, ssl); - + if (servGMap && savestr) { if (SecEntity.name) free(SecEntity.name); SecEntity.name = savestr; } - - + + if (r) TRACEI(ALL, " Certificate data extraction failed: " << SecEntity.moninfo << " Failed. err: " << r); return r;