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

HTTP Header Proxy-Authenticate can be conditionally forwarded. #7374

Closed
dakriy opened this issue Oct 1, 2020 · 4 comments · Fixed by #7433
Closed

HTTP Header Proxy-Authenticate can be conditionally forwarded. #7374

dakriy opened this issue Oct 1, 2020 · 4 comments · Fixed by #7433

Comments

@dakriy
Copy link

dakriy commented Oct 1, 2020

Do you want to request a feature or report a bug?

Feature

What did you expect to see?

When using the forwardAuth middleware and a request using the Proxy-Authenticate header, I expect the header to be forwarded to the authentication server.

Justification

Let me start by saying I am aware that Proxy-Authenticate is a hop by hop header and should normally be stripped according to RFC 2616. However, RFC 7235 extends on authentication headers. In section 4.4 it states:

   Unlike Authorization, the Proxy-Authorization header field applies
   only to the next inbound proxy that demanded authentication using the
   Proxy-Authenticate field.  When multiple proxies are used in a chain,
   the Proxy-Authorization header field is consumed by the first inbound
   proxy that was expecting to receive credentials.  A proxy MAY relay
   the credentials from the client request to the next proxy if that is
   the mechanism by which the proxies cooperatively authenticate a given
   request.

Since the forward auth middleware is delegating authentication for the proxy, the Proxy-Authenticate header can be sent along freeing up the Authorization header for applications behind the proxy that also want authentication. This would be Traefik's way of consuming the header. By optionally allowing the header to be passed, it would also allow for proxy chaining with proxy authentication and would also not not hog the application Authorization header.

@Scapal
Copy link
Contributor

Scapal commented Oct 5, 2020

I'm bumping into the same problem when using Traefik forwardAuth with Authelia.

That header is stripped by calling utils.RemoveHeaders(forwardReq.Header, forward.HopHeaders...).

var HopHeaders = []string{
	Connection,
	KeepAlive,
	ProxyAuthenticate,
	ProxyAuthorization,
	Te, // canonicalized version of "TE"
	Trailers,
	TransferEncoding,
	Upgrade,
}

Since the forward Auth is not considered a next hop but is actually part of the auth process of the current hop, the Proxy-Authorization header should not be removed.

utils.RemoveHeaders(forwardReq.Header, forward.HopHeaders...)

@Scapal
Copy link
Contributor

Scapal commented Oct 8, 2020

@nkonev I saw your commit for authRequestHeaders.

Could you do the removal of HopHeaders in filterForwardRequestHeaders so it allows to keep Proxy-Authentication if listed in authRequestHeaders ?

func filterForwardRequestHeaders(forwardRequestHeaders http.Header, allowedHeaders []string) http.Header {
	if len(allowedHeaders) == 0 {
		return utils.RemoveHeaders(forwardRequestHeaders, forward.HopHeaders...)
	}

A better approach would also be to not consider Proxy-Authentication as a Hop Header for the Authentication since it is actually supposed to perform that authentication.

func keepProxyAuthHeader(hopHeaders []string) []string {
	var headers []string
	for _, h := range hopHeaders {
		if h != forward.ProxyAuthorization {
			headers = append(headers, h)
		}
	}
	return headers
}
func writeHeader(req, forwardReq *http.Request, trustForwardHeader bool, allowedHeaders []string) {
	utils.CopyHeaders(forwardReq.Header, req.Header)
       // utils.RemoveHeaders(forwardReq.Header, forward.HopHeaders...)
	forwardReq.Header = filterForwardRequestHeaders(forwardReq.Header, allowedHeaders)
func filterForwardRequestHeaders(forwardRequestHeaders http.Header, allowedHeaders []string) http.Header {
	if len(allowedHeaders) == 0 {
		return utils.RemoveHeaders(forwardReq.Header, keepProxyAuthHeader(forward.HopHeaders)...)
	}

@nkonev
Copy link
Contributor

nkonev commented Oct 8, 2020

@Scapal I am not a part of traefik team, I just solved my problem with an excess Content-Type header.

I suggest you to make a PR with changing of hop header's processing.

@traefiker
Copy link
Contributor

Closed by #7433.

@kevinpollet kevinpollet added this to issues in v2 via automation Jan 22, 2021
@SantoDE SantoDE added kind/enhancement a new or improved feature. and removed kind/proposal a proposal that needs to be discussed. labels Jan 22, 2021
@kevinpollet kevinpollet added kind/enhancement a new or improved feature. and removed kind/enhancement a new or improved feature. labels Jan 22, 2021
@traefik traefik locked and limited conversation to collaborators Feb 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
v2
issues
Development

Successfully merging a pull request may close this issue.

7 participants