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

Always fall back to reverse DNS if trusted #844

Merged
merged 2 commits into from
Oct 18, 2018
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
31 changes: 21 additions & 10 deletions src/XrdSecgsi/XrdSecProtocolgsi.cc
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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