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

net/http: add digest access authentication to Transport #29409

Open
Baozisoftware opened this issue Dec 24, 2018 · 6 comments
Open

net/http: add digest access authentication to Transport #29409

Baozisoftware opened this issue Dec 24, 2018 · 6 comments
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Milestone

Comments

@Baozisoftware
Copy link

I tried to add Digest access authentication support in http.Transport today. currently it is available for proxy servers. (compatible with basic auth,but not tested.)
I hope the official can integrate it. After all, this is a base library.
Reference: https://github.com/delphinus/go-digest-request

Mainly modified:
Transport.roundTrip
Transport.dialConn

package http

type Transport struct {
	//...
	// digest auth fields
	nonceCount nonceCount
	authParts  map[string]string
	needAuth   bool
	basicAuth  bool
}

const nonce = "nonce"
const qop = "qop"
const realm = "realm"
const proxyAuthenticate = "Proxy-Authenticate"
const proxyAuthorization = "Proxy-Authorization"

var digestAuthHeanderswanted = []string{nonce, qop, realm}

func getRandomString(l int) string {
	str := "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
	bytes := []byte(str)
	var result []byte
	lstr := len(str) - 1
	for i := 0; i < l; i++ {
		n := getRandomInt(0, lstr)
		result = append(result, bytes[n])
	}
	return string(result)
}

var r = rand.New(rand.NewSource(time.Now().UnixNano()))

func getRandomInt(min, max int) int {
	sub := max - min + 1
	if sub <= 1 {
		return min
	}
	return min + r.Intn(sub)
}

func (t *Transport) makeAuthorization(proxy *url.URL, req *Request, parts map[string]string) string {
	username, password := "", ""
	if u := proxy.User; u != nil {
		username = u.Username()
		password, _ = u.Password()
	}
	ha1 := getMD5([]string{username, parts[realm], password})
	ha2 := getMD5([]string{req.Method, req.URL.String()})
	cnonce := getRandomString(16)
	nc := t.getNonceCount()
	response := getMD5([]string{
		ha1,
		parts[nonce],
		nc,
		cnonce,
		parts[qop],
		ha2,
	})
	return fmt.Sprintf(
		`Digest username="%s", realm="%s", nonce="%s", uri="%s", qop=%s, nc=%s, cnonce="%s", response="%s"`,
		username,
		parts[realm],
		parts[nonce],
		req.URL.String(),
		parts[qop],
		nc,
		cnonce,
		response,
	)
}

func makeParts(resp *Response) (map[string]string, error) {
	headers := strings.Split(resp.Header[proxyAuthenticate][0], ",")
	parts := make(map[string]string, len(digestAuthHeanderswanted))
	for _, r := range headers {
		for _, w := range digestAuthHeanderswanted {
			if strings.Contains(r, w) {
				parts[w] = strings.Split(r, `"`)[1]
			}
		}
	}

	if len(parts) != len(digestAuthHeanderswanted) {
		return nil, fmt.Errorf("header is invalid: %+v", parts)
	}

	return parts, nil
}

type nonceCount int

func (nc nonceCount) String() string {
	c := int(nc)
	return fmt.Sprintf("%08x", c)
}

func getMD5(texts []string) string {
	h := md5.New()
	_, _ = io.WriteString(h, strings.Join(texts, ":"))
	return hex.EncodeToString(h.Sum(nil))
}

func (t *Transport) getNonceCount() string {
	t.nonceCount++
	return t.nonceCount.String()
}

func (t *Transport) roundTrip(req *Request) (*Response, error) {
	//...
	isHTTP := scheme == "http" || scheme == "https"
	if isHTTP {
		if scheme == "http" && t.needAuth && !t.basicAuth {
			p, err := t.Proxy(req)
			if err == nil {
				auth := t.makeAuthorization(p, req, t.authParts)
				req.Header.Add(proxyAuthorization, auth)
			}
		}
		//...
	}
	//...
	for {
		//...
		if err == nil {
			if resp.StatusCode == 407 && req.URL.Scheme == "http" {
				if strings.HasPrefix(resp.Header[proxyAuthenticate][0], "Basic") {
					t.basicAuth = true
				} else {
					t.basicAuth = false
					t.authParts, err = makeParts(resp)
					if err != nil {
						return nil, err
					}
				}
				t.needAuth = true
				return t.roundTrip(req)
			}
			return resp, nil
		}
		//...
	}
}

func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistConn, error) {
	// Proxy setup.
	switch {
	//...
	case cm.targetScheme == "http":
		pconn.isProxy = true
		if t.needAuth && t.basicAuth {
			if pa := cm.proxyAuth(); pa != "" {
				pconn.mutateHeaderFunc = func(h Header) {
					h.Set(proxyAuthorization, pa)
				}
			}
		}
	case cm.targetScheme == "https":
		conn := pconn.conn
		hdr := t.ProxyConnectHeader
		if hdr == nil {
			hdr = make(Header)
		}
		connectReq := &Request{
			Method: "CONNECT",
			URL:    &url.URL{Opaque: cm.targetAddr},
			Host:   cm.targetAddr,
			Header: hdr,
		}

		if t.needAuth {
			auth := ""
			if t.basicAuth {
				if pa := cm.proxyAuth(); pa != "" {
					auth = pa
				}
			} else {
				auth = t.makeAuthorization(cm.proxyURL, connectReq, t.authParts)
			}
			connectReq.Header.Add(proxyAuthorization, auth)
		}

		connectReq.Write(conn)

		br := bufio.NewReader(conn)
		resp, err := ReadResponse(br, connectReq)
		if err != nil {
			conn.Close()
			return nil, err
		}
		if resp.StatusCode != 200 {
			if resp.StatusCode == 407 {
				t.authParts, err = makeParts(resp)
				if err != nil {
					return nil, err
				}
				t.needAuth = true
				return t.dialConn(ctx, cm)
			}
			//...
		}
	}
	//...
}
@Baozisoftware Baozisoftware changed the title Tay add Digest access authentication to http.Transport Try add Digest access authentication to http.Transport Dec 24, 2018
@Baozisoftware Baozisoftware changed the title Try add Digest access authentication to http.Transport Try add digest access authentication to http.Transport Dec 24, 2018
@odeke-em odeke-em changed the title Try add digest access authentication to http.Transport proposal: net/http: add digest access authentication to Transport Dec 31, 2018
@gopherbot gopherbot added this to the Proposal milestone Dec 31, 2018
@odeke-em
Copy link
Member

Thank you @Baozisoftware for filing this request and welcome to the Go project!

I'll page some experts @bradfitz @FiloSottile @agl.

@bradfitz
Copy link
Contributor

bradfitz commented Jan 2, 2019

I'm not opposed. I would use this functionality myself. (I have code in a number of places to do this by hand, which gets tedious.)

Please start by proposing a concrete API. Once we like the API we can then move on to reviewing code.

@bradfitz bradfitz changed the title proposal: net/http: add digest access authentication to Transport net/http: add digest access authentication to Transport Jan 2, 2019
@bradfitz bradfitz modified the milestones: Proposal, Unplanned Jan 2, 2019
@bradfitz bradfitz added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jan 2, 2019
@icholy
Copy link

icholy commented Jan 8, 2019

The best third party library I've found is https://github.com/bobziuchkovski/digest

edit: I ended up implementing my own package which re-uses challenges for the same domain. https://github.com/icholy/digest

@yaazkal
Copy link

yaazkal commented Nov 27, 2019

Hi, as I've exposed in a telegraf issue I'll like to also suggest the support for Digest authentication in http.

Let me quote:

The Digest authentication scheme [...] is intended as a replacement for Basic authentication and nothing more. as seen on rfc2617 section 3.1.4

and

The protocol referred to as "HTTP/1.0" includes the specification for a Basic Access Authentication scheme[1]. That scheme is not considered to be a secure method of user authentication, as the user name and password are passed over the network in an unencrypted form. This section provides the specification for a scheme that does not send the password in cleartext, referred to as "Digest Access Authentication". as seen on rfc2617 section 3.1.1

Take in mind that many companies use Digest auth by default instead of basic auth because of the added security features. Even Digest is not perfect or the best, it's at least a step forward to basic auth.

Thanks !

@rolandshoemaker
Copy link
Member

