schema.VerifyPassword not reliable

[markostojanovic087]

I tried implementing basic authentication by using example given in basic-authentication. As soon as I changed test mem handlers to sql handlers

sqlHandler, err := sqlStorage.NewHandler(DB_DRIVER, DB_SOURCE, DB_TABLE_USER)

I started getting "Please provide proper credentials" response.

After long investigation and debug I figured out that the method schema.VerifyPassword seems to be problematic one. As soon as I used the following method instead of VerifyPassword I had no issue.

func MyVerifyPassword(hash interface{}, password []byte) bool { 
    h, ok := hash.(string)
    if !ok {
        return false
    }
    return bcrypt.CompareHashAndPassword([]byte(h), password) == nil
}

The calling code remained exactly the same, just instead of schema.VerifyPassword I called MyVerifyPassword.

// NewBasicAuthHandler handles basic HTTP auth against the provided user resource
func NewBasicAuthHandler(users *resource.Resource) func(next http.Handler) http.Handler {
    return func(next http.Handler) http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            if u, p, ok := r.BasicAuth(); ok {
                // Lookup the user by its id
                ctx := r.Context()
                user, err := users.Get(ctx, u)

                if user != nil && err == resource.ErrForbidden {
                    // Ignore unauthorized errors set by ourselves
                    err = nil
                }

                if err != nil {
                    // If user resource storage handler returned an error, respond with an error
                    if err == resource.ErrNotFound {
                        http.Error(w, "Invalid credential", http.StatusForbidden)
                    } else {
                        http.Error(w, err.Error(), http.StatusInternalServerError)
                    }
                    return
                }

                if MyVerifyPassword(user.Payload["password"], []byte(p)) {
                    // Store the auth user into the context for later use
                    r = r.WithContext(NewContextWithUser(ctx, user))
                    next.ServeHTTP(w, r)
                    return
                }
            }
            // Stop the middleware chain and return a 401 HTTP error
            w.Header().Set("WWW-Authenticate", `Basic realm="API"`)
            http.Error(w, "Please provide proper credentials", http.StatusUnauthorized)
        })
    }
}

[smyrman]

@markostojanovic087, are you open for doing a PR on this?

[markostojanovic087]

@smyrman Yes, I am open to do it!

[smyrman]

@markostojanovic087 -- what is your field type for the password hash in SQL? Have you tried to see if you get a []byte instead of string from your SQL driver when using e.g. bit[SIZE] or bytea column type?

[smyrman]

@markostojanovic087, from pg docs:

• the bytea type is returned as []byte

https://github.com/lib/pq/blob/9eb73efc1fcc404148b56765b0d3f61d9a5ef8ee/doc.go#L144

Using that field type, you can probably solve #247 and #248 without any code changes for your use-case.

[smyrman]

I am going to close this issue.

I view this currently as a documentation issue on the (third-party) SQL Storer backend. Feel free to raise it again there. I believe it to be fixable by altering the SQL schema to use a byre-array for storing the password.