Skip to content

Fix deadlock from taking db lock while holding authz lock in Authz() #564

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.

Sign up for GitHub

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

Open
wallrj-cyberark wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix deadlock from taking db lock while holding authz lock in Authz() #564

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions wfe/wfe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2356,29 +2356,39 @@ func (wfe *WebFrontEndImpl) Authz(
return
}

// Do the account and order lookups before locking the authz.
// MemoryStore.FindValidAuthorization and Order.GetStatus take the db
// lock, then the order lock, then the authz lock. Taking either of the
// first two while holding the authz lock deadlocks if a writer queues up.
var account *core.Account
if postData.postAsGet {
account, prob = wfe.validPOSTAsGET(postData)
} else {
account, prob = wfe.getAcctByKey(postData.jwk)
}
if prob != nil {
wfe.sendError(prob, response)
return
}
Comment thread
aarongable marked this conversation as resolved.
Resolved

authzID := strings.TrimPrefix(request.URL.Path, authzPath)
authz := wfe.db.GetAuthorizationByID(authzID)
if authz == nil {
response.WriteHeader(http.StatusNotFound)
return
}

authz.Lock()
defer authz.Unlock()
authz.Order.RLock()
orderAcctID := authz.Order.AccountID
authz.Order.RUnlock()

authz.Lock()
defer authz.Unlock()

// If the postData is not a POST-as-GET, treat this as case A) and update
// the authorization based on the postData
if !postData.postAsGet {
existingAcct, prob := wfe.getAcctByKey(postData.jwk)
if prob != nil {
wfe.sendError(prob, response)
return
}

if orderAcctID != existingAcct.ID {
if orderAcctID != account.ID {
wfe.sendError(acme.UnauthorizedProblem(
"Account does not own authorization"), response)
return
Expand Down Expand Up @@ -2406,12 +2416,6 @@ func (wfe *WebFrontEndImpl) Authz(
// Otherwise this was a POST-as-GET request and we need to verify it
// accordingly and ensure the authorized account owns the authorization
// being fetched.
account, prob := wfe.validPOSTAsGET(postData)
if prob != nil {
wfe.sendError(prob, response)
return
}

if orderAcctID != account.ID {
wfe.sendError(acme.UnauthorizedProblem(
"Account authorizing the request is not the owner of the authorization"),
Expand Down
Loading