forked from openprivacy/libricochet-go
Fix concurrency issues in ProcessAuthAsClient/Server
There were a few related issues with ProcessAuthAsClient/Server that could cause deadlocks or leak goroutines: Break() could end up being called more than once, which is always a deadlock. This is fixed by using a sync.Once. RequestOpenChannel was called without Do, and was called before Process in the same goroutine, which would have deadlocked if Do was used. Timing out the authentication attempt wouldn't directly abort Process(); it would only exit if the connection was closed somewhere else. It may be a good idea to change some of this to guarantee that ProcessAuthAsClient returns either an authenticated connection or closes the connection, but I'll leave that as a separate task for the moment.
This commit is contained in:
parent
e459a56286
commit
b2c87b1b72
|
@ -5,6 +5,7 @@ import (
|
|||
"github.com/s-rah/go-ricochet/channels"
|
||||
"github.com/s-rah/go-ricochet/policies"
|
||||
"github.com/s-rah/go-ricochet/utils"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// InboundConnectionHandler is a convieniance wrapper for handling inbound
|
||||
|
@ -39,6 +40,8 @@ func (ich *InboundConnectionHandler) ProcessAuthAsServer(privateKey *rsa.Private
|
|||
return utils.PrivateKeyNotSetError
|
||||
}
|
||||
|
||||
var breakOnce sync.Once
|
||||
|
||||
var authAllowed, authKnown bool
|
||||
var authHostname string
|
||||
|
||||
|
@ -47,12 +50,12 @@ func (ich *InboundConnectionHandler) ProcessAuthAsServer(privateKey *rsa.Private
|
|||
if authAllowed {
|
||||
authHostname = hostname
|
||||
}
|
||||
ich.connection.Break()
|
||||
breakOnce.Do(func() { go ich.connection.Break() })
|
||||
return authAllowed, authKnown
|
||||
}
|
||||
onAuthInvalid := func(err error) {
|
||||
// err is ignored at the moment
|
||||
ich.connection.Break()
|
||||
breakOnce.Do(func() { go ich.connection.Break() })
|
||||
}
|
||||
|
||||
ach := new(AutoConnectionHandler)
|
||||
|
@ -66,6 +69,9 @@ func (ich *InboundConnectionHandler) ProcessAuthAsServer(privateKey *rsa.Private
|
|||
}
|
||||
})
|
||||
|
||||
// Ensure that the call to Process() cannot outlive this function,
|
||||
// particularly for the case where the policy timeout expires
|
||||
defer breakOnce.Do(func() { ich.connection.Break() })
|
||||
policy := policies.UnknownPurposeTimeout
|
||||
err := policy.ExecuteAction(func() error {
|
||||
return ich.connection.Process(ach)
|
||||
|
|
|
@ -5,6 +5,7 @@ import (
|
|||
"github.com/s-rah/go-ricochet/channels"
|
||||
"github.com/s-rah/go-ricochet/policies"
|
||||
"github.com/s-rah/go-ricochet/utils"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// OutboundConnectionHandler is a convieniance wrapper for handling outbound
|
||||
|
@ -40,33 +41,50 @@ func (och *OutboundConnectionHandler) ProcessAuthAsClient(privateKey *rsa.Privat
|
|||
ach := new(AutoConnectionHandler)
|
||||
ach.Init()
|
||||
|
||||
// Make sure that calls to Break in this function cannot race
|
||||
var breakOnce sync.Once
|
||||
|
||||
var accepted, isKnownContact bool
|
||||
authCallback := func(accept, known bool) {
|
||||
accepted = accept
|
||||
isKnownContact = known
|
||||
// Cause the Process() call below to return
|
||||
och.connection.Break()
|
||||
// Cause the Process() call below to return.
|
||||
// If Break() is called from here, it _must_ use go, because this will
|
||||
// execute in the Process goroutine, and Break() will deadlock.
|
||||
breakOnce.Do(func() { go och.connection.Break() })
|
||||
}
|
||||
|
||||
_, err := och.connection.RequestOpenChannel("im.ricochet.auth.hidden-service",
|
||||
&channels.HiddenServiceAuthChannel{
|
||||
PrivateKey: privateKey,
|
||||
ServerHostname: och.connection.RemoteHostname,
|
||||
ClientAuthResult: authCallback,
|
||||
processResult := make(chan error, 1)
|
||||
go func() {
|
||||
// Break Process() if timed out; no-op if Process returned a conn error
|
||||
defer func() { breakOnce.Do(func() { och.connection.Break() }) }()
|
||||
policy := policies.UnknownPurposeTimeout
|
||||
err := policy.ExecuteAction(func() error {
|
||||
return och.connection.Process(ach)
|
||||
})
|
||||
processResult <- err
|
||||
}()
|
||||
|
||||
err := och.connection.Do(func() error {
|
||||
_, err := och.connection.RequestOpenChannel("im.ricochet.auth.hidden-service",
|
||||
&channels.HiddenServiceAuthChannel{
|
||||
PrivateKey: privateKey,
|
||||
ServerHostname: och.connection.RemoteHostname,
|
||||
ClientAuthResult: authCallback,
|
||||
})
|
||||
return err
|
||||
})
|
||||
if err != nil {
|
||||
breakOnce.Do(func() { och.connection.Break() })
|
||||
return false, err
|
||||
}
|
||||
|
||||
policy := policies.UnknownPurposeTimeout
|
||||
err = policy.ExecuteAction(func() error {
|
||||
return och.connection.Process(ach)
|
||||
})
|
||||
if err = <-processResult; err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
if err == nil {
|
||||
if accepted == true {
|
||||
return isKnownContact, nil
|
||||
}
|
||||
if accepted == true {
|
||||
return isKnownContact, nil
|
||||
}
|
||||
return false, utils.ServerRejectedClientConnectionError
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue