search

grindsa / acme2certifier

library implementing ACME server functionality
GNU General Public License v3.0
163 stars 33 forks source link

Account Key Rollover: Nonce validation for inner payload #42

Closed geekpex closed 4 years ago

geekpex commented 4 years ago

First of all, really nice project! But sadly I stumbled upon a little problem on key rollover functionality... Is this a bug or am I misinterpreting the RFC?

According to RFC 8555 (https://tools.ietf.org/html/rfc8555#section-7.3.5) spec the inner JWS should not contain nonce header.

o The inner JWS MUST omit the "nonce" header parameter.

When using the master, the key rollover process fails to "urn:ietf:params:acme:error:badNonce" error message in Account._key_change function and this is because it tries to validate nonce from inner JWS headers but it does not exist.

  1. Debug log of the key rollover process:

Directory._config_load() load_config(./acme/acme_srv.cfg:Directory) CAhandler._config_load() ended 127.0.0.1 /directory Directory.directory_get() [pid: 3687|app: 0|req: 7/7] 127.0.0.1 () {26 vars in 365 bytes} [Thu Jul 23 10:03:42 2020] GET /directory => generated 592 bytes in 3 msecs (HTTP/1.1 200) 1 headers in 51 bytes (1 switches on core 0) Nonce.nonce_generate_and_add() Nonce.nonce__new() got nonce: 4390a4cbd579469aaa16488ed196d625 DBStore.nonce_add(4390a4cbd579469aaa16488ed196d625) DBStore.nonce_add() ended Nonce.generate_and_add() ended with:4390a4cbd579469aaa16488ed196d625 [pid: 3687|app: 0|req: 8/8] 127.0.0.1 () {24 vars in 346 bytes} [Thu Jul 23 10:03:42 2020] HEAD /acme/newnonce => generated 0 bytes in 6 msecs (HTTP/1.1 200) 2 headers in 93 bytes (0 switches on core 0) _config_load() _config_load() Account.parse() Message.check() decode_message() Nonce.check_nonce() Nonce.nonce._check_and_delete(4390a4cbd579469aaa16488ed196d625) DBStore.nonce_check(4390a4cbd579469aaa16488ed196d625) DBStore.nonce_check() ended DBStore.nonce_delete(4390a4cbd579469aaa16488ed196d625) DBStore.nonce_delete() ended Nonce._check_and_delete() ended with:200 Nonce.check_nonce() ended with:200 Message._name_get() kid: http://localhost:8888/acme/acct/QnH7J78o8Pkj Message._name_get() returns: QnH7J78o8Pkj Signature.check(QnH7J78o8Pkj) check signature against account key Signature._jwk_load(QnH7J78o8Pkj) DBStore.jwk_load(QnH7J78o8Pkj) DBStore._account_search(column:name, pattern:QnH7J78o8Pkj) DBStore._account_search() ended with: True DBStore.jwk_load() ended with: {u'y': u'6QnBwKWfIGK2CLZSUT6E4jNRRIjKFdo45K-fOAjXtl0', u'x': u'd6x9rBDj91g7CZIPB3T8xKqL3u1P2Fj_kJShvCAfuzE', u'crv': u'P-256', u'kty': u'EC', 'alg': u'ES256'} signature_check() Signature.check() ended with: True:None Message.check() ended with:200 Account._key_change(QnH7J78o8Pkj) Message.check() decode_message() Nonce.check_nonce() Nonce.check_nonce() ended with:400 Message.check() ended with:400 Message.prepare_response() Error.enrich_error() Error.acme_errormessage(urn:ietf:params:acme:error:badNonce) Account.account_parse() returns: {"header": {}, "code": 400, "data": {"status": 400, "message": "urn:ietf:params:acme:error:badNonce", "detail": "JWS has invalid anti-replay nonce: NONE"}} [pid: 3687|app: 0|req: 9/9] 127.0.0.1 () {30 vars in 437 bytes} [Thu Jul 23 10:03:42 2020] POST /acme/key-change => generated 118 bytes in 21 msecs (HTTP/1.1 400) 1 headers in 60 bytes (1 switches on core 0)

  1. Request

{ "payload": "eyJwYXlsb2FkIjoiZXlKaFkyTnZkVzUwSWpvaWFIUjBjRG92TDJ4dlkyRnNhRzl6ZERvNE9EZzRMMkZqYldVdllXTmpkQzgzU2taUE1uUmtlVVZUUzJvaUxDSnZiR1JMWlhraU9uc2lhM1I1SWpvaVJVTWlMQ0pqY25ZaU9pSlFMVEkxTmlJc0ltRnNaeUk2SWtWVE1qVTJJaXdpZUNJNkltTmhNM2RUTFd0NmR6Smhia1JZYm1KbGRrc3hXbEZ2WVRJMWEwWkJNR2w0UVRKRVNETkxXV3hOYlRRaUxDSjVJam9pTW1Wdk1tRnhRamhLY0VGdk4xOUlXRFpSVmpsdE1FSm5kSFJQTm1sSlMxZFRPV0k1YzFjd2FVOUhZeUo5ZlEiLCJwcm90ZWN0ZWQiOiJleUpoYkdjaU9pSkZVekkxTmlJc0ltcDNheUk2ZXlKcmRIa2lPaUpGUXlJc0ltTnlkaUk2SWxBdE1qVTJJaXdpZUNJNklreGtTRTl4YTFkc1NrRTNTSFZNTkhVMk4weE1Xa0ZZVnpoUE5FdDJVMEZmTnpkR1luaFpUa0pqVVVFaUxDSjVJam9pVW1aU01teDJSVFZ1Y2sxdlh6RTJTSHB4UW1VdFoyazNYMmxpTFhaMVYzVTBlR2hEUkdOa2QwTkxOQ0o5TENKMWNtd2lPaUpvZEhSd09pOHZiRzlqWVd4b2IzTjBPamc0T0RndllXTnRaUzlyWlhrdFkyaGhibWRsSW4wIiwic2lnbmF0dXJlIjoiNGprZ19OQVJqWEtPU3pfVFFNTzZmU3lBa091SkNKVmhiT1B6TGRRd1puWUdrSUYxSFZPdFVrbm5LODBlNlk2d05OM0x6YnNfOGVnSjJFeWdBVUhHd2cifQ", "protected": "eyJhbGciOiJFUzI1NiIsImtpZCI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODg4OC9hY21lL2FjY3QvN0pGTzJ0ZHlFU0tqIiwibm9uY2UiOiI2MGIxOTIxNTY3MGU0YzI3YjIyMjAyNTYyNWFiY2ZjOCIsInVybCI6Imh0dHA6Ly9sb2NhbGhvc3Q6ODg4OC9hY21lL2tleS1jaGFuZ2UifQ", "signature": "CbJsmjU4eK6PojorSzJ8_mcauT93xbcpOxOlEvPKCGPkwsnpQoXZdFyCaPaKWGh1G6yuZiswwPIy1JFK1clCXQ" }

  1. Decoded request

{ "protected": base64url({ "alg": "ES256", "kid": "http://localhost:8888/acme/acct/7JFO2tdyESKj", "nonce": "60b19215670e4c27b222025625abcfc8", "url": "http://localhost:8888/acme/key-change" }), "payload": base64url({ "protected": base64url({ "alg": "ES256", "jwk": { "kty": "EC", "crv": "P-256", "x": "LdHOqkWlJA7HuL4u67LLZAXW8O4KvSA_77FbxYNBcQA", "y": "RfR2lvE5nrMo_16HzqBe-gi7_ib-vuWu4xhCDcdwCK4" }, "url": "http://localhost:8888/acme/key-change" }), "payload": base64url({ "account": "http://localhost:8888/acme/acct/7JFO2tdyESKj", "oldKey": { "kty": "EC", "crv": "P-256", "alg": "ES256", "x": "ca3wS-kzw2anDXnbevK1ZQoa25kFA0ixA2DH3KYlMm4", "y": "2eo2aqB8JpAo7_HX6QV9m0BgttO6iIKWS9b9sW0iOGc" } }), "signature": "4jkg_NARjXKOSz_TQMO6fSyAkOuJCJVhbOPzLdQwZnYGkIF1HVOtUknnK80e6Y6wNN3Lzbs_8egJ2EygAUHGwg" }), "signature": "CbJsmjU4eK6PojorSzJ8_mcauT93xbcpOxOlEvPKCGPkwsnpQoXZdFyCaPaKWGh1G6yuZiswwPIy1JFK1clCXQ" }

I solved the problem by creating own message.check function for key rollover process, which does not contain the nonce check (maybe not the most elegant solution).

def check_key_change_payload(self, content, use_emb_key=False):
    """ validate message """
   self.logger.debug('Message.check()')

    # disable signature check if paramter has been set
    if self.disable_dic['signature_check_disable']:
        print('**** SIGNATURE_CHECK_DISABLE!!! Security issue ****')
        skip_signature_check = True
    else:
        skip_signature_check = False

    # decode message
    (result, error_detail, protected, payload, _signature) = decode_message(self.logger, content)
    account_name = None
    if result:
        # nonce check is not needed because it must not exist in key change payload's headers

        if not skip_signature_check:
            # nonce check successful - check signature
            account_name = self._name_get(protected)
            signature = Signature(self.debug, self.server_name, self.logger)
            # we need the decoded protected header to grab a key to verify signature
            (sig_check, error, error_detail) = signature.check(account_name, content, use_emb_key, protected)
            if sig_check:
                code = 200
                message = None
                detail = None
            else:
                code = 403
                message = error
                detail = error_detail
    else:
        # message could not get decoded
        code = 400
        message = 'urn:ietf:params:acme:error:malformed'
        detail = error_detail

    self.logger.debug('Message.check() ended with:{0}'.format(code))
    return(code, message, detail, protected, payload, account_name)
grindsa commented 4 years ago

Thx for your feedback. Which client are you using to rollover the account-key?

geekpex commented 4 years ago

I'm creating ACME integration to our system that uses https://github.com/go-acme/lego as a base. But sadly it does not include the key rollover functionality so I'm adding the support based on the rfc.

And I just noticed that when I'm talking about "inner payload" I mean the inner JWS object that is the payload.

grindsa commented 4 years ago

your understanding is correct. I pushed a fix into devel branch which should skip the nonce-check of the inner payload during key-rollover. I tested with acmeshell for verifyication (see below log). Give it at try and me know if it works for you.

/G.

'2020-07-25 19:29:17 - acme2certifier - DEBUG - _config_load()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - _config_load()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account.parse()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message.check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - decode_message()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.check_nonce()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.nonce._check_and_delete(d2394997432049cca8dd71b27d30eef7)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_check(d2394997432049cca8dd71b27d30eef7)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_check() ended'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_delete(d2394997432049cca8dd71b27d30eef7)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_delete() ended'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce._check_and_delete() ended with:200'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.check_nonce() ended with:200'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message._name_get()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - kid: http://acme-srv.bar.local/acme/acct/ZfsImvDUDbXS'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message._name_get() returns: ZfsImvDUDbXS'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Signature.check(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - check signature against account key'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Signature._jwk_load(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.jwk_load(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search(column:name, pattern:ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search() ended with: True'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.jwk_load() ended with: {'kty': 'EC', 'crv': 'P-256', 'x': 'qJ63in1wJ3skC4pjD8o1_bKXoJ635F-_tKFVDF3wd3k', 'y': 'w-qFkcSp52dJj6gS3wxDwZrM51ccdQzLJEteK64NIgo', 'alg': 'ES256'}'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - signature_check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Signature.check() ended with: True:None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message.check() ended with:200'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._key_change(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message.check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - decode_message()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - skip nonce check of inner payload during keyrollover'     <---------------
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message._name_get()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message._name_get() returns: None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Signature.check(None)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - check signature against key includedn in jwk'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - signature_check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Signature.check() ended with: True:None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message.check() ended with:200'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._key_change_validate(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._lookup(jwk:{"kty": "EC", "crv": "P-256", "x": "o8kTd3bJLbFWXjBizyImz7z7OvKc3MO4KqqmDrYJ4d4", "y": "hCoYZ96zg1veD6nOvUIiDGl8dhE97NffJQdGktHMhAs"})'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.account_lookup(column:jwk, pattern:{"kty": "EC", "crv": "P-256", "x": "o8kTd3bJLbFWXjBizyImz7z7OvKc3MO4KqqmDrYJ4d4", "y": "hCoYZ96zg1veD6nOvUIiDGl8dhE97NffJQdGktHMhAs"})'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search(column:jwk, pattern:{"kty": "EC", "crv": "P-256", "x": "o8kTd3bJLbFWXjBizyImz7z7OvKc3MO4KqqmDrYJ4d4", "y": "hCoYZ96zg1veD6nOvUIiDGl8dhE97NffJQdGktHMhAs"})'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search() ended with: False'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.account_lookup() ended'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._inner_jws_check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._inner_jws_check() ended with: 200:None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._inner_payload_check()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._key_compare(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.jwk_load(ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search(column:name, pattern:ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search() ended with: True'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.jwk_load() ended with: {'kty': 'EC', 'crv': 'P-256', 'x': 'qJ63in1wJ3skC4pjD8o1_bKXoJ635F-_tKFVDF3wd3k', 'y': 'w-qFkcSp52dJj6gS3wxDwZrM51ccdQzLJEteK64NIgo', 'alg': 'ES256'}'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._key_compare() ended with: 200'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._inner_payload_check() ended with: 200:None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account._key_change_validate() ended with: 200:None'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.account_update({'name': 'ZfsImvDUDbXS', 'jwk': '{"kty": "EC", "crv": "P-256", "x": "o8kTd3bJLbFWXjBizyImz7z7OvKc3MO4KqqmDrYJ4d4", "y": "hCoYZ96zg1veD6nOvUIiDGl8dhE97NffJQdGktHMhAs"}'})'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search(column:name, pattern:ZfsImvDUDbXS)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore._account_search() ended with: True'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.account_update() ended'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Message.prepare_response()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.nonce_generate_and_add()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.nonce__new()'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - got nonce: b4d64e0084f642c99841dacd970d8f31'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_add(b4d64e0084f642c99841dacd970d8f31)'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - DBStore.nonce_add() ended'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Nonce.generate_and_add() ended with:b4d64e0084f642c99841dacd970d8f31'
'2020-07-25 19:29:17 - acme2certifier - DEBUG - Account.account_parse() returns: {"data": {}, "code": 200, "header": {"Replay-Nonce": "b4d64e0084f642c99841dacd970d8f31"}}'
'2020-07-25 19:29:17 - acme2certifier - INFO - 192.168.14.131 /acme/key-change {'data': {}, 'code': 200, 'header': {'Replay-Nonce': '- modified -'}}'