Custom 'user' placeholder along with 'header' directive

Coding
1

I’m working on a a new authentication middleware that performs HTTP Basic Auth, but authenticating against RADIUS backend. That all works so far.

My issue, is that I noticed there was no way to log the username portion of the HTTP request. Username has been part of the Apache common log format, since forever (Common Log Format - Wikipedia), and is a part of the %{COMBINEDAPACHELOG} grok pattern used by many logging platforms.

So I set about to add the username for my middleware. Below is code and example. It works well.

However when I added a header directive to enable HSTS, it wipes out my custom placeholder. So
What can I do to remedy this? Am I going about this all wrong? Should header be overriding other ResponseRecorders?

Any suggestion are appreciated.

	// Check for HTTP Basic Authorization Headers and valid username, password
	username, password, ok := r.BasicAuth()

<snip some if !ok validation, if blank, etc.>

	// Capture username into {user} placeholder for caddyfile log directive
	// ex:  log / stdout "{remote} - {user} [{when}] {method} {uri} {proto} {status} {size}"
	if rr, ok := w.(*httpserver.ResponseRecorder); ok && rr.Replacer != nil {
		rr.Replacer.Set("user", username)
	}

#Caddyfile
log / /var/log/caddy/access.log "{remote} - {user} [{when}] \"{method} {uri} {proto}\" {status} {size} \"{>Referer}\" \"{>User-Agent}\""

# produces
127.0.0.1 - jim [24/Mar/2017:13:11:15 -0400] "GET / HTTP/2.0" 200 4272 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.98 Safari/537.36"

# with header directive enabled in Caddy file
# {user} has been overwritten?
127.0.0.1 - - [24/Mar/2017:13:11:15 -0400] "GET / HTTP/2.0" 200 4272 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.98 Safari/537.36"
2

Great.

I noted when fixing #1399 https://github.com/mholt/caddy/pull/1399 that we would ideally add in a {user} placeholder rather than a - . We should probably create the same placeholder {user} in basicauth at some point.

Could you show the exact Caddyfile that causes the overwrite?

Could it be caused by ordering? Is your plugin loaded before Header plugin? (I’m just grasping at straws here)

3

@tobya I concur. Capturing the username is key for log analysis. In addition {user} or whatever we call it should be added to {common} definition

In regards to my plugin ordering. It’s loading just after {basicauth}, and both of those are after {header}

Here is my testing Caddyfile, which is same structure as my production. I have to completely remove header section to get {user} to log correctly. Ordering of Caddyfile seems to make no difference. Even with {header} as last directive it overrides {user}. Username logging only works if {header} isn’t used at all.

https://localhost {

  # TLS certificates
  tls self_signed

  # Hardening headers
  header / {
    Strict-Transport-Security "max-age=60;"
    X-XSS-Protection "1; mode=block"
    X-Content-Type-Options "nosniff"
    X-Frame-Options "DENY"
}

  # Logging
  log / /tmp/access.log "{remote} - {user} [{when}] \"{method} {uri} {proto}\" {status} {size} \"{>Referer}\" \"{>User-Agent}\""
  errors /tmp/error.log

  radiusauth {
          server 127.0.0.1:1812
          secret testing123
          realm  "RADIUS Auth"
          except /public /img
          cache /tmp
          cachetimeout 60
          }

  root /var/www/hugo
  fastcgi / 127.0.0.1:9000 php
  proxy /cgit localhost:8080 {
  }
}
# redir port 80 to 443
:80 {
  redir https://localhost{uri}
}
```
4

@jim Can you post a link to the gihub repo where you have the changes?

5

@tobya sure. My plugin is here GitHub - jamesboswell/caddy-radius: RADIUS user authentication for Caddy

Hooking into Caddy is done as

diff --git a/caddy/caddymain/run.go b/caddy/caddymain/run.go
index fd193d5..f29f90b 100644
--- a/caddy/caddymain/run.go
+++ b/caddy/caddymain/run.go
@@ -21,6 +21,7 @@ import (

        "github.com/mholt/caddy/caddytls"
        // This is where other plugins get plugged in (imported)
+        _ "github.com/jamesboswell/caddy-radius"
 )

 func init() {
diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go
index 65d0a55..7e23928 100644
--- a/caddyhttp/httpserver/plugin.go
+++ b/caddyhttp/httpserver/plugin.go
@@ -463,6 +463,7 @@ var directives = []string{
        "search",    // github.com/pedronasser/caddy-search
        "expires",   // github.com/epicagency/caddy-expires
        "basicauth",
+        "radiusauth", // github.com/jamesboswell/caddy-radius
        "redir",
        "status",
        "cors", // github.com/captncraig/cors/caddy
6

@tobya the {user} is set here caddy-radius/radius.go at master · jamesboswell/caddy-radius · GitHub

7

I may be a million miles off here, but since I have no way to run and test your code I’ll make a suggestion to you.

The code for replacer says to always use NewReplacer to get one

https://github.com/mholt/caddy/blob/master/caddyhttp/httpserver/replacer.go#L30

however in your code you are retrieving from rr

Perhaps if you use NewReplace to get access to the replacer it might not overwrite user. There are a number of examples in the code. An example from rewrite

https://github.com/mholt/caddy/blob/master/caddyhttp/rewrite/rewrite.go#L158

or proxy

https://github.com/mholt/caddy/blob/master/caddyhttp/proxy/proxy.go#L100

If I’m way off sorry, but I though it was worth trying anyway.

8

@tobya that is very helpful! I’ll test it now. FYI if on a Mac, brew install freeradius for a local RADIUS server.

9

I have just submitted a PR to create the {user} placeholder in the main caddy repo.

https://github.com/mholt/caddy/pull/1542

If/when it is accepted you can update your code to use context to store the user value, similar to

https://github.com/mholt/caddy/blob/705694b081684d41a4968a2d1de4b282419ce955/caddyhttp/basicauth/basicauth.go#L66

10

@tobya your PR patch fixes my issue! Nice job! I should do some more testing tomorrow, but so far everything looks good.

I did open a GitHub issue on this for tracking
https://github.com/mholt/caddy/issues/1543

11

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.