[2/2] git commit: TS-2972 Change the hook used, to always do the authproxy phase

[zw...@apache.org]

TS-2972 Change the hook used, to always do the authproxy phase


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/38b4765a
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/38b4765a
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/38b4765a

Branch: refs/heads/master
Commit: 38b4765a2e14c07c8884668e072c27166d3ef719
Parents: 083e21b
Author: Leif Hedstrom <zw...@apache.org>
Authored: Thu Jul 31 10:17:46 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Sat Aug 16 17:50:13 2014 -0600

----------------------------------------------------------------------
 CHANGES                                     |  5 +++
 plugins/experimental/authproxy/authproxy.cc | 57 +++++++++++-------------
 2 files changed, 30 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/38b4765a/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index c2ec1a9..e51385e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.1.0
 
+  *) [TS-2722] authproxy: Eliminate the DNS lookup state, just use the client
+   IP with TSHttpConnect.
+
+  *) [TS-2972] authproxy: Change the hook used, to always do the auth.
+
   *) [TS-698] Add IP filter to logging.
 
   *) [TS-3002] Add DSCP support to header_rewrite.

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/38b4765a/plugins/experimental/authproxy/authproxy.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/authproxy/authproxy.cc b/plugins/experimental/authproxy/authproxy.cc
index ae20a31..4e22d14 100644
--- a/plugins/experimental/authproxy/authproxy.cc
+++ b/plugins/experimental/authproxy/authproxy.cc
@@ -22,8 +22,8 @@
 // the request (or not) by consulting some authoritative source.
 //
 // This plugin follows the pattern of the basic-auth sample code. We use the
-// TS_HTTP_OS_DNS_HOOK to perform the initial authorization, and the
-// TS_HTTP_SEND_RESPONSE_HDR_HOOK to send an error response if necessary.
+// TS_HTTP_POST_REMAP_HOOK to perform the initial authorization, and
+// the TS_HTTP_SEND_RESPONSE_HDR_HOOK to send an error response if necessary.
 
 #include "utils.h"
 #include <string>
@@ -50,26 +50,23 @@ const static int MAX_HOST_LENGTH = 4096;
 // We can operate in global plugin mode or remap plugin mode. If we are in
 // global mode, then we will authorize every request. In remap mode, we will
 // only authorize tagged requests.
-static bool AuthTaggedRequestOnly = false;
 static int AuthTaggedRequestArg = -1;
 
 static TSCont AuthOsDnsContinuation;
 
 struct AuthOptions
 {
-  char* hostname;
-  int hostname_len;
+  std::string hostname;
   int hostport;
-  bool force;
   AuthRequestTransform transform;
+  bool force;
 
   AuthOptions():
-    hostname(NULL), hostname_len(-1), hostport(-1), force(false), transform(NULL)
+    hostport(-1), transform(NULL), force(false)
   { }
 
   ~AuthOptions()
   {
-    TSfree(hostname);
   }
 };
 
@@ -149,7 +146,7 @@ static const StateTransition StateTableProxyRequest[] = {
 
 // Initial state table.
 static const StateTransition StateTableInit[] = {
-  { TS_EVENT_HTTP_OS_DNS, StateAuthProxyConnect, StateTableProxyRequest },
+  { TS_EVENT_HTTP_POST_REMAP, StateAuthProxyConnect, StateTableProxyRequest },
   { TS_EVENT_ERROR, StateUnauthorized, NULL },
   { TS_EVENT_NONE, NULL, NULL }
 };
@@ -345,12 +342,12 @@ AuthWriteRedirectedRequest(AuthRequestContext* auth)
   // scheme, forcing ATS to go to the Host header. I wonder how HTTPS would
   // work in that case. At any rate, we should add a new header containing
   // the original host so that the auth proxy can examine it.
-  TSUrlHostSet(rq.buffer, murl, options->hostname, options->hostname_len);
+  TSUrlHostSet(rq.buffer, murl, options->hostname.c_str(), options->hostname.size());
   if (-1 != options->hostport) {
-    snprintf(hostbuf, sizeof(hostbuf), "%s:%d", options->hostname, options->hostport);
+    snprintf(hostbuf, sizeof(hostbuf), "%s:%d", options->hostname.c_str(), options->hostport);
     TSUrlPortSet(rq.buffer, murl, options->hostport);
   } else {
-    snprintf(hostbuf, sizeof(hostbuf), "%s", options->hostname);
+    snprintf(hostbuf, sizeof(hostbuf), "%s", options->hostname.c_str());
   }
 
   TSHandleMLocRelease(rq.buffer, rq.header, murl);
@@ -370,7 +367,7 @@ AuthWriteRedirectedRequest(AuthRequestContext* auth)
 }
 
 static TSEvent
