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

[XrdHttp] Q-values presence is considered BUT discarded when pattern matching the digest algorithm name to run #1950

Merged
merged 3 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/XrdHttp.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ if( BUILD_HTTP )
XrdHttp/XrdHttpExtHandler.cc XrdHttp/XrdHttpExtHandler.hh
XrdHttp/XrdHttpStatic.hh
XrdHttp/XrdHttpTrace.hh
XrdHttp/XrdHttpUtils.cc XrdHttp/XrdHttpUtils.hh )
XrdHttp/XrdHttpUtils.cc XrdHttp/XrdHttpUtils.hh
XrdHttp/XrdHttpChecksumHandler.cc XrdHttp/XrdHttpChecksumHandler.hh
XrdHttp/XrdHttpChecksum.cc XrdHttp/XrdHttpChecksum.hh)

# Note this is marked as a shared library as XrdHttp plugins are expected to
# link against this for the XrdHttpExt class implementations.
Expand Down
14 changes: 14 additions & 0 deletions src/XrdHttp/README-CKSUM.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
In the case XrdHttp has been configured, it is important that the configuration contains at least one digest algorithm registered in the IANA database: https://www.iana.org/assignments/http-dig-alg/http-dig-alg.xhtml
Otherwise the user will get a 403 error for each checksum request they will do.

Here is a table summarizing the IANA-compliant checksum names:

| Digest algorithm configured | HTTP digest | Will be base64 encoded |
|-----------------------------|-------------|------------------------|
| md5 | md5 | true |
| adler32 | adler32 | false |
| sha1 | sha | true |
| sha256 | sha-256 | true |
| sha512 | sha-512 | true |
| cksum | UNIXcksum | false |
| crc32c | crc32c | true |
47 changes: 47 additions & 0 deletions src/XrdHttp/XrdHttpChecksum.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//------------------------------------------------------------------------------
// This file is part of XrdHTTP: A pragmatic implementation of the
// HTTP/WebDAV protocol for the Xrootd framework
//
// Copyright (c) 2013 by European Organization for Nuclear Research (CERN)
// Author: Cedric Caffy <ccaffy@cern.ch>
// File Date: Mar 2023
//------------------------------------------------------------------------------
// XRootD is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// XRootD is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with XRootD. If not, see <http://www.gnu.org/licenses/>.
//------------------------------------------------------------------------------

#include "XrdHttpChecksum.hh"
#include <algorithm>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System header first than local headers. Stylistically all local headers are always qualified by the directory they are in so we would says:

#include "XrdHttp/XrdHttpChecksum.hh"

However, I noticed that style was never followed for any code in this directory. So, don't bother changing it. We'll address this as some future point.


XrdHttpChecksum::XrdHttpChecksum(const std::string & xrootConfigDigestName, const std::string & httpName, bool needsBase64Padding):
mXRootDConfigDigestName(xrootConfigDigestName),
mHTTPName(httpName),
mNeedsBase64Padding(needsBase64Padding){}

std::string XrdHttpChecksum::getHttpName() const {
return mHTTPName;
}

std::string XrdHttpChecksum::getHttpNameLowerCase() const {
std::string ret = getHttpName();
std::transform(ret.begin(),ret.end(),ret.begin(),::tolower);
return ret;
}

std::string XrdHttpChecksum::getXRootDConfigDigestName() const {
return mXRootDConfigDigestName;
}

bool XrdHttpChecksum::needsBase64Padding() const {
return mNeedsBase64Padding;
}
54 changes: 54 additions & 0 deletions src/XrdHttp/XrdHttpChecksum.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
//------------------------------------------------------------------------------
// This file is part of XrdHTTP: A pragmatic implementation of the
// HTTP/WebDAV protocol for the Xrootd framework
//
// Copyright (c) 2013 by European Organization for Nuclear Research (CERN)
// Author: Cedric Caffy <ccaffy@cern.ch>
// File Date: Mar 2023
//------------------------------------------------------------------------------
// XRootD is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// XRootD is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with XRootD. If not, see <http://www.gnu.org/licenses/>.
//------------------------------------------------------------------------------

#ifndef XROOTD_XRDHTTPCHECKSUM_HH
#define XROOTD_XRDHTTPCHECKSUM_HH

#include <string>

