From f2e1c56c6cd9b524bca283d3759dcb2610712d01 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 13 Dec 2022 09:33:31 +0100 Subject: [PATCH 1/4] Improve SCEP provisioner marshaling --- authority/provisioner/provisioner.go | 26 +++++- authority/provisioner/provisioner_test.go | 88 +++++++++++++++++++++ authority/provisioner/scep.go | 7 +- authority/provisioners_test.go | 20 +++-- ca/testdata/rsaca.json | 47 +++++++++++ ca/testdata/secrets/rsa_intermediate_ca.crt | 30 +++++++ ca/testdata/secrets/rsa_intermediate_ca_key | 54 +++++++++++++ ca/testdata/secrets/rsa_root_ca.crt | 29 +++++++ ca/testdata/secrets/rsa_root_ca_key | 54 +++++++++++++ 9 files changed, 342 insertions(+), 13 deletions(-) create mode 100644 ca/testdata/rsaca.json create mode 100644 ca/testdata/secrets/rsa_intermediate_ca.crt create mode 100644 ca/testdata/secrets/rsa_intermediate_ca_key create mode 100644 ca/testdata/secrets/rsa_root_ca.crt create mode 100644 ca/testdata/secrets/rsa_root_ca_key diff --git a/authority/provisioner/provisioner.go b/authority/provisioner/provisioner.go index 9d65d585..d14b39e1 100644 --- a/authority/provisioner/provisioner.go +++ b/authority/provisioner/provisioner.go @@ -10,8 +10,9 @@ import ( "strings" "github.com/pkg/errors" - "github.com/smallstep/certificates/errs" "golang.org/x/crypto/ssh" + + "github.com/smallstep/certificates/errs" ) // Interface is the interface that all provisioner types must implement. @@ -234,6 +235,29 @@ 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 65fb8e1d..2dea9376 100644 --- a/authority/provisioner/provisioner_test.go +++ b/authority/provisioner/provisioner_test.go @@ -2,11 +2,14 @@ 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" @@ -249,3 +252,88 @@ 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) + +} diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 0f27b206..3f8fb5a2 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -33,7 +33,6 @@ type SCEP struct { Options *Options `json:"options,omitempty"` Claims *Claims `json:"claims,omitempty"` ctl *Controller - secretChallengePassword string encryptionAlgorithm int } @@ -91,10 +90,6 @@ func (s *SCEP) Init(config Config) (err error) { return errors.New("provisioner name cannot be empty") } - // Mask the actual challenge value, so it won't be marshaled - s.secretChallengePassword = s.ChallengePassword - s.ChallengePassword = "*** redacted ***" - // Default to 2048 bits minimum public key length (for CSRs) if not set if s.MinimumPublicKeyLength == 0 { s.MinimumPublicKeyLength = 2048 @@ -135,7 +130,7 @@ func (s *SCEP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e // GetChallengePassword returns the challenge password func (s *SCEP) GetChallengePassword() string { - return s.secretChallengePassword + return s.ChallengePassword } // GetCapabilities returns the CA capabilities diff --git a/authority/provisioners_test.go b/authority/provisioners_test.go index 7901de6a..b4eb1bf9 100644 --- a/authority/provisioners_test.go +++ b/authority/provisioners_test.go @@ -9,14 +9,15 @@ import ( "testing" "time" + "go.step.sm/crypto/jose" + "go.step.sm/crypto/keyutil" + "go.step.sm/linkedca" + "github.com/smallstep/assert" "github.com/smallstep/certificates/api/render" "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/authority/provisioner" "github.com/smallstep/certificates/db" - "go.step.sm/crypto/jose" - "go.step.sm/crypto/keyutil" - "go.step.sm/linkedca" ) func TestGetEncryptedKey(t *testing.T) { @@ -100,6 +101,13 @@ func TestGetProvisioners(t *testing.T) { assert.FatalError(t, err) return &gp{a: a} }, + "ok/rsa": func(t *testing.T) *gp { + c, err := LoadConfiguration("../ca/testdata/rsaca.json") + assert.FatalError(t, err) + a, err := New(c) + assert.FatalError(t, err) + return &gp{a: a} + }, } for name, genTestCase := range tests { @@ -111,13 +119,13 @@ func TestGetProvisioners(t *testing.T) { if assert.NotNil(t, tc.err) { var sc render.StatusCodedError if assert.True(t, errors.As(err, &sc), "error does not implement StatusCodedError interface") { - assert.Equals(t, sc.StatusCode(), tc.code) + assert.Equals(t, tc.code, sc.StatusCode()) } - assert.HasPrefix(t, err.Error(), tc.err.Error()) + assert.HasPrefix(t, tc.err.Error(), err.Error()) } } else { if assert.Nil(t, tc.err) { - assert.Equals(t, ps, tc.a.config.AuthorityConfig.Provisioners) + assert.Equals(t, tc.a.config.AuthorityConfig.Provisioners, ps) assert.Equals(t, "", next) } } diff --git a/ca/testdata/rsaca.json b/ca/testdata/rsaca.json new file mode 100644 index 00000000..2e3acdb1 --- /dev/null +++ b/ca/testdata/rsaca.json @@ -0,0 +1,47 @@ +{ + "root": "../ca/testdata/secrets/rsa_root_ca.crt", + "federatedRoots": [], + "crt": "../ca/testdata/secrets/rsa_intermediate_ca.crt", + "key": "../ca/testdata/secrets/rsa_intermediate_ca_key", + "password": "1234", + "address": "127.0.0.1:0", + "dnsNames": ["127.0.0.1"], + "_logger": {"format": "text"}, + "tls": { + "minVersion": 1.2, + "maxVersion": 1.3, + "renegotiation": false, + "cipherSuites": [ + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256" + ] + }, + "authority": { + "backdate": "0s", + "provisioners": [ + { + "name": "scep", + "type": "scep", + "challenge": "not-so-secret" + }, { + "name": "step-cli", + "type": "jwk", + "encryptedKey": "eyJhbGciOiJQQkVTMi1IUzI1NitBMTI4S1ciLCJlbmMiOiJBMTI4R0NNIiwicDJjIjoxMDAwMDAsInAycyI6IlhOdmYxQjgxSUlLMFA2NUkwcmtGTGcifQ.XaN9zcPQeWt49zchUDm34FECUTHfQTn_.tmNHPQDqR3ebsWfd.9WZr3YVdeOyJh36vvx0VlRtluhvYp4K7jJ1KGDr1qypwZ3ziBVSNbYYQ71du7fTtrnfG1wgGTVR39tWSzBU-zwQ5hdV3rpMAaEbod5zeW6SHd95H3Bvcb43YiiqJFNL5sGZzFb7FqzVmpsZ1efiv6sZaGDHtnCAL6r12UG5EZuqGfM0jGCZitUz2m9TUKXJL5DJ7MOYbFfkCEsUBPDm_TInliSVn2kMJhFa0VOe5wZk5YOuYM3lNYW64HGtbf-llN2Xk-4O9TfeSPizBx9ZqGpeu8pz13efUDT2WL9tWo6-0UE-CrG0bScm8lFTncTkHcu49_a5NaUBkYlBjEiw.thPcx3t1AUcWuEygXIY3Fg", + "key": { + "use": "sig", + "kty": "EC", + "kid": "4UELJx8e0aS9m0CH3fZ0EB7D5aUPICb759zALHFejvc", + "crv": "P-256", + "alg": "ES256", + "x": "7ZdAAMZCFU4XwgblI5RfZouBi8lYmF6DlZusNNnsbm8", + "y": "sQr2JdzwD2fgyrymBEXWsxDxFNjjqN64qLLSbLdLZ9Y" + } + } + ], + "template": { + "country": "US", + "locality": "San Francisco", + "organization": "Smallstep" + } + } +} diff --git a/ca/testdata/secrets/rsa_intermediate_ca.crt b/ca/testdata/secrets/rsa_intermediate_ca.crt new file mode 100644 index 00000000..a575b91f --- /dev/null +++ b/ca/testdata/secrets/rsa_intermediate_ca.crt @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFJTCCAw2gAwIBAgIRAMBEHdXQtHUla+J13aUn/0gwDQYJKoZIhvcNAQELBQAw +FjEUMBIGA1UEAxMLcnNhLXJvb3QtY2EwHhcNMjIxMjAyMTE0MzE2WhcNMzIxMTI5 +MTE0MzE2WjAeMRwwGgYDVQQDExNyc2EtaW50ZXJtZWRpYXRlLWNhMIICIjANBgkq +hkiG9w0BAQEFAAOCAg8AMIICCgKCAgEArxVkidtUrM6KIdGZ8a2QtJWezrTxTiEM +lDeYqLd4CKp1bjQ7JOi1uc0mBG0Y4u5NwQRDk3L2aulLrENsPx4PMsPwMPXZgw67 +zTTuug1/uec8phW9IvEqu8FDQhFCMzZZMmc/0UTLmhJq5NZhIU8SQ6XYF/5s11Gm +zBbBG1CEV6KcwVul8+T/GcHr60h2/X4uRkibEdUsDy0jHFLMPOWMeKQXoA8hVWHc +QRYInRS5q+aFZ79YqMTUFT2tKdgSCiDsm6MqAPhFVB20ZrxMU6zco67+DBKAzSGy +qO0H6fxkStN4RBrCFTgUdyUPwSe5xCOVfR4JbF8pXMI9cA7iCT0Mw9ZgbTncKVdn +epwIZfqqYMP0C3EL+BZOSfEQeXIq7qlmHKwRRkc010ZaLmbKB9Kug/HcsS3CevU2 +J0Efosi2xfMcfhi11rAfKvZpyAuOVap7BONro3yYXjv6Co9sDWtyK6VkLsczp2MM +NHxhzjGXAcQdnU79UbGxO67imZm6FYLTwcg/6SVrfh+slLJ5nCyXqC/LaQ+Mc7Q+ +mdibgOzHSYg/QHVamic0uqn4BLw8QjICIZAnWWJHYjVgCieZrvK/7BGOjQ8+LT/8 +NhjI6MSuNMcXLxyOciiPw1r8fT/NUbJZblMDhibGTaOFCoMc3niY/fwxPb3p1J8I +tmOLoK8HCysCAwEAAaNmMGQwDgYDVR0PAQH/BAQDAgEGMBIGA1UdEwEB/wQIMAYB +Af8CAQAwHQYDVR0OBBYEFO0ULj6Dt1RakbRqV4rVFUdRHK3KMB8GA1UdIwQYMBaA +FCd9WZYMPpfDLBKjySFENwIXJpuzMA0GCSqGSIb3DQEBCwUAA4ICAQCiWxrj4HqV +J9tGj59Ea2cMZUcBfGPYh4dZ0af6IlNZnqW9ZlmNNF/h0VvCpd28STZlkW7hp2Xb +RcJ0tXs3MvnU0Sqzw8ZTevJgIIbiOIwndfmi4apfSC63JXftBkThP0xpR5LI/4pH +UPYyeGA13fynH4YmO4QBsGEXlKMKSYSjwrheYKkSB73AYlc7r8OqE/NAVHc1xzov +9GT4p7w+tF6vrgzUtwqpAEVM/3USmSx4rgSdkI4DPkrYb1HEqT8ixOIH/3IG42ag +UZgICckBPqcki8UbnU4nbxWVGJd18FE2n4wC2erewlBL+1PJFTmgDEKmOlcabot8 +QEk/YOpMThCm79VGuFB7frXoFefLCl5q1K5yV1eDsmr79ZFIy2WM2alnVk2Cvk/9 +oJQQ42AWRVHGFuaIrG+hLLtwq17MnoeyQ/A2IRlpWu7DpaCVfuPA+3yQC06qo98u +A3vGpifN8eohTSEMYNGQAsUsArYPwMEp/QrP4EwK8YnaJtd2HCnG4VS3D+RenRIF +04b8EXX64ePD07uzPh7dKpWfmdJf1xj8GSndw2vk14KYDOvjrXirVkNCXFxgU9jp +uTLGU/7Panm81xQgjeNwRaXxWvvDSrQaKMZ1QL6i0U7OTso0Q4VHivGG7IDhYSkA +zNRdjmJnuap8XWGs/4xKjMJcv12UtnaMgw== +-----END CERTIFICATE----- diff --git a/ca/testdata/secrets/rsa_intermediate_ca_key b/ca/testdata/secrets/rsa_intermediate_ca_key new file mode 100644 index 00000000..673df81b --- /dev/null +++ b/ca/testdata/secrets/rsa_intermediate_ca_key @@ -0,0 +1,54 @@ +-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,03e26f42f8642e55946bcad62fef0c2e + +54jydVXdnixOccnF90L9pkfsy4mrRC9xyl4BbZMaYwplZC+LE+U80GAdXOqSxBEo +sQBz+OTYaq2bmT2MDnoHty8I4vdDTVmxovc+NdtCJdC+etc2bSEKt68K57BPEqa0 +o7SE5Lk39zSDIFkyltQeYNII8sCX7H26kRsfZhmDYPoFXGCfnxrEQoASaF8S9n3l +9yERxk4untsVpvOPPde6Vn3b40ALqg0J0PaqzIbWifbWL8Uu3IeP27VHJLS4AH23 +emkWaZiT1bjWNevwWiU0REZ1CxyShaggJa4YwXPJJyRcQlvnVMZ8+DjXoQ1EdSGA +EGMfG6i5zDrRAdRDRgbJM56wZqIWup+/Kd0WyVGOteGFhzyl8Pad65NGYP9saPE/ +P0/Wi51t30KllF6i6XHATeAKPgGAMkl8E9x9KCQVqGEWi8Ceu3w5AMxC1tcwB0Xy +1X9NBipHaDh0DneTTdRRpwGCEIkZefDwy0z4rgsxrbKyY0YP1NKsFt+rNFkdNSnK +RevnejtYHSDjOyGImnLRJ0c2nxwet93hfY1g3yzagKtWUp/TXOO7EkggqUPObQhC +n9U5tkPxvHTCXSzeK3QqrbReyb3AlEay8Th8R8roxcClV83E4vcjjuvitcJ0MbSW ++/jCU1WhCanat67je749MB19msA95XYxNsAmCn17vJIVRI/QBS9HQCkf7UW1Jptm +hU06/7sytuOFboXh/xhfoQUomlx8Hl/GqV2yGZyL7SsH4sxoT9cVCO2vXCnr/r63 +Uo1nkEHQNddbBCR7yvjoeeq5PypGxZibC7YzWx87Hwcr8dEhBwzoqeIFkhnEVMyq +Y3xFIilqqRaLwG1c77wy5jReTv/OTJ2OU2VFDu3Pf7zAOcGtcQazcNv7PMkiqunK +Bp/vDLL+LiaWO/5Zl49DFPTGkRj5kNK/aNajfHyw0hvYYytiaGaH9DM3L+7kC7YG +2le8eLbUgZ7tqw3P2KueCK1F6Ef5X2It2sjxv/w5hz6lDtGfEIVXJuOamSUEewkY +9xM9njmqFhQjb71Khm3+/HUoxvmOebpuQ884xORfvzJ1rl8IHA84VTo8/XKp3EST +yMC39rGhtVuADHvNz3Y/WAWIbrJkkdZvMXyYKoOTosNVeFjJxyfKlz8ZMYmy6cM6 +mjOcsaI8xYUslYtpj/7vAjtcF4tJv94cQB/KGdUc/Z5JQ3r8zooG8ghEPt/5jiEr +4ECCK7btew0mexVv+HY3rX7UiPCHugfX6+XEIxQ8+AsM27FNFaKxjxTE2r9h95mP +jmcWO7YqyqyeEZmKoxNo5oLMKXIDKxzK6ianJYg65xMnT+cH5vcnVaQKaC2QcnMI +TiLOz/+ZdJSz2FiyE4myjnp9COKQhsDOfQA/1xzPF/4dqWMyWijGnlcozCHlU0i+ +2oG7izmDl9zn79v8VH6y0WjeEywoH5XlrF5eKBA2g7AtB8MCJTpIRVazTRbvhjaP +EXr+Zk6vPVlDS0KOIUJ4V8iYcatdoaJz1fM3XjVZ6Wwy8TaYd9EBwWlWdFDx6r3s +1aT5fDDyZNjnTx80OHyWT2IS/+/FrColWGc9s/t5raFm3KEnvVpFc+7/AKOV3keB ++3KVSg4ILLDYf7PfMrT2IPrWObuUXZ2InZPEG3T7BOtbbdO8BDbDng1xLxPGDFgQ +zKUFngsPO90PoDmNUZ9dBZ/oOI54e38hqUGB7vdTsNlX+VTK4n+qb8w7GzNhGgnR +fTP927HeuFBdq8Y2ngxt2i6vg9yo7Ojd+nG5OLj2T7uyNraKdaaBx4Nd9ZUZbNGt +4EueDSHCALKBsimLl4DfnMDnUK3G79dsoazs/nUr5y7kaUlkBGNZ/iSuoqpgeTKU +jsTmVjRj4W5opC+UUBiY/tE7qHGczLDw/mw/NP14nQ5iFdwi6EJv3viprYL+zL/A +zRTkcQ0KqBfc1ChVWhvxIg7QCsnPT6+y0yn2k6n4a9cUvQXcOQKqF5eOJWPE3ZeC +7fgIwt7ZdqHPZHyMxAnwWbmsj1Tn09SBW1b7S4t50aAPTRjDmrp5iC4vK59L7qPZ +ekoft0VduaJlKqq90Bh5ouRvTO6ytDI261bbEIGQqH1nJVt12bhNA6h3xI3Iwn/E +qlMLAN1M36LenUEp9l77AfiFU1f+d8ZP2U6bJo4FKTnRR33R6+89sezUmVEuqozt +qONJo0DE9XSAVhxpVX1QF91RjrJSiQtNyRkaOEyTsw2VvJpQNI5GAQN0TbcqgCVD +aUqPUuwntC7Wx7PkF6OR07rVxSIvhXs1NlG09nPZVByVCRJf/zKp9jMcRVJSXp16 ++Sqw4qifz/INEPGPgM3vr0GdvEN27S1IEFUZDU0M+e6KcHeIoLEPnhQPnZfO/kPT +69gRFOZAcONvnGyP+Fj74fRWpWWIIN6b8oIzPN8tez9g+DdmXHf/LnD0fGIfhqPI +GjjZcNJ8oa2F2qZfmwtrYs8UIChJxfZXK/lV7Jgf48ZDSF73war8nGHA/Sir4NsF +9cp3TxTSpXo2iXqb8ZH679q7OJ3UE7OiVKr2XzVEo7T/QSPnV4l9eiq9lDb/1cnS +AFfm3m0+Zqy+uE+Qfkigt5jWXBLQ3DbJEUNriumsit5dMeh2zCMwtYsWC8fumJw1 +6kJVZ7yEFXhFggTkHrgTZCI/9ym8FxCcz7W9qNy47h3aDOMs+yRidyl279FsKMR3 +gkjZmvGyAuZRqNttqldexMGwH1qVPIwDtCHdwesdefAydr/9h/ElDzAyBG31u3zN +7Bp5/JkN9OycTvUB7SIMR80Q7wwPJngovRu1wdKQVZC+y/snJR6tQx9u+OuSHrB+ +X0J4LFuxSj5PjsTH5y2o3UFbuKzxaIwbEibPvUc7FqW7O9/N4gYZaANgcodo0ozb +ZjhcL+oE90AGQyKSKGna5bZWdokLQBOUyro442gKXAOVARMzEHwIIWwD3bm6Mj0a +AmaMta3/LoCj54ESPFqRm7lCTmTj4gR6t5TED810hEimbxE8CBB6yrGTyj+vn+nH +9Wn1D+Pgo0QuHp1yBZI5xrFtX2Dm6TW7cKuv0oohgjd2WFKNIqzDhOeIslk3K9TL +kcBqeYMDJ5xi/R5/dfE5yLg7WhsPcH5QcMO2I6Sm+smXWytB8zo3NkX5UXUTdWNp +-----END RSA PRIVATE KEY----- diff --git a/ca/testdata/secrets/rsa_root_ca.crt b/ca/testdata/secrets/rsa_root_ca.crt new file mode 100644 index 00000000..8c2a72f5 --- /dev/null +++ b/ca/testdata/secrets/rsa_root_ca.crt @@ -0,0 +1,29 @@ +-----BEGIN CERTIFICATE----- +MIIE/DCCAuSgAwIBAgIRAOFB5q6CzRilW0ERurTeSQ4wDQYJKoZIhvcNAQELBQAw +FjEUMBIGA1UEAxMLcnNhLXJvb3QtY2EwHhcNMjIxMjAyMTE0MjI1WhcNMzIxMTI5 +MTE0MjI1WjAWMRQwEgYDVQQDEwtyc2Etcm9vdC1jYTCCAiIwDQYJKoZIhvcNAQEB +BQADggIPADCCAgoCggIBALIcD6VfJ6NZLWOhrLHr9au3WhKOmvt2gp+l53rjmwP3 +PLApSnFi3PGE9gvwzdGd0XeIIithgj+FiZEk/gdWfjx3abjpNM4uTsjBweQ4d3uT +zgH5h/AmGbSVUweqOCvmK5cingcvc2UGVbDo5VOP50bZR8O9NY2OQNgFHig7Z+xT +eZSkGF7Sxm1zNMNU7BZqBNofFcwYDIaR/sBFuE9Im2qXj0duHbC1GXuVivE+iTDI +ir52qsuobnXwEQyGe3EOwIAD9AMPsmmJ/vZSaVLFO0dIbSwTqB3nXaNC8+hA/dyX +a9gEdVsSzKUiXfsk5awAOHOAEpCusywyJzZhhIyqot4rr3A3nuVOmg5utvJX2jMr +wtGT7n7YhJWJVIcB/ahx/G7qwkcphEM7jnfweVgdDGTjcJ2tZchqx4U0axo+5wQy +hebLz6z9QLkmfIMW0qjV6JcrYz2U1T4xSFmyNBhOrJQw4OFufSEWqYSJxoUHHOBn +Dy4V98AhoIkK5UDTeTrQea5QJRGRhiCfl6VpuO1YAP/4oNrJa+rWrzYPU5bq3FF2 +z2aCb9MAxnDQmfHfCSn6avioM2BcRQ8SfVVj1XsI4JtS7i7kqsHzuezJp28Jvll5 +sOTGp6CNASLJg2zRE3LZbNuuZ3JlVDZPDHqOqci7Gw8xwNXZv1SNNVDBDLsN3sSd +AgMBAAGjRTBDMA4GA1UdDwEB/wQEAwIBBjASBgNVHRMBAf8ECDAGAQH/AgEBMB0G +A1UdDgQWBBQnfVmWDD6XwywSo8khRDcCFyabszANBgkqhkiG9w0BAQsFAAOCAgEA +mV/q1xjM9k+2Z9MhC7RXT0a/9bMVry9RiWp4xD09bPLRso+T9Pys/m222DxTjW6+ +JAM1fwm6HKESeWHToIBnB1htIG2jMSC5wn2/oKfEFnJU16f4lE7aoFMHP6Pxhf9w +dGXvb7Pbze1MHNtNabx5x2uVp5DLTjOjL2o7pufSXNpB3djx20jADx5KqqXQiIqk +rMDi1rpWRnNT/IqkkmDdGbG9WyKp28z8HPW2Iyq80zp1d3diJvtRZTeDTBrc8NGk +96RpK1IVY0c8Z56UfecILuthm18ChSxm8DTXdc1CA1e89fiZ/pfEXPrbYLdcq8/b +WQjA39z0zTiGC6gjd0g5hGeXZ5ThuW0s1EwpWmcF5bvHOxK2SOtYzxxy6bhbOzU7 +4J0uCj+GIR7eKtdrHdRv0cHFPE4/XDEI/93UCJjOphNekSKGUiQKzTZhjP7g6DdM +bBtsdEwkVckqFTrOlHy1aDfoUzuOB8DDwSs/59h/0a2MtGBq1MAjLaZUlDAUUbYO +x8VbloQHxcEdrUYmIGEhoI+zPz6Bm2xsaIs72R10y9PfFV5xY9JcsnA9AvJ2KOHo +RH7gmqh7GyqCNcQf7bfhC2SLMa8luEn0tQFVx7F/vbO1rzpvsEvtsvHka1SEEqS/ +ctNS8RyWPh92jaJQ4U9nWMHOJJZ4LYW38gsMc/om7+k= +-----END CERTIFICATE----- diff --git a/ca/testdata/secrets/rsa_root_ca_key b/ca/testdata/secrets/rsa_root_ca_key new file mode 100644 index 00000000..b0dba1db --- /dev/null +++ b/ca/testdata/secrets/rsa_root_ca_key @@ -0,0 +1,54 @@ +-----BEGIN RSA PRIVATE KEY----- +Proc-Type: 4,ENCRYPTED +DEK-Info: AES-256-CBC,2b2138d58dc4fe659251306226ee53ef + +f6H0Rvs3FmbjNk31qTe/CikGW6oFT3p+6A/g6E7gnloHuxVv4HdM0RDHOUvSMG49 +hb2kLYfbztJ7+RyGdc5JhozgfwRJsSP+iT0JbDQyHlTWzG5YasMnGrMbeLayn0Bw +fhawiDOaBzK36hBFnx3aE5D3MOEbJM81/tZ7SoAovvgLZmhTH5w6cGYSJle6Fgey +47skoiuRX4JJ1Us6aiME203l6AdPEs01XVPRQFZHMdbTCQ5ZVeH/BS2GHn2vfosg +PLJ6RUQIILuBytRwiWZIXoVZDI7T0d6eiUizj2cyIi58rypkGDDeDvN6Uzq/r8Or +epwo28YlIDRz4H40XIGVDnD8LcIbAmcfe2FTz+TbTTcQcBySnuQQJhJ6aGDnE2LG +5QPSZOLAStlMP6ceGB6oeo7nBYLnqxUbDyeNeeIfBmf2NDgFpIwjgjs+QMXg1XFP +/Z0BnKm/bmKKc94w3BwsAsZ0RwZTS+WyK+xKoXpQNVRoECKZt2oDTIPUX5no7dQO +CQPOvJoYjGd+IS1jykvViYZYW4Lae08thWOMbWVTyV882/wpR7DN697w38VFLw2x +q9dhd4wFZfzwGndO6xUq5h3qHGXg5xPS/ArvK5KGRXFusHI0HKqK5TeJwW2NBFky +AhYPr/wdGdyL+mjU4ynjG4AekdAUi8t2Jpxf7+NuWGIbD/J00GPExOUuM68gqmuw +9wGXj0EaPEyBSWc7Sq95o5+eg5VjwLsGnEKLtYLJRuwOnQ+6LZddOsaNNHgafP3N +yrDN4Xu2NBowzrbAPv4nFxQF6pNkAJTtTOimRQA5qwf04Er1KSw3SAs0s8WXTDnp +kySMpvSibBo4CQE+XvjGISg+yTY7/Uj6lZrJFzwrl4Nne1k960qofY7B6D8sGSxk +8DZpsNkfY86juIBUri9pp2+nqEmj8NcK0gGpNgomYbPQHoQuudfWKER8JEX5hp9g +ik3RZIpes3yKJYbzEKpeAOMRy2yS75B9DOpvIO7YPfUsjGVWnV55Cqni+z2QUOWR +laRnRReQRQ/C3sinoFCEDZNmw5W5ex+iaGxj7d88tolFzvN6P7JdJTrq9kZ6pEIV +yJnWT6dxoabxtyArpOAIwsEbeVXyFq1o0UF5x8Y1xOJOvlWallj0cZo/mHO+sFVT +VLR1Ijh+klcKjJnU1s7yk/Ls/eRMJzSnk3iAV9WJuuOFyvpzmO26uTQh0f1rSrk/ +k4DA9Klywo9OFlCvGU5xuRhISDEBBrxtKQMkefFQRBxclqZldDmbss2Zr4vfmhjx +5JdETi7q40Nt0kWsXi/XXIriEILvVIShYuER84aYSG2LQw3kOREA2BLOfJXYvRxc +g3UzHviOYRpzPb7fJmOsSa1sRMWTKbZn1eBwFZbmqZmFboVzUmYUiFqFFCMGaPq0 +afhkZGmM/dPgStruKEyXCcAHnsIFruNZGICnDUbQyAwXlw66fJG/IwL0FbTdhr3i +68wlLKA3uAAdTPkNQvef9Ed5b2xu9Yazt3ub93sKTbSzv1PZU+VVyrfmCVXXRrku +ybRoLd4HAeuMKZ7jF4dLNzPDvJ6SfdMP7Qw/NoeCBogbtsstsHe+3hOEmBYlPZpT ++AyXV/BNEvli9uBUlwy9B7B6s0hj1bxMnxHTCxEuCBf3EYgRlwcIRqSCi7EV6FuT +1ScpROJP3U1+FSF8b2pP7W23xAGtXUOBSoGvMlZcxF3+xB4L3zVMqfqwlbLXvSix +QoXKYtESBmVVLT5jc+sWUelEynXowG+YaDVUyEBx99vlXAznQ3D99rD1dzvzx9Um +TI0aV2IjeUgOXWP5b7rVs+GJ82DDUBBsZYEYK6JIpiBtdhAYhWbplusJCwdOAmyj ++9JtfLrdJTohAn1smp285wHrHgdhLECEotaHqh8Cubrw5u66couCw7ibVSDrmshi +8xiPL3hp0jWbE10Lah4MK5pMLfjq2wOta435RuD3HNJu1nGGvEIb/+Malef8JzBW +y7iABGlAHPNhcOheNQX/nXuTnUOwv69N03/i+/hWzGHIjYH/nI02EZ+CHtuCbeUd +JCP3Ia1xCDEZJEtb2GmswgB5P06U7z2rZemb2HIWC6/Sors72WlEZhtis1y9/mRF +1pmGFsqmQCHk7XNrdZB56KjB4Kkj7eOE5xO01ALdZXs7nIhB7S9Sqk6Rtf+Th85N +1BT/esB3d30ORVu3TbV1uashC71ThtdNEpYNi441Yfs8u/c/c+7NtUoxBcIIvMEs +FMCLs4Nqt1y2UxocWGQtii2EvjwStAgtNIGhq+/6SZVRIU3CyYm3RRx5eQ1VdfNh +i+bOJlf5l8/gZXsaWwD2tBOCibml9GbFJeGPQi9Rc6AUUeTGmNRnA1PUcwbs96uz +F/lmo1dms5jiV2+d+SFQgAujrJSRsST4GxpqDlU3T/anIknusTkOyyuP3Z3EY6eA +LiY6sdKYj40IFdpM3aLl6LAIgkTXS1ji4nvfu5CAdBAsntTRVRB2Ew3ux8+ZsShg +Rg/LMEmEP8oMq1JFrx9q2rlBghWyUdY5M+ZY/e8hGheMuaUGs8SeqWlI513+CvLw +sWOUwnox+j9rjvj43Q3ac9mbqjwjykMpBDAMhAeJkW5FSK5gc6LPmRvUhfyv0De7 +bgA6dpQYh6+l3yKoWmNQdFZ0YtuEc+wzzgbyUE1s/BOTB3WDLaBnUAw7R3nkTUyX +05t5b1NCcrj2fpe0DhRa7KqNQTVazEgZIkd0nPVGP8bmfMEMCXw2ri0wls0F4KkB +Y52Ctx+/kQkP8HYJMV79RURNvI9204C8a+w09++w9rmHuUlGXfJ7/iVADRaXI1pM +E+N4q7KrhcQYlRWthmwsol2unqtnTHjSyHiYtHeagNTt2eNkAqG61E+mtYsjQ6Al ++aL3vi73hJ6oNLpT8Cb2S4XYDziIlKTtX4biZYJgkc/P4Ado0Z5ZhXqLnt+BsrDv +FuqpZoHp0BA9qaCPuocL7Ne6cVTY1PGKS+Gkh9u+QWmrp1QGltNQNUiNUiuSKP79 +41tdta3UYstwtuTydQPGbg71YPSXM6CqEUuYINP5yVSiO3k1aPA82Uxr3TYdnym7 +D54ctp9HHk3SYpA/zdT5clNwyNiTv/bZ2Wa0DUpBRK3epvLVB6fyGlmSFnOtyelP +-----END RSA PRIVATE KEY----- From c365d8580e55ed5dd4835ecc4f3af79636c2cc96 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Tue, 13 Dec 2022 10:22:35 +0100 Subject: [PATCH 2/4] 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) - -} From 0153ff4377401535064968907f505fc5aad41f25 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 4 May 2023 11:43:57 +0200 Subject: [PATCH 3/4] Remove superfluous `GetChallengePassword` --- authority/provisioner/scep.go | 9 ++------- authority/provisioners.go | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/authority/provisioner/scep.go b/authority/provisioner/scep.go index 0db40864..f098a6e4 100644 --- a/authority/provisioner/scep.go +++ b/authority/provisioner/scep.go @@ -201,11 +201,6 @@ func (s *SCEP) AuthorizeSign(ctx context.Context, token string) ([]SignOption, e }, nil } -// GetChallengePassword returns the challenge password -func (s *SCEP) GetChallengePassword() string { - return s.ChallengePassword -} - // GetCapabilities returns the CA capabilities func (s *SCEP) GetCapabilities() []string { return s.Capabilities @@ -236,7 +231,7 @@ func (s *SCEP) ValidateChallenge(ctx context.Context, challenge, transactionID s case validationMethodWebhook: return s.challengeValidationController.Validate(ctx, challenge, transactionID) default: - if subtle.ConstantTimeCompare([]byte(s.secretChallengePassword), []byte(challenge)) == 0 { + if subtle.ConstantTimeCompare([]byte(s.ChallengePassword), []byte(challenge)) == 0 { return errors.New("invalid challenge password provided") } return nil @@ -259,7 +254,7 @@ func (s *SCEP) selectValidationMethod() validationMethod { if len(s.challengeValidationController.webhooks) > 0 { return validationMethodWebhook } - if s.secretChallengePassword != "" { + if s.ChallengePassword != "" { return validationMethodStatic } return validationMethodNone diff --git a/authority/provisioners.go b/authority/provisioners.go index 24d25caa..5d594536 100644 --- a/authority/provisioners.go +++ b/authority/provisioners.go @@ -1223,7 +1223,7 @@ func ProvisionerToLinkedca(p provisioner.Interface) (*linkedca.Provisioner, erro Data: &linkedca.ProvisionerDetails_SCEP{ SCEP: &linkedca.SCEPProvisioner{ ForceCn: p.ForceCN, - Challenge: p.GetChallengePassword(), + Challenge: p.ChallengePassword, Capabilities: p.Capabilities, MinimumPublicKeyLength: int32(p.MinimumPublicKeyLength), IncludeRoot: p.IncludeRoot, From 8c53dc90294a6fb4113c2e5da91c4659d31aadad Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Thu, 4 May 2023 11:44:22 +0200 Subject: [PATCH 4/4] Use `require.NoError` where appropriate in provisioner tests --- authority/provisioners_test.go | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/authority/provisioners_test.go b/authority/provisioners_test.go index b4eb1bf9..f6af6f54 100644 --- a/authority/provisioners_test.go +++ b/authority/provisioners_test.go @@ -13,6 +13,8 @@ import ( "go.step.sm/crypto/keyutil" "go.step.sm/linkedca" + "github.com/stretchr/testify/require" + "github.com/smallstep/assert" "github.com/smallstep/certificates/api/render" "github.com/smallstep/certificates/authority/admin" @@ -30,9 +32,9 @@ func TestGetEncryptedKey(t *testing.T) { tests := map[string]func(t *testing.T) *ek{ "ok": func(t *testing.T) *ek { c, err := LoadConfiguration("../ca/testdata/ca.json") - assert.FatalError(t, err) + require.NoError(t, err) a, err := New(c) - assert.FatalError(t, err) + require.NoError(t, err) return &ek{ a: a, kid: c.AuthorityConfig.Provisioners[1].(*provisioner.JWK).Key.KeyID, @@ -40,9 +42,9 @@ func TestGetEncryptedKey(t *testing.T) { }, "fail-not-found": func(t *testing.T) *ek { c, err := LoadConfiguration("../ca/testdata/ca.json") - assert.FatalError(t, err) + require.NoError(t, err) a, err := New(c) - assert.FatalError(t, err) + require.NoError(t, err) return &ek{ a: a, kid: "foo", @@ -96,16 +98,16 @@ func TestGetProvisioners(t *testing.T) { tests := map[string]func(t *testing.T) *gp{ "ok": func(t *testing.T) *gp { c, err := LoadConfiguration("../ca/testdata/ca.json") - assert.FatalError(t, err) + require.NoError(t, err) a, err := New(c) - assert.FatalError(t, err) + require.NoError(t, err) return &gp{a: a} }, "ok/rsa": func(t *testing.T) *gp { c, err := LoadConfiguration("../ca/testdata/rsaca.json") - assert.FatalError(t, err) + require.NoError(t, err) a, err := New(c) - assert.FatalError(t, err) + require.NoError(t, err) return &gp{a: a} }, } @@ -135,20 +137,20 @@ func TestGetProvisioners(t *testing.T) { func TestAuthority_LoadProvisionerByCertificate(t *testing.T) { _, priv, err := keyutil.GenerateDefaultKeyPair() - assert.FatalError(t, err) + require.NoError(t, err) csr := getCSR(t, priv) sign := func(a *Authority, extraOpts ...provisioner.SignOption) *x509.Certificate { key, err := jose.ReadKey("testdata/secrets/step_cli_key_priv.jwk", jose.WithPassword([]byte("pass"))) - assert.FatalError(t, err) + require.NoError(t, err) token, err := generateToken("smallstep test", "step-cli", testAudiences.Sign[0], []string{"test.smallstep.com"}, time.Now(), key) - assert.FatalError(t, err) + require.NoError(t, err) ctx := provisioner.NewContextWithMethod(context.Background(), provisioner.SignMethod) opts, err := a.Authorize(ctx, token) - assert.FatalError(t, err) + require.NoError(t, err) opts = append(opts, extraOpts...) certs, err := a.Sign(csr, provisioner.SignOptions{}, opts...) - assert.FatalError(t, err) + require.NoError(t, err) return certs[0] } getProvisioner := func(a *Authority, name string) provisioner.Interface { @@ -177,9 +179,7 @@ func TestAuthority_LoadProvisionerByCertificate(t *testing.T) { }, MGetCertificateData: func(serialNumber string) (*db.CertificateData, error) { p, err := a1.LoadProvisionerByName("dev") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) return &db.CertificateData{ Provisioner: &db.ProvisionerData{ ID: p.GetID(), @@ -194,9 +194,7 @@ func TestAuthority_LoadProvisionerByCertificate(t *testing.T) { a2.adminDB = &mockAdminDB{ MGetCertificateData: (func(s string) (*db.CertificateData, error) { p, err := a2.LoadProvisionerByName("dev") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) return &db.CertificateData{ Provisioner: &db.ProvisionerData{ ID: p.GetID(),