From c365d8580e55ed5dd4835ecc4f3af79636c2cc96 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 13 Dec 2022 10:22:35 +0100 Subject: [PATCH] Move provisioner marshaling logic to api package --- api/api.go | 35 ++++++++- api/api_test.go | 95 ++++++++++++++++++++++- authority/provisioner/provisioner.go | 23 ------ authority/provisioner/provisioner_test.go | 88 --------------------- 4 files changed, 127 insertions(+), 114 deletions(-) diff --git a/api/api.go b/api/api.go index 9c2f1f31..0ac73317 100644 --- a/api/api.go +++ b/api/api.go @@ -224,8 +224,39 @@ type RootResponse struct { // ProvisionersResponse is the response object that returns the list of // provisioners. type ProvisionersResponse struct { - Provisioners provisioner.List `json:"provisioners"` - NextCursor string `json:"nextCursor"` + Provisioners provisioner.List + NextCursor string +} + +// MarshalJSON implements json.Marshaler. It marshals the ProvisionersResponse +// into a byte slice. +// +// Special treatment is given to the SCEP provisioner, as it contains a +// challenge secret that MUST NOT be leaked in (public) HTTP responses. The +// challenge value is thus redacted in HTTP responses. +func (p ProvisionersResponse) MarshalJSON() ([]byte, error) { + for _, item := range p.Provisioners { + scepProv, ok := item.(*provisioner.SCEP) + if !ok { + continue + } + + old := scepProv.ChallengePassword + scepProv.ChallengePassword = "*** REDACTED ***" + defer func(p string) { //nolint:gocritic // defer in loop required to restore initial state of provisioners + scepProv.ChallengePassword = p + }(old) + } + + var list = struct { + Provisioners []provisioner.Interface `json:"provisioners"` + NextCursor string `json:"nextCursor"` + }{ + Provisioners: []provisioner.Interface(p.Provisioners), + NextCursor: p.NextCursor, + } + + return json.Marshal(list) } // ProvisionerKeyResponse is the response object that returns the encrypted key diff --git a/api/api_test.go b/api/api_test.go index e24751b3..24e77c75 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -4,7 +4,7 @@ import ( "bytes" "context" "crypto" - "crypto/dsa" //nolint + "crypto/dsa" //nolint:staticcheck // support legacy algorithms "crypto/ecdsa" "crypto/ed25519" "crypto/elliptic" @@ -28,7 +28,9 @@ import ( "github.com/go-chi/chi" "github.com/pkg/errors" + sassert "github.com/stretchr/testify/assert" "golang.org/x/crypto/ssh" + squarejose "gopkg.in/square/go-jose.v2" "go.step.sm/crypto/jose" "go.step.sm/crypto/x509util" @@ -1564,3 +1566,94 @@ func mustCertificate(t *testing.T, pub, priv interface{}) *x509.Certificate { } return cert } + +func TestProvisionersResponse_MarshalJSON(t *testing.T) { + + k := map[string]any{ + "use": "sig", + "kty": "EC", + "kid": "4UELJx8e0aS9m0CH3fZ0EB7D5aUPICb759zALHFejvc", + "crv": "P-256", + "alg": "ES256", + "x": "7ZdAAMZCFU4XwgblI5RfZouBi8lYmF6DlZusNNnsbm8", + "y": "sQr2JdzwD2fgyrymBEXWsxDxFNjjqN64qLLSbLdLZ9Y", + } + key := squarejose.JSONWebKey{} + b, err := json.Marshal(k) + assert.FatalError(t, err) + err = json.Unmarshal(b, &key) + assert.FatalError(t, err) + + r := ProvisionersResponse{ + Provisioners: provisioner.List{ + &provisioner.SCEP{ + Name: "scep", + Type: "scep", + ChallengePassword: "not-so-secret", + MinimumPublicKeyLength: 2048, + EncryptionAlgorithmIdentifier: 2, + }, + &provisioner.JWK{ + EncryptedKey: "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", + Key: &key, + Name: "step-cli", + Type: "JWK", + }, + }, + NextCursor: "next", + } + + expected := map[string]any{ + "provisioners": []map[string]any{ + { + "type": "scep", + "name": "scep", + "challenge": "*** REDACTED ***", + "minimumPublicKeyLength": 2048, + "encryptionAlgorithmIdentifier": 2, + }, + { + "type": "JWK", + "name": "step-cli", + "key": map[string]any{ + "use": "sig", + "kty": "EC", + "kid": "4UELJx8e0aS9m0CH3fZ0EB7D5aUPICb759zALHFejvc", + "crv": "P-256", + "alg": "ES256", + "x": "7ZdAAMZCFU4XwgblI5RfZouBi8lYmF6DlZusNNnsbm8", + "y": "sQr2JdzwD2fgyrymBEXWsxDxFNjjqN64qLLSbLdLZ9Y", + }, + "encryptedKey": "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", + }, + }, + "nextCursor": "next", + } + + expBytes, err := json.Marshal(expected) + sassert.NoError(t, err) + + br, err := r.MarshalJSON() + sassert.NoError(t, err) + sassert.JSONEq(t, string(expBytes), string(br)) + + keyCopy := key + expList := provisioner.List{ + &provisioner.SCEP{ + Name: "scep", + Type: "scep", + ChallengePassword: "not-so-secret", + MinimumPublicKeyLength: 2048, + EncryptionAlgorithmIdentifier: 2, + }, + &provisioner.JWK{ + EncryptedKey: "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", + Key: &keyCopy, + Name: "step-cli", + Type: "JWK", + }, + } + + // MarshalJSON must not affect the struct properties itself + sassert.Equal(t, expList, r.Provisioners) +} diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index d14b39e1..f2e7e68f 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -235,29 +235,6 @@ type provisioner struct { // List represents a list of provisioners. type List []Interface -// MarshalJSON implements json.Marshaler. It marshals a List of Interfaces -// into a byte slice. -// -// Special treatment is given to the SCEP provisioner, as it contains a -// challenge secret that MUST NOT be leaked in public HTTP responses. The -// challenge value is redacted in HTTP responses. -func (l List) MarshalJSON() ([]byte, error) { - for _, item := range l { - scepProv, ok := item.(*SCEP) - if !ok { - continue - } - - old := scepProv.ChallengePassword - scepProv.ChallengePassword = "*** REDACTED ***" - defer func(p string) { - scepProv.ChallengePassword = p - }(old) - } - - return json.Marshal([]Interface(l)) -} - // UnmarshalJSON implements json.Unmarshaler and allows to unmarshal a list of a // interfaces into the right type. func (l *List) UnmarshalJSON(data []byte) error { diff --git a/authority/provisioner/provisioner_test.go b/authority/provisioner/provisioner_test.go index 2dea9376..65fb8e1d 100644 --- a/authority/provisioner/provisioner_test.go +++ b/authority/provisioner/provisioner_test.go @@ -2,14 +2,11 @@ package provisioner import ( "context" - "encoding/json" "errors" "net/http" "testing" - sassert "github.com/stretchr/testify/assert" "golang.org/x/crypto/ssh" - squarejose "gopkg.in/square/go-jose.v2" "github.com/smallstep/assert" "github.com/smallstep/certificates/api/render" @@ -252,88 +249,3 @@ func TestUnimplementedMethods(t *testing.T) { }) } } - -func TestList_MarshalJSON(t *testing.T) { - - k := map[string]any{ - "use": "sig", - "kty": "EC", - "kid": "4UELJx8e0aS9m0CH3fZ0EB7D5aUPICb759zALHFejvc", - "crv": "P-256", - "alg": "ES256", - "x": "7ZdAAMZCFU4XwgblI5RfZouBi8lYmF6DlZusNNnsbm8", - "y": "sQr2JdzwD2fgyrymBEXWsxDxFNjjqN64qLLSbLdLZ9Y", - } - key := squarejose.JSONWebKey{} - b, err := json.Marshal(k) - assert.FatalError(t, err) - err = json.Unmarshal(b, &key) - assert.FatalError(t, err) - - l := List{ - &SCEP{ - Name: "scep", - Type: "scep", - ChallengePassword: "not-so-secret", - MinimumPublicKeyLength: 2048, - EncryptionAlgorithmIdentifier: 0, - }, - &JWK{ - EncryptedKey: "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", - Key: &key, - Name: "step-cli", - Type: "JWK", - }, - } - - expected := []map[string]any{ - { - "type": "scep", - "name": "scep", - "challenge": "*** REDACTED ***", - "minimumPublicKeyLength": 2048, - }, - { - "type": "JWK", - "name": "step-cli", - "key": map[string]any{ - "use": "sig", - "kty": "EC", - "kid": "4UELJx8e0aS9m0CH3fZ0EB7D5aUPICb759zALHFejvc", - "crv": "P-256", - "alg": "ES256", - "x": "7ZdAAMZCFU4XwgblI5RfZouBi8lYmF6DlZusNNnsbm8", - "y": "sQr2JdzwD2fgyrymBEXWsxDxFNjjqN64qLLSbLdLZ9Y", - }, - "encryptedKey": "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", - }, - } - - expBytes, err := json.Marshal(expected) - sassert.NoError(t, err) - - bl, err := l.MarshalJSON() - sassert.NoError(t, err) - sassert.JSONEq(t, string(expBytes), string(bl)) - - keyCopy := key - expList := List{ - &SCEP{ - Name: "scep", - Type: "scep", - ChallengePassword: "not-so-secret", - MinimumPublicKeyLength: 2048, - EncryptionAlgorithmIdentifier: 0, - }, - &JWK{ - EncryptedKey: "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", - Key: &keyCopy, - Name: "step-cli", - Type: "JWK", - }, - } - - // MarshalJSON must not affect the struct properties itself - sassert.Equal(t, expList, l) - -}