/**
* Simple object containing information about
* a checksum
*/
class XrdHttpChecksum {
public:
/**
* Constructor
* @param xrootConfigDigestName the name that will be used by XRootD server to run the checksum
* @param httpName the HTTP RFC compliant name of the checksum
* @param needsBase64Padding sets to true if the checksum needs to be base64 encoded before being sent, false otherwise
*/
XrdHttpChecksum(const std::string & xrootConfigDigestName, const std::string & httpName, bool needsBase64Padding);

std::string getXRootDConfigDigestName() const;
std::string getHttpName() const;
std::string getHttpNameLowerCase() const;
bool needsBase64Padding() const;

private:
std::string mXRootDConfigDigestName;
std::string mHTTPName;
bool mNeedsBase64Padding;
};


#endif //XROOTD_XRDHTTPCHECKSUM_HH
116 changes: 116 additions & 0 deletions src/XrdHttp/XrdHttpChecksumHandler.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//------------------------------------------------------------------------------
// This file is part of XrdHTTP: A pragmatic implementation of the
// HTTP/WebDAV protocol for the Xrootd framework
//
// Copyright (c) 2013 by European Organization for Nuclear Research (CERN)
// Author: Cedric Caffy <ccaffy@cern.ch>
// File Date: Mar 2023
//------------------------------------------------------------------------------
// XRootD is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// XRootD is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with XRootD. If not, see <http://www.gnu.org/licenses/>.
//------------------------------------------------------------------------------

#include "XrdHttpChecksumHandler.hh"
#include "XrdOuc/XrdOucTUtils.hh"
#include "XrdOuc/XrdOucUtils.hh"
#include <exception>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

System headers ahead of local headers.

#include <algorithm>

std::map<std::string,XrdHttpChecksumHandlerImpl::XrdHttpChecksumPtr> XrdHttpChecksumHandlerImpl::XROOTD_DIGEST_NAME_TO_CKSUMS;

void XrdHttpChecksumHandlerImpl::initializeCksumsMaps() {
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("md5","md5",true));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("adler32","adler32",false));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("sha1","sha",true));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("sha256","sha-256",true));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("sha512","sha-512",true));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("cksum","UNIXcksum",false));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment about cksum vs crc32 in a previous note.

addChecksumToMaps(std::make_unique<XrdHttpChecksum>("crc32","crc32",false));
addChecksumToMaps(std::make_unique<XrdHttpChecksum>("crc32c","crc32c",true));
}

void XrdHttpChecksumHandlerImpl::addChecksumToMaps(XrdHttpChecksumHandlerImpl::XrdHttpChecksumPtr && checksum) {
// We also map xrootd-configured checksum's HTTP names to the corresponding checksums --> this will allow
// users to configure algorithms like, for example, `sha-512` and be considered as `sha512` algorithm
XROOTD_DIGEST_NAME_TO_CKSUMS[checksum->getHttpNameLowerCase()] = std::make_unique<XrdHttpChecksum>(checksum->getHttpNameLowerCase(), checksum->getHttpName(), checksum->needsBase64Padding());
XROOTD_DIGEST_NAME_TO_CKSUMS[checksum->getXRootDConfigDigestName()] = std::move(checksum);
}

XrdHttpChecksumHandlerImpl::XrdHttpChecksumRawPtr XrdHttpChecksumHandlerImpl::getChecksumToRun(const std::string &userDigestIn) const {
if(!mConfiguredChecksums.empty()) {
std::vector<std::string> userDigests = getUserDigests(userDigestIn);
//Loop over the user digests and find the corresponding checksum
for(auto userDigest: userDigests) {
auto httpCksum = std::find_if(mConfiguredChecksums.begin(), mConfiguredChecksums.end(),[userDigest](const XrdHttpChecksumRawPtr & cksum){
return userDigest == cksum->getHttpNameLowerCase();
});
if(httpCksum != mConfiguredChecksums.end()) {
return *httpCksum;
}
}
return mConfiguredChecksums[0];
}
//If there are no configured checksums, return nullptr
return nullptr;
}

const std::vector<std::string> &XrdHttpChecksumHandlerImpl::getNonIANAConfiguredCksums() const {
return mNonIANAConfiguredChecksums;
}

