Skip to content

Commit

Permalink
Merge pull request #844 from bbockelm/fix_gsi_reverse_dns
Browse files Browse the repository at this point in the history
Always fall back to reverse DNS if trusted
  • Loading branch information
abh3 committed Oct 18, 2018
2 parents 2a5d93f + 5762f13 commit 9b957ca
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 12 deletions.
31 changes: 21 additions & 10 deletions src/XrdSecgsi/XrdSecProtocolgsi.cc
Expand Up @@ -308,6 +308,7 @@ XrdSecProtocolgsi::XrdSecProtocolgsi(int opts, const char *hname,
// usage of DNS (processed on XrdSecProtocolgsiInit).
// Default is to fallback to DNS lookups in limited
// cases for backward compatibility.
expectedHost = NULL;
if (TrustDNS) {
if (!hname || !XrdNetAddrInfo::isHostName(hname)) {
Entity.host = strdup(endPoint.Name(""));
Expand Down Expand Up @@ -337,6 +338,7 @@ XrdSecProtocolgsi::XrdSecProtocolgsi(int opts, const char *hname,
// We have been told via environment variable to not trust DNS; use the exact
// hostname provided by the user.
Entity.host = strdup(hname);
expectedHost = strdup(hname);
}
epAddr = endPoint;
Entity.addrInfo = &epAddr;
Expand Down Expand Up @@ -1031,6 +1033,7 @@ void XrdSecProtocolgsi::Delete()
SafeDelete(sessionKsig); // RSA key to sign
SafeDelete(sessionKver); // RSA key to verify
SafeDelete(proxyChain); // Chain with delegated proxies
SafeDelete(expectedHost);

delete this;
}
Expand Down Expand Up @@ -3118,10 +3121,18 @@ int XrdSecProtocolgsi::ClientDoCert(XrdSutBuffer *br, XrdSutBuffer **bm,
}
//
// Verify server identity
// First, check the DN. On failure, we will iterate through
// the alternate names.
if (!ServerCertNameOK(hs->Chain->End()->Subject(), emsg) &&
// First, check the DN against the client-provided hostname.
// On failure, we will iterate through the alternate names in the cert.
// If that fails, we will do a reverse DNS lookup of the IP address.
if (!ServerCertNameOK(hs->Chain->End()->Subject(), Entity.host, emsg) &&
!hs->Chain->End()->MatchesSAN(Entity.host)) {
if ((expectedHost == NULL) && TrustDNS && Entity.addrInfo) {
const char *name = Entity.addrInfo->Name();
expectedHost = strdup(name);
if ((name == NULL) || !ServerCertNameOK(hs->Chain->End()->Subject(), expectedHost, emsg)) {
return -1;
}
}
return -1;
}
//
Expand Down Expand Up @@ -5259,7 +5270,7 @@ XrdSecgsiVOMS_t XrdSecProtocolgsi::LoadVOMSFun(const char *plugin,


//_____________________________________________________________________________
bool XrdSecProtocolgsi::ServerCertNameOK(const char *subject, XrdOucString &emsg)
bool XrdSecProtocolgsi::ServerCertNameOK(const char *subject, const char *hname, XrdOucString &emsg)
{
// Check that the server certificate subject name is consistent with the
// expectations defined by the static SrvAllowedNames
Expand All @@ -5277,20 +5288,20 @@ bool XrdSecProtocolgsi::ServerCertNameOK(const char *subject, XrdOucString &emsg
if (cnidx != STR_NPOS) srvcn.assign(srvsubj, cnidx + 3);

// Always check if the server CN is in the standard form "[*/]<target host name>[/*]"
if (Entity.host) {
if (hname) {
size_t ih = srvcn.find("/");
if (ih != std::string::npos) {
srvcn.erasefromstart(ih + 1);
}
allowed = XrdCryptoX509::MatchHostnames(srvcn.c_str(), Entity.host);
allowed = XrdCryptoX509::MatchHostnames(srvcn.c_str(), hname);

// Update the error msg, if the case
if (!allowed) {
if (emsg.length() <= 0) {
emsg = "server certificate CN '"; emsg += srvcn;
emsg += "' does not match the expected format(s):";
}
String defcn("[*/]"); defcn += Entity.host; defcn += "[/*]";
String defcn("[*/]"); defcn += hname; defcn += "[/*]";
emsg += " '"; emsg += defcn; emsg += "' (default)";
}
}
Expand All @@ -5299,12 +5310,12 @@ bool XrdSecProtocolgsi::ServerCertNameOK(const char *subject, XrdOucString &emsg
if (SrvAllowedNames.length() > 0) {
// The SrvAllowedNames string contains the allowed formats separated by a '|'.
// The specifications can contain the <host> or <fqdn> placeholders which
// are replaced by Entity.host; they can also contain the '*' wildcard, in
// are replaced by hname; they can also contain the '*' wildcard, in
// which case XrdOucString::matches is used. A '-' before the specification
// will deny the matching CN's; the last matching wins.
String allowedfmts(SrvAllowedNames);
allowedfmts.replace("<host>", (const char *) Entity.host);
allowedfmts.replace("<fqdn>", (const char *) Entity.host);
allowedfmts.replace("<host>", hname);
allowedfmts.replace("<fqdn>", hname);
int from = 0;
String fmt;
while ((from = allowedfmts.tokenize(fmt, from, '|')) != -1) {
Expand Down
5 changes: 3 additions & 2 deletions src/XrdSecgsi/XrdSecProtocolgsi.hh
Expand Up @@ -390,7 +390,8 @@ private:
XrdCryptoRSA *sessionKsig; // RSA key to sign
XrdCryptoRSA *sessionKver; // RSA key to verify
X509Chain *proxyChain; // Chain with the delegated proxy on servers
bool srvMode; // TRUE if server mode
bool srvMode; // TRUE if server mode
char *expectedHost; // Expected hostname if TrustDNS is enabled.

// Temporary Handshake local info
gsiHSVars *hs;
Expand Down Expand Up @@ -426,7 +427,7 @@ private:
static bool VerifyCA(int opt, X509Chain *cca, XrdCryptoFactory *cf);
static int VerifyCRL(XrdCryptoX509Crl *crl, XrdCryptoX509 *xca, XrdOucString crldir,
XrdCryptoFactory *CF, int hashalg);
bool ServerCertNameOK(const char *subject, String &e);
bool ServerCertNameOK(const char *subject, const char *hname, String &e);
static XrdSutCacheEntry *GetSrvCertEnt(XrdSutCERef &gcref,
XrdCryptoFactory *cf,
time_t timestamp, String &cal);
Expand Down

0 comments on commit 9b957ca

Please sign in to comment.