From deacbdc3585be4c802739b36e8d10365d2139243 Mon Sep 17 00:00:00 2001
From: David Cowden <dcow@smallstep.com>
Date: Wed, 13 May 2020 20:06:50 -0700
Subject: [PATCH] acme: Don't panic on logic errors

Since it will ultimately 500 anyway, just return an error.
---
 acme/authority.go | 18 +++++++++++++-----
 acme/challenge.go | 20 +++++++++++++-------
 2 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/acme/authority.go b/acme/authority.go
index 471b1bca..1863b48f 100644
--- a/acme/authority.go
+++ b/acme/authority.go
@@ -5,6 +5,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
+	"log"
 	"net"
 	"net/http"
 	"net/url"
@@ -306,7 +307,8 @@ func (a *Authority) ValidateChallenge(p provisioner.Interface, accID, chID strin
 	case StatusInvalid, StatusValid:
 		return ch.toACME(a.dir, p)
 	default:
-		panic("unknown challenge state: " + ch.getStatus())
+		e:= errors.Errorf("unknown challenge state: %s", ch.getStatus())
+		return nil, ServerInternalErr(e)
 	}
 
 	// Validate the challenge belongs to the account owned by the requester.
@@ -352,7 +354,8 @@ func (a *Authority) ValidateChallenge(p provisioner.Interface, accID, chID strin
 			})
 		}
 	default:
-		panic("post-validation challenge in unexpected state" + ch.getStatus())
+		e := errors.Errorf("post-validation challenge in unexpected state, %s", ch.getStatus())
+		return nil, ServerInternalErr(e)
 	}
 	return ch.toACME(a.dir, p)
 }
@@ -388,13 +391,17 @@ func (a *Authority) RetryChallenge(chID string) {
 	}
 	switch ch.getStatus() {
 	case StatusPending:
-		panic("pending challenges must first be moved to the processing state")
+		e := errors.New("pending challenges must first be moved to the processing state")
+		log.Printf("%v", e)
+		return
 	case StatusInvalid, StatusValid:
 		return
 	case StatusProcessing:
 		break
 	default:
-		panic("unknown challenge state: " + ch.getStatus())
+		e:= errors.Errorf("unknown challenge state: %s", ch.getStatus())
+		log.Printf("%v", e)
+		return
 	}
 
 	// When retrying, check to make sure the ordinal has not changed.
@@ -449,7 +456,8 @@ func (a *Authority) RetryChallenge(chID string) {
 			})
 		}
 	default:
-		panic("post-validation challenge in unexpected state " + ch.getStatus())
+		e := errors.Errorf("post-validation challenge in unexpected state, %s", ch.getStatus())
+		log.Printf("%v", e)
 	}
 }
 
diff --git a/acme/challenge.go b/acme/challenge.go
index b87f4ee2..3ee27af8 100644
--- a/acme/challenge.go
+++ b/acme/challenge.go
@@ -302,7 +302,7 @@ func (bc *baseChallenge) morph() challenge {
 	case "tls-alpn-01":
 		return &tlsALPN01Challenge{bc}
 	default:
-		panic("unrecognized challenge type: " + bc.getType())
+		return bc
 	}
 }
 
@@ -349,13 +349,15 @@ func (hc *http01Challenge) validate(jwk *jose.JSONWebKey, vo validateOptions) (c
 	// If already valid or invalid then return without performing validation.
 	switch hc.getStatus() {
 	case StatusPending:
-		panic("pending challenges must first be moved to the processing state")
+		e := errors.New("pending challenges must first be moved to the processing state")
+		return nil, ServerInternalErr(e)
 	case StatusProcessing:
 		break
 	case StatusValid, StatusInvalid:
 		return hc, nil
 	default:
-		panic("unknown challenge state: " + hc.getStatus())
+		e := errors.Errorf("unknown challenge state: %s", hc.getStatus())
+		return nil, ServerInternalErr(e)
 	}
 
 	up := &http01Challenge{hc.baseChallenge.clone()}
@@ -426,13 +428,15 @@ func (tc *tlsALPN01Challenge) validate(jwk *jose.JSONWebKey, vo validateOptions)
 	// If already valid or invalid then return without performing validation.
 	switch tc.getStatus() {
 	case StatusPending:
-		panic("pending challenges must first be moved to the processing state")
+		e := errors.New("pending challenges must first be moved to the processing state")
+		return nil, ServerInternalErr(e)
 	case StatusProcessing:
 		break
 	case StatusValid, StatusInvalid:
 		return tc, nil
 	default:
-		panic("unknown challenge state: " + tc.getStatus())
+		e := errors.Errorf("unknown challenge state: %s", tc.getStatus())
+		return nil, ServerInternalErr(e)
 	}
 
 	up := &tlsALPN01Challenge{tc.baseChallenge.clone()}
@@ -565,13 +569,15 @@ func (dc *dns01Challenge) validate(jwk *jose.JSONWebKey, vo validateOptions) (ch
 	// If already valid or invalid then return without performing validation.
 	switch dc.getStatus() {
 	case StatusPending:
-		panic("pending challenges must first be moved to the processing state")
+		e := errors.New("pending challenges must first be moved to the processing state")
+		return nil, ServerInternalErr(e)
 	case StatusProcessing:
 		break
 	case StatusValid, StatusInvalid:
 		return dc, nil
 	default:
-		panic("unknown challenge state: " + dc.getStatus())
+		e := errors.Errorf("unknown challenge state: %s", dc.getStatus())
+		return nil, ServerInternalErr(e)
 	}
 
 	up := &dns01Challenge{dc.baseChallenge.clone()}