-StateAuthProxyConnect(AuthRequestContext* auth, void* edata)
+StateAuthProxyConnect(AuthRequestContext* auth, void* /* edata ATS_UNUSED */)
 {
   const AuthOptions* options = auth->options();
   struct sockaddr const* ip = TSHttpTxnClientAddrGet(auth->txn);
@@ -617,13 +614,17 @@ AuthProxyGlobalHook(TSCont /* cont ATS_UNUSED */ , TSEvent event, void* edata)
   AuthLogDebug("handling event=%d edata=%p", (int) event, edata);
 
   switch (event) {
-  case TS_EVENT_HTTP_OS_DNS:
+  case TS_EVENT_HTTP_POST_REMAP:
     // Ignore internal requests since we generated them.
     if (TSHttpIsInternalRequest(txn) == TS_SUCCESS) {
       // All our internal requests *must* hit the origin since it is the
       // agent that needs to make the authorization decision. We can't
-      // allow that to be cached.
-      TSHttpTxnReqCacheableSet(txn, 0);
+      // allow that to be cached. Note that this only affects the remap
+      // rule that this plugin is instantiated for, *unless* you are using
+      // it as a global plugin (not highly recommended). Also remember that
+      // the HEAD auth request might trip a different remap rule, particularly
+      // if you do not have pristine host-headers enabled.
+      TSHttpTxnConfigIntSet(txn, TS_CONFIG_HTTP_CACHE_HTTP, 0);
 
       AuthLogDebug("re-enabling internal transaction");
       TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
@@ -674,8 +675,7 @@ AuthParseOptions(int argc, const char** argv)
     opt = getopt_long(argc, (char* const *) argv, "", longopt, NULL);
     switch (opt) {
     case 'h':
-      TSfree(options->hostname); // Probably isn't set, but safe to always free
-      options->hostname = TSstrdup(optarg);
+      options->hostname = optarg;
       break;
     case 'p':
       options->hostport = std::atoi(optarg);
@@ -692,7 +692,6 @@ AuthParseOptions(int argc, const char** argv)
         AuthLogError("invalid authorization transform '%s'", optarg);
         // XXX make this a fatal error?
       }
-
       break;
     }
 
@@ -701,10 +700,9 @@ AuthParseOptions(int argc, const char** argv)
     }
   }
 
-  if (NULL == options->hostname) {
-    options->hostname = TSstrdup("127.0.0.1");
+  if (options->hostname.empty()) {
+    options->hostname = "127.0.0.1";
   }
-  options->hostname_len = strlen(options->hostname);
 
   return options;
 }
@@ -727,16 +725,12 @@ TSPluginInit(int argc, const char* argv[])
   TSReleaseAssert(TSHttpArgIndexReserve("AuthProxy", "AuthProxy authorization tag", &AuthTaggedRequestArg) ==
                   TS_SUCCESS);
 
-  // We are in global mode. Authorize all requests.
-  AuthTaggedRequestOnly = false;
-
   AuthOsDnsContinuation = TSContCreate(AuthProxyGlobalHook, NULL);
   AuthGlobalOptions = AuthParseOptions(argc, argv);
-  AuthLogDebug("using authorization proxy at %s:%d", AuthGlobalOptions->hostname, AuthGlobalOptions->hostport);
+  AuthLogDebug("using authorization proxy at %s:%d", AuthGlobalOptions->hostname.c_str(), AuthGlobalOptions->hostport);
 
-  // Catch the DNS hook. This triggers after reading the headers and
-  // resolving the requested host, but before performing any cache lookups.
-  TSHttpHookAdd(TS_HTTP_OS_DNS_HOOK, AuthOsDnsContinuation);
+  // Use the appropriate hook for consistent auth checks.
+  TSHttpHookAdd(TS_HTTP_POST_REMAP_HOOK, AuthOsDnsContinuation);
 }
 
 TSReturnCode
@@ -745,8 +739,6 @@ TSRemapInit(TSRemapInterface* /* api ATS_UNUSED */, char* /* err ATS_UNUSED */,
   TSReleaseAssert(TSHttpArgIndexReserve("AuthProxy", "AuthProxy authorization tag", &AuthTaggedRequestArg) ==
                   TS_SUCCESS);
 
-  // We are in remap mode. Only authorize tagged requests.
-  AuthTaggedRequestOnly = true;
   AuthOsDnsContinuation = TSContCreate(AuthProxyGlobalHook, NULL);
   return TS_SUCCESS;
 }
@@ -782,7 +774,8 @@ TSRemapDoRemap(void* instance, TSHttpTxn txn, TSRemapRequestInfo* /* rri ATS_UNU
   AuthOptions* options = (AuthOptions*) instance;
 
   TSHttpTxnArgSet(txn, AuthTaggedRequestArg, options);
-  TSHttpTxnHookAdd(txn, TS_HTTP_OS_DNS_HOOK, AuthOsDnsContinuation);
+  TSHttpTxnHookAdd(txn, TS_HTTP_POST_REMAP_HOOK, AuthOsDnsContinuation);
+
   return TSREMAP_NO_REMAP;
 }