const std::vector<XrdHttpChecksumHandler::XrdHttpChecksumRawPtr> & XrdHttpChecksumHandlerImpl::getConfiguredChecksums() const {
return mConfiguredChecksums;
}


void XrdHttpChecksumHandlerImpl::configure(const char *csList) {
initializeCksumsMaps();
if(csList != nullptr) {
initializeXRootDConfiguredCksums(csList);
}
}

void XrdHttpChecksumHandlerImpl::initializeXRootDConfiguredCksums(const char *csList) {
std::vector<std::string> splittedCslist;
XrdOucTUtils::splitString(splittedCslist,csList,",");
for(auto csElt: splittedCslist) {
auto csName = getElement(csElt,":",1);
auto checksumItor = XROOTD_DIGEST_NAME_TO_CKSUMS.find(csName);
if(checksumItor != XROOTD_DIGEST_NAME_TO_CKSUMS.end()) {
mConfiguredChecksums.push_back(checksumItor->second.get());
} else {
mNonIANAConfiguredChecksums.push_back(csName);
}
}
}

std::string XrdHttpChecksumHandlerImpl::getElement(const std::string &input, const std::string & delimiter,
const size_t position) {
std::vector<std::string> elementsAfterSplit;
XrdOucTUtils::splitString(elementsAfterSplit,input,delimiter);
return elementsAfterSplit[position];
}

std::vector<std::string> XrdHttpChecksumHandlerImpl::getUserDigests(const std::string &userDigests) {
//userDigest is a comma-separated list with q-values
std::vector<std::string> userDigestsRet;
std::vector<std::string> userDigestsWithQValues;
XrdOucTUtils::splitString(userDigestsWithQValues,userDigests,",");
for(auto & userDigestWithQValue: userDigestsWithQValues){
std::transform(userDigestWithQValue.begin(),userDigestWithQValue.end(),userDigestWithQValue.begin(),::tolower);
auto userDigest = getElement(userDigestWithQValue,";",0);
XrdOucUtils::trim(userDigest);
userDigestsRet.push_back(userDigest);
}
return userDigestsRet;
}
116 changes: 116 additions & 0 deletions src/XrdHttp/XrdHttpChecksumHandler.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//------------------------------------------------------------------------------
// This file is part of XrdHTTP: A pragmatic implementation of the
// HTTP/WebDAV protocol for the Xrootd framework
//
// Copyright (c) 2013 by European Organization for Nuclear Research (CERN)
// Author: Cedric Caffy <ccaffy@cern.ch>
// File Date: Mar 2023
//------------------------------------------------------------------------------
// XRootD is free software: you can redistribute it and/or modify
// it under the terms of the GNU Lesser General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// XRootD is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with XRootD. If not, see <http://www.gnu.org/licenses/>.
//------------------------------------------------------------------------------
#ifndef XROOTD_XRDHTTPCHECKSUMHANDLER_HH
#define XROOTD_XRDHTTPCHECKSUMHANDLER_HH

#include "XrdHttpChecksum.hh"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local headers after system headers.


#include <string>
#include <map>
#include <vector>
#include <memory>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we try to include our headers first? This order of includes is somewhat strange.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. System headers go first followed by our headers. I know it's arbitrary but that's what we decided on way back.


/**
* Implementation class of the XrdHttpChecksumHandler
*
* Is useful for unit testing
*/
class XrdHttpChecksumHandlerImpl {
public:
using XrdHttpChecksumPtr = std::unique_ptr<XrdHttpChecksum>;
using XrdHttpChecksumRawPtr = XrdHttpChecksum *;

XrdHttpChecksumHandlerImpl() = default;

void configure(const char * csList);
XrdHttpChecksumRawPtr getChecksumToRun(const std::string & userDigest) const;
const std::vector<std::string> & getNonIANAConfiguredCksums() const;
/**
* For testing purposes
*/
const std::vector<XrdHttpChecksumRawPtr> & getConfiguredChecksums() const;
private:
/**
* Initializes the checksums from the csList parameter passed
*
* The elements of the csList parameter should all be lower-cased
* @param csList the list of the configured checksum under the format 0:adler32,1:sha1,2:sha512
*/
void initializeXRootDConfiguredCksums(const char *csList);
/**
* Modify this if new checksums have to be supported or
* if some don't require base64 padding anymore
*/
static void initializeCksumsMaps();
static void addChecksumToMaps(XrdHttpChecksumPtr && checksum);
static std::string getElement(const std::string & input, const std::string & delimiter, const size_t position);
/**
* Returns a vector of user digests (lower-cased) extracted from the userDigests string passed in parameter
* @param userDigests the string containing a quality-valued checksum list e.g: adler32, md5;q=0.4, md5
* @return the lower-cased user digests vector
*/
static std::vector<std::string> getUserDigests(const std::string & userDigests);

//The map that containing all possible IANA-HTTP-compatible xrootd checksums
static std::map<std::string,XrdHttpChecksumPtr> XROOTD_DIGEST_NAME_TO_CKSUMS;
// The vector of IANA-HTTP-compatible configured checksum
std::vector<XrdHttpChecksumRawPtr> mConfiguredChecksums;
// The vector of non-HTTP configured checksum names (for testing purposes)
std::vector<std::string> mNonIANAConfiguredChecksums;
};

