Merge pull request #1006 from smallstep/max/revoke-serial-validation

Validate revocation serial number

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

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 a base 16 representation with '0x' prefix", r.Serial)
+	}
+	r.Serial = sn.String()
 	if r.ReasonCode < ocsp.Unspecified || r.ReasonCode > ocsp.AACompromise {
 		return errs.BadRequest("reasonCode out of bounds")
 	}

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 a base 16 representation with '0x' prefix"), 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",