From 7052a32c2c5c51e8d17e0a97a2295dd947dd6aeb Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Tue, 9 Aug 2022 11:04:00 -0700
Subject: [PATCH] Validate revocation serial number

---
 CHANGELOG.md       |  1 +
 api/revoke.go      |  6 ++++++
 api/revoke_test.go | 20 ++++++++++++--------
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 38b72822..4f564c21 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Changed
 - Certificates signed by an issuer using an RSA key will be signed using the same algorithm as the issuer certificate was signed with. The signature will no longer default to PKCS #1. For example, if the issuer certificate was signed using RSA-PSS with SHA-256, a new certificate will also be signed using RSA-PSS with SHA-256.
 - Support two latest versions of Go (1.18, 1.19)
+- Vadlidate revocation serial number (either base 10 or prefixed with an appropriate base)
 
 ## [0.20.0] - 2022-05-26
 ### Added
diff --git a/api/revoke.go b/api/revoke.go
index aebbb875..4f8ce6b6 100644
--- a/api/revoke.go
+++ b/api/revoke.go
@@ -1,6 +1,7 @@
 package api
 
 import (
+	"math/big"
 	"net/http"
 
 	"golang.org/x/crypto/ocsp"
@@ -33,6 +34,11 @@ func (r *RevokeRequest) Validate() (err error) {
 	if r.Serial == "" {
 		return errs.BadRequest("missing serial")
 	}
+	sn, ok := new(big.Int).SetString(r.Serial, 0)
+	if !ok {
+		return errs.BadRequest("'%s' is not a valid serial number - use a base 10 representation or add a prefix indicating the base", r.Serial)
+	}
+	r.Serial = sn.String()
 	if r.ReasonCode < ocsp.Unspecified || r.ReasonCode > ocsp.AACompromise {
 		return errs.BadRequest("reasonCode out of bounds")
 	}
diff --git a/api/revoke_test.go b/api/revoke_test.go
index c3fa6ceb..74293d50 100644
--- a/api/revoke_test.go
+++ b/api/revoke_test.go
@@ -31,9 +31,13 @@ func TestRevokeRequestValidate(t *testing.T) {
 			rr:  &RevokeRequest{},
 			err: &errs.Error{Err: errors.New("missing serial"), Status: http.StatusBadRequest},
 		},
+		"error/bad sn": {
+			rr:  &RevokeRequest{Serial: "sn"},
+			err: &errs.Error{Err: errors.New("'sn' is not a valid serial number - use a base 10 representation or add a prefix indicating the base"), Status: http.StatusBadRequest},
+		},
 		"error/bad reasonCode": {
 			rr: &RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 15,
 				Passive:    true,
 			},
@@ -41,7 +45,7 @@ func TestRevokeRequestValidate(t *testing.T) {
 		},
 		"error/non-passive not implemented": {
 			rr: &RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 8,
 				Passive:    false,
 			},
@@ -49,7 +53,7 @@ func TestRevokeRequestValidate(t *testing.T) {
 		},
 		"ok": {
 			rr: &RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 9,
 				Passive:    true,
 			},
@@ -97,7 +101,7 @@ func Test_caHandler_Revoke(t *testing.T) {
 		},
 		"200/ott": func(t *testing.T) test {
 			input, err := json.Marshal(RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 4,
 				Reason:     "foo",
 				OTT:        "valid",
@@ -114,7 +118,7 @@ func Test_caHandler_Revoke(t *testing.T) {
 					revoke: func(ctx context.Context, opts *authority.RevokeOptions) error {
 						assert.True(t, opts.PassiveOnly)
 						assert.False(t, opts.MTLS)
-						assert.Equals(t, opts.Serial, "sn")
+						assert.Equals(t, opts.Serial, "10")
 						assert.Equals(t, opts.ReasonCode, 4)
 						assert.Equals(t, opts.Reason, "foo")
 						return nil
@@ -125,7 +129,7 @@ func Test_caHandler_Revoke(t *testing.T) {
 		},
 		"400/no OTT and no peer certificate": func(t *testing.T) test {
 			input, err := json.Marshal(RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 4,
 				Passive:    true,
 			})
@@ -176,7 +180,7 @@ func Test_caHandler_Revoke(t *testing.T) {
 		},
 		"500/ott authority.Revoke": func(t *testing.T) test {
 			input, err := json.Marshal(RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 4,
 				Reason:     "foo",
 				OTT:        "valid",
@@ -198,7 +202,7 @@ func Test_caHandler_Revoke(t *testing.T) {
 		},
 		"403/ott authority.Revoke": func(t *testing.T) test {
 			input, err := json.Marshal(RevokeRequest{
-				Serial:     "sn",
+				Serial:     "10",
 				ReasonCode: 4,
 				Reason:     "foo",
 				OTT:        "valid",