/**
* This class allows to handle xrd http checksum algorithm selection
* based on what the user provided as a digest
*/
class XrdHttpChecksumHandler {
public:
using XrdHttpChecksumRawPtr = XrdHttpChecksumHandlerImpl::XrdHttpChecksumRawPtr;

XrdHttpChecksumHandler() = default;
/**
* Configure this handler.
* @throws runtime_exception if no algorithm in the csList is compatible with HTTP
* @param csList the list coming from the server configuration. Should be under the format 0:adler32,1:sha512
*/
void configure(const char * csList) { pImpl.configure(csList); }
/**
* Returns the checksum to run from the user "Want-Digest" provided string
* @param userDigest the digest string under the format "sha-512,sha-256;q=0.8,sha;q=0.6,md5;q=0.4,adler32;q=0.2"
* @return the checksum to run depending on the userDigest provided string
* The logic behind it is simple: returns the first userDigest provided that matches the one configured.
* If none is matched, the first algorithm configured on the server side will be returned.
* If no HTTP-IANA compatible checksum algorithm has been configured or NO checksum algorithm have been configured, nullptr will be returned.
*/
XrdHttpChecksumRawPtr getChecksumToRun(const std::string & userDigest) const { return pImpl.getChecksumToRun(userDigest); }

/**
* Returns the checksums that are incompatible with HTTP --> the ones that
* we do not know whether the result should be base64 encoded or not
*/
const std::vector<std::string> & getNonIANAConfiguredCksums() const { return pImpl.getNonIANAConfiguredCksums(); }
private:
XrdHttpChecksumHandlerImpl pImpl;
};


#endif //XROOTD_XRDHTTPCHECKSUMHANDLER_HH
14 changes: 14 additions & 0 deletions src/XrdHttp/XrdHttpProtocol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ XrdSecService *XrdHttpProtocol::CIA = 0; // Authentication Server
int XrdHttpProtocol::m_bio_type = 0; // BIO type identifier for our custom BIO.
BIO_METHOD *XrdHttpProtocol::m_bio_method = NULL; // BIO method constructor.
char *XrdHttpProtocol::xrd_cslist = nullptr;
XrdHttpChecksumHandler XrdHttpProtocol::cksumHandler = XrdHttpChecksumHandler();

XrdSysTrace XrdHttpTrace("http");

Expand Down Expand Up @@ -949,6 +950,19 @@ int XrdHttpProtocol::Config(const char *ConfigFN, XrdOucEnv *myEnv) {
char *var;
int cfgFD, GoNo, NoGo = 0, ismine;

cksumHandler.configure(xrd_cslist);
auto nonIanaChecksums = cksumHandler.getNonIANAConfiguredCksums();
if(nonIanaChecksums.size()) {
std::stringstream warningMsgSS;
warningMsgSS << "Config warning: the following checksum algorithms are not IANA compliant: [";
std::string unknownCksumString;
for(auto unknownCksum: nonIanaChecksums) {
unknownCksumString += unknownCksum + ",";
}
unknownCksumString.erase(unknownCksumString.size() - 1);
warningMsgSS << unknownCksumString << "]" << ". They therefore cannot be queried by a user via HTTP." ;
eDest.Say(warningMsgSS.str().c_str());
}

// Initialize our custom BIO type.
if (!m_bio_type) {
Expand Down