Archives
/
smallstep-certificates
mirror of https://github.com/smallstep/certificates
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make the Apple JWS fix more robust and catch more cases.

Browse Source
pull/1631/head
Herman Slatman 6 months ago
parent 113491e7af
commit 26a3bb3c11
No known key found for this signature in database
GPG Key ID: F4D8A44EA0A75A4F
2 changed files with 179 additions and 39 deletions
Show Stats Download Patch File Download Diff File

94
acme/api/middleware.go
Unescape Escape View File

@ -1,7 +1,6 @@
package api
import (
"bytes"
"context"
"crypto/rsa"
"errors"
@ -452,20 +451,23 @@ func verifyAndExtractJWSPayload(next nextHTTP) nextHTTP {
// Generally this shouldn't happen, but we've observed this to be the case with
// the macOS ACME client, which seems to omit (at least one) leading null byte(s).
// The error returned is `square/go-jose: error in cryptographic primitive`, which
// is a sential error that hides the details of the actual underlying error, which
// is a sentinel error that hides the details of the actual underlying error, which
// is as follows: `square/go-jose: invalid signature size, have 63 bytes, wanted 64`,
// for ES256.
func retryVerificationWithPatchedSignatures(jws *jose.JSONWebSignature, jwk *jose.JSONWebKey) ([]byte, error) {
patchSignatures(jws)
return jws.Verify(jwk)
}
// patchSignatures patches the signatures in a JWS if it finds signatures that
// are too short for the signature algorithm.
func patchSignatures(jws *jose.JSONWebSignature) {
for i, s := range jws.Signatures {
func retryVerificationWithPatchedSignatures(jws *jose.JSONWebSignature, jwk *jose.JSONWebKey) (data []byte, err error) {
originalSignatureValues := make([][]byte, len(jws.Signatures))
patched := false
defer func() {
if patched && err != nil {
for i, sig := range jws.Signatures {
sig.Signature = originalSignatureValues[i]
jws.Signatures[i] = sig
}
}
}()
for i, sig := range jws.Signatures {
var expectedSize int
alg := strings.ToUpper(s.Header.Algorithm)
alg := strings.ToUpper(sig.Header.Algorithm)
switch alg {
case jose.ES256:
expectedSize = 64
@ -477,11 +479,73 @@ func patchSignatures(jws *jose.JSONWebSignature) {
// other cases are (currently) ignored
continue
}
if diff := expectedSize - len(s.Signature); diff > 0 {
s.Signature = append(bytes.Repeat([]byte{0x00}, diff), s.Signature...)
jws.Signatures[i] = s
switch diff := expectedSize - len(sig.Signature); diff {
case 0:
// expected length; nothing to do; will result in just doing the
// same verification (as done before calling this function) again,
// and thus an error will be returned.
continue
case 1:
patched = true
original := make([]byte, expectedSize-diff)
copy(original, sig.Signature)
originalSignatureValues[i] = original
patchedR := make([]byte, expectedSize)
copy(patchedR[0:1], []byte{0x00})
copy(patchedR[1:], original[0:expectedSize-diff])
sig.Signature = patchedR
jws.Signatures[i] = sig
// verify it with a patched R; return early if successful; continue
// with patching S if not.
data, err = jws.Verify(jwk)
if err == nil {
return
}
patchedS := make([]byte, expectedSize)
halfSize := expectedSize / 2
copy(patchedS[:halfSize], original[:halfSize])
copy(patchedS[halfSize:expectedSize/2+1], []byte{0x00})
copy(patchedS[halfSize+1:], original[halfSize:])
sig.Signature = patchedS
jws.Signatures[i] = sig
case 2:
// assumption is currently the Apple case, in which only the
// first null byte of R and/or S are removed, and thus not a case in
// which two first bytes of either R or S are removed.
patched = true
original := make([]byte, expectedSize-diff)
copy(original, sig.Signature)
originalSignatureValues[i] = original
patchedRS := make([]byte, expectedSize)
halfSize := expectedSize / 2
copy(patchedRS[0:1], []byte{0x00})
copy(patchedRS[1:halfSize], original[0:halfSize-1])
copy(patchedRS[halfSize:halfSize+1], []byte{0x00})
copy(patchedRS[halfSize+1:], original[halfSize-1:expectedSize-2])
sig.Signature = patchedRS
jws.Signatures[i] = sig
default:
// Technically, there can be multiple null bytes in either R or S,
// so when the difference is larger than 2, there is more than one
// option to pick. Apple's ACME client seems to only cut off the
// first null byte of either R or S, so we don't do anything in this
// case. Will result in just doing the same verification (as done
// before calling this function) again, and thus an error will be
// returned.
// TODO(hs): log this specific case? It might mean some other ACME
// client is doing weird things.
continue
}
}
data, err = jws.Verify(jwk)
return
}
// isPostAsGet asserts that the request is a PostAsGet (empty JWS payload).

124
acme/api/middleware_test.go
Unescape Escape View File

@ -471,7 +471,7 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) {
err: acme.NewErrorISE("jwk expected in request context"),
}
},
"fail/verify-jws-failure": func(t *testing.T) test {
"fail/verify-jws-failure-wrong-jwk": func(t *testing.T) test {
_jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
_pub := _jwk.Public()
@ -483,6 +483,33 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) {
err: acme.NewError(acme.ErrorMalformedType, "error verifying jws: square/go-jose: error in cryptographic primitive"),
}
},
"fail/verify-jws-failure-too-many-signatures": func(t *testing.T) test {
newParsedJWS, err := jose.ParseJWS(raw)
assert.FatalError(t, err)
newParsedJWS.Signatures = append(newParsedJWS.Signatures, newParsedJWS.Signatures...)
ctx := context.WithValue(context.Background(), jwsContextKey, newParsedJWS)
ctx = context.WithValue(ctx, jwkContextKey, pub)
return test{
ctx: ctx,
statusCode: 400,
err: acme.NewError(acme.ErrorMalformedType, "error verifying jws: square/go-jose: too many signatures in payload; expecting only one"),
}
},
"fail/apple-acmeclient-omitting-leading-null-byte-in-signature-with-wrong-jwk": func(t *testing.T) test {
_jwk, err := jose.GenerateJWK("EC", "P-256", "ES256", "sig", "", 0)
assert.FatalError(t, err)
_pub := _jwk.Public()
appleNullByteCaseBody := `{"payload":"dGVzdC0xMTA1","protected":"eyJhbGciOiJFUzI1NiJ9","signature":"rQPYKYflfKnlgBKqDeWsJH2TJ6iHAnou7sFzXlmYD4ArXqLfYuqotWERKrna2wfzh0pu7USWO2gzlOqRK9qq"}`
appleNullByteCaseJWS, err := jose.ParseJWS(appleNullByteCaseBody)
require.NoError(t, err)
ctx := context.WithValue(context.Background(), jwsContextKey, appleNullByteCaseJWS)
ctx = context.WithValue(ctx, jwkContextKey, &_pub)
return test{
ctx: ctx,
statusCode: 400,
err: acme.NewError(acme.ErrorMalformedType, "error verifying jws: square/go-jose: error in cryptographic primitive"),
}
},
"fail/algorithm-mismatch": func(t *testing.T) test {
_pub := *pub
clone := &_pub
@ -579,19 +606,19 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) {
},
}
},
"ok/apple-acmeclient-omitting-leading-null-bytes-in-signature": func(t *testing.T) test {
"ok/apple-acmeclient-omitting-leading-null-byte-in-signature": func(t *testing.T) test {
appleNullByteCaseKey := []byte(`{
"kid": "2eIRaNCmST4CwcOHyRSrGO-7k4TAr9fdNk0_2qw4-ps",
"kid": "uioinbiTlJICL0MYsb6ar1totfRA2tiPqWgntF8xUdo",
"crv": "P-256",
"alg": "ES256",
"kty": "EC",
"x": "v_Twh1khKHS32OJF7Bhzr3DVExbC54bRokOE9wdweRY",
"y": "-IWh-AXzmzh1Mg9OMuWfzje24wLPJmU6AwERo86zk3k"
"x": "wlz-Kv9X0h32fzLq-cogls9HxoZQqV-GuWxdb2MCeUY",
"y": "xzP6zRrg_jynYljZTxfJuql_QWtdQR6lpJ52q_6Vavg"
}`)
appleNullByteCaseJWK := &jose.JSONWebKey{}
err = json.Unmarshal(appleNullByteCaseKey, appleNullByteCaseJWK)
require.NoError(t, err)
appleNullByteCaseBody := `{"protected":"eyJhbGciOiJFUzI1NiIsImp3ayI6eyJjcnYiOiJQLTI1NiIsImt0eSI6IkVDIiwieCI6InZfVHdoMWtoS0hTMzJPSkY3Qmh6cjNEVkV4YkM1NGJSb2tPRTl3ZHdlUlkiLCJ5IjoiLUlXaC1BWHptemgxTWc5T011V2Z6amUyNHdMUEptVTZBd0VSbzg2emszayJ9LCJub25jZSI6ImVERTRjMnhPVDIxQ1lUWnNjMjl4ZFhjNVNFcEdVM0ZRVkc5SGEzRjBSMVEiLCJ1cmwiOiJodHRwczovL2FhZTItMTYzLTE1OC00NS00OS5uZ3Jvay1mcmVlLmFwcC9hY21lL21kYS9uZXctYWNjb3VudCJ9","payload":"eyJ0ZXJtc09mU2VydmljZUFncmVlZCI6dHJ1ZX0","signature":"57RgynZOnBrkx8UIrXZD80Lfql1IitFjBqFoiz2YOew2_oCb8JLx2C2RFp1AGQmP-3p6KP8e3GKb4anZVrdf"}`
appleNullByteCaseBody := `{"payload":"dGVzdC0xMTA1","protected":"eyJhbGciOiJFUzI1NiJ9","signature":"rQPYKYflfKnlgBKqDeWsJH2TJ6iHAnou7sFzXlmYD4ArXqLfYuqotWERKrna2wfzh0pu7USWO2gzlOqRK9qq"}`
appleNullByteCaseJWS, err := jose.ParseJWS(appleNullByteCaseBody)
require.NoError(t, err)
ctx := context.WithValue(context.Background(), jwsContextKey, appleNullByteCaseJWS)
@ -603,7 +630,7 @@ func TestHandler_verifyAndExtractJWSPayload(t *testing.T) {
p, err := payloadFromContext(r.Context())
tassert.NoError(t, err)
if tassert.NotNil(t, p) {
tassert.Equal(t, []byte(`{"termsOfServiceAgreed":true}`), p.value)
tassert.Equal(t, []byte(`test-1105`), p.value)
tassert.False(t, p.isPostAsGet)
tassert.False(t, p.isEmptyJSON)
}
@ -1730,36 +1757,85 @@ func TestHandler_checkPrerequisites(t *testing.T) {
}
}
func Test_patchSignatures(t *testing.T) {
key := []byte(`{
"kid": "2eIRaNCmST4CwcOHyRSrGO-7k4TAr9fdNk0_2qw4-ps",
func Test_retryVerificationWithPatchedSignatures(t *testing.T) {
patchedRKey := []byte(`{
"kid": "uioinbiTlJICL0MYsb6ar1totfRA2tiPqWgntF8xUdo",
"crv": "P-256",
"alg": "ES256",
"kty": "EC",
"x": "wlz-Kv9X0h32fzLq-cogls9HxoZQqV-GuWxdb2MCeUY",
"y": "xzP6zRrg_jynYljZTxfJuql_QWtdQR6lpJ52q_6Vavg"
}`)
patchedRJWK := &jose.JSONWebKey{}
err := json.Unmarshal(patchedRKey, patchedRJWK)
require.NoError(t, err)
patchedRBody := `{"payload":"dGVzdC0xMTA1","protected":"eyJhbGciOiJFUzI1NiJ9","signature":"rQPYKYflfKnlgBKqDeWsJH2TJ6iHAnou7sFzXlmYD4ArXqLfYuqotWERKrna2wfzh0pu7USWO2gzlOqRK9qq"}`
patchedR, err := jose.ParseJWS(patchedRBody)
require.NoError(t, err)
patchedSKey := []byte(`{
"kid": "PblXsnK59uTiF5k3mmAN2B6HDPPxqBL_4UGhEG8ZO6g",
"crv": "P-256",
"alg": "ES256",
"kty": "EC",
"x": "v_Twh1khKHS32OJF7Bhzr3DVExbC54bRokOE9wdweRY",
"y": "-IWh-AXzmzh1Mg9OMuWfzje24wLPJmU6AwERo86zk3k"
"x": "T5aM_TOSattXNeUkH1VHZXh8URzdjZTI2zLvVgI0cy0",
"y": "Lf8h8qZnURXIxm6OnQ69kxGC91YtTZRD2GAroEf1UA8"
}`)
jwk := &jose.JSONWebKey{}
err := json.Unmarshal(key, jwk)
patchedSJWK := &jose.JSONWebKey{}
err = json.Unmarshal(patchedSKey, patchedSJWK)
require.NoError(t, err)
body := `{"protected":"eyJhbGciOiJFUzI1NiIsImp3ayI6eyJjcnYiOiJQLTI1NiIsImt0eSI6IkVDIiwieCI6InZfVHdoMWtoS0hTMzJPSkY3Qmh6cjNEVkV4YkM1NGJSb2tPRTl3ZHdlUlkiLCJ5IjoiLUlXaC1BWHptemgxTWc5T011V2Z6amUyNHdMUEptVTZBd0VSbzg2emszayJ9LCJub25jZSI6ImVERTRjMnhPVDIxQ1lUWnNjMjl4ZFhjNVNFcEdVM0ZRVkc5SGEzRjBSMVEiLCJ1cmwiOiJodHRwczovL2FhZTItMTYzLTE1OC00NS00OS5uZ3Jvay1mcmVlLmFwcC9hY21lL21kYS9uZXctYWNjb3VudCJ9","payload":"eyJ0ZXJtc09mU2VydmljZUFncmVlZCI6dHJ1ZX0","signature":"57RgynZOnBrkx8UIrXZD80Lfql1IitFjBqFoiz2YOew2_oCb8JLx2C2RFp1AGQmP-3p6KP8e3GKb4anZVrdf"}`
jws, err := jose.ParseJWS(body)
patchedSBody := `{"payload":"dGVzdC02Ng","protected":"eyJhbGciOiJFUzI1NiJ9","signature":"krtSKSgVB04oqx6i9QLeal_wZSnjV1_PSIM3AubT0WRIxnhl_yYbVpa3i53p3dUW56TtP6_SUZboH6SvLHMz"}`
patchedS, err := jose.ParseJWS(patchedSBody)
require.NoError(t, err)
patchedRSKey := []byte(`{
"kid": "U8BmBVbZsNUawvhOomJQPa6uYj1rdxCPQWF_nOLVsc4",
"crv": "P-256",
"alg": "ES256",
"kty": "EC",
"x": "Ym0l3GMS6aHBLo-xe73Kub4kafnOBu_QAfOsx5y-bV0",
"y": "wKijX9Cu67HbK94StPcI18WulgRfIMbP2ZU7gQuf3-M"
}`)
patchedRSJWK := &jose.JSONWebKey{}
err = json.Unmarshal(patchedRSKey, patchedRSJWK)
require.NoError(t, err)
patchedRSBody := `{"payload":"dGVzdC05MDY3","protected":"eyJhbGciOiJFUzI1NiJ9","signature":"2r_My19oRg7mWf9I5JTkNYp8otfEMz-yXRA8ltZTAKZxyJLurpVEgicmNItu7lfcCrGrTgI3Obye_gSaIyc"}`
patchedRS, err := jose.ParseJWS(patchedRSBody)
require.NoError(t, err)
patchedRWithWrongJWK, err := jose.ParseJWS(patchedRBody)
require.NoError(t, err)
tests := []struct {
name string
jws *jose.JSONWebSignature
jwk *jose.JSONWebKey
name string
jws *jose.JSONWebSignature
jwk *jose.JSONWebKey
expectedData []byte
expectedSignature string
expectedError error
}{
{"ok/patched", jws, jwk},
{"ok/patched-r", patchedR, patchedRJWK, []byte(`test-1105`), `AK0D2CmH5Xyp5YASqg3lrCR9kyeohwJ6Lu7Bc15ZmA-AK16i32LqqLVhESq52tsH84dKbu1EljtoM5TqkSvaqg`, nil},
{"ok/patched-s", patchedS, patchedSJWK, []byte(`test-66`), `krtSKSgVB04oqx6i9QLeal_wZSnjV1_PSIM3AubT0WQASMZ4Zf8mG1aWt4ud6d3VFuek7T-v0lGW6B-kryxzMw`, nil},
{"ok/patched-rs", patchedRS, patchedRSJWK, []byte(`test-9067`), `ANq_zMtfaEYO5ln_SOSU5DWKfKLXxDM_sl0QPJbWUwAApnHIku6ulUSCJyY0i27uV9wKsatOAjc5vJ7-BJojJw`, nil},
{"fail/patched-r-wrong-jwk", patchedRWithWrongJWK, patchedRSJWK, nil, `rQPYKYflfKnlgBKqDeWsJH2TJ6iHAnou7sFzXlmYD4ArXqLfYuqotWERKrna2wfzh0pu7USWO2gzlOqRK9qq`, errors.New("square/go-jose: error in cryptographic primitive")},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
patchSignatures(tt.jws)
tassert.Len(t, tt.jws.Signatures[0].Signature, 64)
expectedSignature, decodeErr := base64.RawURLEncoding.DecodeString(tt.expectedSignature)
require.NoError(t, decodeErr)
data, err := retryVerificationWithPatchedSignatures(tt.jws, tt.jwk)
if tt.expectedError != nil {
tassert.EqualError(t, err, tt.expectedError.Error())
tassert.Equal(t, expectedSignature, tt.jws.Signatures[0].Signature)
tassert.Empty(t, data)
return
}
data, err := tt.jws.Verify(tt.jwk)
tassert.NoError(t, err)
tassert.Equal(t, []byte(`{"termsOfServiceAgreed":true}`), data)
tassert.Len(t, tt.jws.Signatures[0].Signature, 64)
tassert.Equal(t, expectedSignature, tt.jws.Signatures[0].Signature)
tassert.Equal(t, tt.expectedData, data)
})
}
}

Write Preview
Loading…
Cancel
Save