(This ticket has the Proposal-Accepted label but it looks like there wasn't ever an API proposed?)

A rough proposed API that extends http.Request and http.Response to provide client/server support for Digest Authentication:

// DigestChallenge contains the fields sent in an Authentication
// digest-challenge header.
type DigestChallenge struct {
    Realm      string
    Domain     string
    Nonce      string
    Opaque     string
    Stale      bool
    Algorithm  string
    QOPOptions []string
}

// RequestDigestAuth sets the request's Authorization header to request
// HTTP Digest Authentication using the provided DigestChallenge.
func (r *Response) SetDigestAuth(challenge DigestChallenge)

// DigestAuth returns the digest-challenge fields in the response's
// Authorization header, if the request uses HTTP Digest Authentication.
func (r *Response) DigestAuth() (challenge DigestChallenge, ok bool) // ???

// DigestResponse contains the fields sent in an Authentication
// digest-response header.
type DigestResponse struct {
    Username   string
    Realm      string
    Nonce      string
    URI        string
    Response   string
    Algorithm  string
    CNonce     string
    Opaque     string
    QOP        string
    NonceCount int
}

// DigestAuth returns the digest-response fields in the request's Authorization
// header, if the request uses HTTP Digest Authentication.
func (r *Request) DigestAuth() (response DigestResponse, ok bool) // ???

// SetDigestAuth sets the request's Authorization header to use HTTP Digest
// Authentication based on the provided password and DigestChallenge from
// a previous HTTP response.
func (r *Request) SetDigestAuth(password string, challenge DigestChallenge)

@ncruces
Copy link
Contributor

ncruces commented May 13, 2020

I see two separate concerns here, which I feel should be addressed separately: request authentication (the 401 challenge/response), and proxy authentication (the 407 challenge/response).

Disclosure: I'm more interested in the latter, having had to implement it. This may color my views bellow.

Request authentication

I haven't had to implement this, so please correct me if I'm wrong.

Digest authentication for requests:

  • requires API changes mentioned by @bradfitz, fleshed out by @rolandshoemaker
  • tedious, but the standard library provides a clear path for implementation in a library:
    • a http.RoundTripper that wraps http.Transport should be enough to do it?
  • inferior to Basic authentication over TLS
    • Digest authentication protects the credentials but not the request/response
  • AFAIK, Go does not implement this in the standard library for any a authentication scheme

Proxy authentication

Digest authentication for proxies:

  • does not require any API changes
  • http.Transport already implements Basic authentication for proxies
    • current implementation needlessly leaks credentials in cleartext by default (more below)
  • if the proxy is accessed over HTTP, all you're protecting is the credentials
    • Digest authentication is useful to protect those
  • it's challenging to implement this on top of http.Transport while retaining the full benefits of connection caching
    • a http.RoundTripper wrapping http.Transport does not get to see the 407 response headers

As I've said, I had to implement this myself, github.com/ncruces/go-proxied is my tentative library solution. Comments welcome.

Having had to implement this, I personally feel the standard library is the correct place to do it. Some proxies that do not currently work, would suddenly begin to work, and that would be the only user visible change (also, their credentials would not leak as easily).

Particularly concerning to me was that, for HTTP proxies, Go leaks user credentials in the clear by issuing Authorization: Basic without any challenge. This means that even if an HTTP proxy explicitly does not support Basic authentication to try to protect user credentials, they get sent in the clear all the same.

TLDR

If there's any interest in Digest authentication for proxies, I would be willing to clean up and improve my implementation (there's work to do), and adapt it for inclusion in the standard library.

I have the following questions/concerns regarding implementation:

  • I would want to avoid preemptively sending Authorization: Basic over HTTP proxies. This is a change of behaviour. So for HTTP proxies I would not be sending credentials in clear text (in the initiating request) without being explicitly asked to do so by the proxy. Basic authentication would still work, but would require (at least) two requests.

  • Parsing of Proxy-Authenticate and generation of Proxy-Authorization would be useful to anyone implementing WWW-Authenticate/Authorization for requests. Should an API to do this be added to golang.org/x/net/http?

Comments, suggestions? Should I go ahead with this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

8 participants