Immediately terminate improperly auth'd ws conns.

This fixes a bug with the authentication handling for websocket
clients where it was possible that even after supplying bad
authentication using the HTTP Authorization header, the connection
would remain open and flagged as unauthenticated, and clients (if they
somehow knew auth failed, although btcwallet would never tell them
until after they failed their next request) could try their hand at
authorization again by issuing an authenticate request.

While I don't know of any reason the above described bug could result
in a security leak, it's better to fail the connection as soon as
possible if they failed their first authentication attempt.

While here, also set a read deadline of 10 seconds for the first
request.  If the initial handshake cannot complete in this timeframe,
the connection is terminated.  This matches the behavior in btcd, and
prevents websocket clients from connecting without the Authorization
header and never issuing their first authenticate request.
This commit is contained in:
Josh Rickmar 2014-04-28 18:16:51 -05:00
parent 6a908d63bc
commit e956d0b290

View file

@ -47,6 +47,10 @@ var (
// a missing, incorrect, or duplicate authentication request. // a missing, incorrect, or duplicate authentication request.
ErrBadAuth = errors.New("bad auth") ErrBadAuth = errors.New("bad auth")
// ErrNoAuth represents an error where authentication could not succeed
// due to a missing Authorization HTTP header.
ErrNoAuth = errors.New("no auth")
// ErrConnRefused represents an error where a connection to another // ErrConnRefused represents an error where a connection to another
// process cannot be established. // process cannot be established.
ErrConnRefused = errors.New("connection refused") ErrConnRefused = errors.New("connection refused")
@ -403,6 +407,10 @@ out:
// forever (until disconnected), reading JSON-RPC requests and sending // forever (until disconnected), reading JSON-RPC requests and sending
// sending responses and notifications. // sending responses and notifications.
func (s *server) WSSendRecv(ws *websocket.Conn, remoteAddr string, authenticated bool) { func (s *server) WSSendRecv(ws *websocket.Conn, remoteAddr string, authenticated bool) {
// Clear the read deadline set before the websocket hijacked
// the connection.
ws.SetReadDeadline(time.Time{})
// Add client context so notifications duplicated to each // Add client context so notifications duplicated to each
// client are received by this client. // client are received by this client.
recvQuit := make(chan struct{}) recvQuit := make(chan struct{})
@ -546,7 +554,14 @@ func (s *server) Start() {
log.Trace("Starting RPC server") log.Trace("Starting RPC server")
serveMux := http.NewServeMux() serveMux := http.NewServeMux()
httpServer := &http.Server{Handler: serveMux} const rpcAuthTimeoutSeconds = 10
httpServer := &http.Server{
Handler: serveMux,
// Timeout connections which don't complete the initial
// handshake within the allowed timeframe.
ReadTimeout: time.Second * rpcAuthTimeoutSeconds,
}
serveMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { serveMux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
if err := s.checkAuth(r); err != nil { if err := s.checkAuth(r); err != nil {
log.Warnf("Unauthorized client connection attempt") log.Warnf("Unauthorized client connection attempt")
@ -557,7 +572,15 @@ func (s *server) Start() {
}) })
serveMux.HandleFunc("/frontend", func(w http.ResponseWriter, r *http.Request) { serveMux.HandleFunc("/frontend", func(w http.ResponseWriter, r *http.Request) {
authenticated := false authenticated := false
if err := s.checkAuth(r); err == nil { if err := s.checkAuth(r); err != nil {
// If auth was supplied but incorrect, rather than simply being
// missing, immediately terminate the connection.
if err != ErrNoAuth {
log.Warnf("Disconnecting improperly authorized websocket client")
http.Error(w, "401 Unauthorized.", http.StatusUnauthorized)
return
}
} else {
authenticated = true authenticated = true
} }
@ -590,8 +613,8 @@ func (s *server) Start() {
// This check is time-constant. // This check is time-constant.
func (s *server) checkAuth(r *http.Request) error { func (s *server) checkAuth(r *http.Request) error {
authhdr := r.Header["Authorization"] authhdr := r.Header["Authorization"]
if len(authhdr) <= 0 { if len(authhdr) == 0 {
return ErrBadAuth return ErrNoAuth
} }
authsha := sha256.Sum256([]byte(authhdr[0])) authsha := sha256.Sum256([]byte(authhdr[0]))