Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tenant, web_util: ensure that the content type is actually application/json #845

Merged
merged 2 commits into from Jan 24, 2022

Conversation

THS-on
Copy link
Member

@THS-on THS-on commented Jan 21, 2022

The rust agent requires Content-Type application/json to be set.
This did not happen in two places:

  • tornado_requests set the content type to application/x-www-form-urlencoded, we now manually set it for JSON data.
  • In the tenant the data option was used instead of json for the RequestClient.

@ueno
Copy link
Contributor

ueno commented Jan 21, 2022

@THS-on yes, this fixes the issue (new capture with this change).

The rust agent requires Content-Type application/json to be set.

Signed-off-by: Thore Sommer <mail@thson.de>
@THS-on
Copy link
Member Author

THS-on commented Jan 21, 2022

As you pointed out the Tornado responses from the verifier had application/x-www-form-urlencoded instead of application/json set as content type.
This should now also be fixed.

@THS-on THS-on changed the title tenant: post U key to agent with correct content type header tenant, web_util: ensure that the content type is actually application/json. Jan 21, 2022
@THS-on THS-on changed the title tenant, web_util: ensure that the content type is actually application/json. tenant, web_util: ensure that the content type is actually application/json Jan 21, 2022
@ueno
Copy link
Contributor

ueno commented Jan 21, 2022

As you pointed out the Tornado responses from the verifier had application/x-www-form-urlencoded instead of application/json set as content type. This should now also be fixed.

It was actually a vkey request rather than the response; I guess it still needs a fix around the call to tornado_requests.request in cloud_verifier_tornado.py:invoke_provide_v.

@THS-on THS-on marked this pull request as draft January 21, 2022 08:32
@THS-on THS-on marked this pull request as ready for review January 21, 2022 10:51
@THS-on
Copy link
Member Author

THS-on commented Jan 21, 2022

@ueno yes you are right. I've now also changed that. Now hopefully I caught every instance where we don't set the correct header.

@THS-on
Copy link
Member Author

THS-on commented Jan 21, 2022

/packit test

keylime/web_util.py Outdated Show resolved Hide resolved
res = web_util.echo_json_response(mock_handler, 200, "Success", test_data)
self.assertTrue(res)
mock_handler.set_status.assert_called_once_with(200)
mock_handler.set_header.assert_called_once_with('Content-Type', 'application/json')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're taking out the check of the mock for the method call, but is there another way we can test that the Content-Type header is set correctly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really because this is set by the handler which we are mocking. (I can also mock that, but this defeats the purpose of the test)

@keylime-bot keylime-bot assigned maugustosilva and unassigned THS-on Jan 21, 2022
@THS-on
Copy link
Member Author

THS-on commented Jan 21, 2022

/packit test

@mpeters
Copy link
Member

mpeters commented Jan 21, 2022

The test using a tenant command is failing:

:: [ 18:45:33 ] :: [  BEGIN   ] :: Running 'lime_keylime_tenant -c cvlist'
Reading configuration from ['/etc/keylime.conf']
2022-01-21 18:45:33.652 - keylime.tpm - INFO - TPM2-TOOLS Version: 5.2
2022-01-21 18:45:33.654 - keylime.tenant - INFO - Setting up client TLS in /var/lib/keylime/cv_ca
2022-01-21 18:45:33.654 - keylime.tenant - WARNING - Using default UUID d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/lib/python3.10/site-packages/urllib3/connectionpool.py:1013: InsecureRequestWarning: Unverified HTTPS request is being made to host '127.0.0.1'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/1.26.x/advanced-usage.html#ssl-warnings
  warnings.warn(
2022-01-21 18:45:33.678 - keylime.tenant - INFO - Status command response: 500. Unexpected response from Cloud Verifier 127.0.0.1 on port 8881. <Response [500]>
:: [ 18:45:34 ] :: [   PASS   ] :: Command 'lime_keylime_tenant -c cvlist' (Expected 0, got 0)
:: [ 18:45:34 ] :: [   FAIL   ] :: File '/var/tmp/rlRun_LOG.a42YeXiI' should contain '{'code': 200, 'status': 'Success', 'results': {'uuids':.*'d432fbb3-d2f1-4a97-9ef7-75bd81c00000'' 

@kkaarreell Any way to know what the verifier log contains for that 500 error?

@kkaarreell
Copy link
Contributor

Not from CI. I would recommend running it using tmt tool as described at the very end of
https://github.com/RedHat-SP-Security/keylime-tests/blob/docmulti/TESTING.md
Also, see the section about using virtual system regarding the required setup.
Logs are stored in /var/tmp/limeLib.

@kkaarreell
Copy link
Contributor

Pasting below the content of verifier.log

Reading configuration from ['/etc/keylime.conf']
2022-01-23 14:45:44.065 - keylime.keylime_db - INFO - database_url is not set, using multi-parameter database configuration options
2022-01-23 14:45:44.121 - keylime.keylime_db - INFO - database_url is not set, using multi-parameter database configuration options
2022-01-23 14:45:44.122 - alembic.env - INFO - Migrating database cloud_verifier
2022-01-23 14:45:44.122 - alembic.runtime.migration - INFO - Context impl SQLiteImpl.
2022-01-23 14:45:44.122 - alembic.runtime.migration - INFO - Will assume non-transactional DDL.
2022-01-23 14:45:44.130 - alembic.runtime.migration - INFO - Running upgrade  -> 8a44a4364f5a, Initial database migration
2022-01-23 14:45:44.134 - alembic.runtime.migration - INFO - Running upgrade 8a44a4364f5a -> eeb702f77d7d, allowlist rename
2022-01-23 14:45:44.142 - alembic.runtime.migration - INFO - Running upgrade eeb702f77d7d -> 8da20383f6e1, extend_ip_field
2022-01-23 14:45:44.146 - alembic.runtime.migration - INFO - Running upgrade 8da20383f6e1 -> a7a64155ab3a, Add ima_sign_verification_keys column
2022-01-23 14:45:44.147 - alembic.runtime.migration - INFO - Running upgrade a7a64155ab3a -> cc2630851a1f, receive the aik_tpm from the agent
2022-01-23 14:45:44.147 - alembic.runtime.migration - INFO - Running upgrade cc2630851a1f -> ae898986c6e9, add_mb_refstate_column
2022-01-23 14:45:44.148 - alembic.runtime.migration - INFO - Running upgrade ae898986c6e9 -> eb869a77abd1, create_allowlist_table
2022-01-23 14:45:44.156 - alembic.runtime.migration - INFO - Running upgrade eb869a77abd1 -> b4d024197413, add_verfier_id_to_verifiermain_table
2022-01-23 14:45:44.157 - alembic.runtime.migration - INFO - Running upgrade b4d024197413 -> f82c4252bc4f, Add ip and port to registrar
2022-01-23 14:45:44.158 - alembic.runtime.migration - INFO - Running upgrade f82c4252bc4f -> 7d5db1a6ffb0, Add agentstates columns
2022-01-23 14:45:44.159 - alembic.runtime.migration - INFO - Running upgrade 7d5db1a6ffb0 -> f35cdd35eb83, Move (v)tpm_policy to JSONPickleType
2022-01-23 14:45:44.172 - alembic.runtime.migration - INFO - Running upgrade f35cdd35eb83 -> 257fe0f0c039, Add fields for revocation context to verifier
2022-01-23 14:45:44.173 - alembic.runtime.migration - INFO - Running upgrade 257fe0f0c039 -> c3842cc9ee69, Store keyrings learned from IMA log
2022-01-23 14:45:44.182 - keylime.cloudverifier - INFO - Starting Cloud Verifier (tornado) on port 8881, use <Ctrl-C> to stop
2022-01-23 14:45:44.183 - keylime.cloudverifier - INFO - Current API version 1.0
2022-01-23 14:45:44.183 - keylime.cloudverifier - INFO - Setting up TLS...
2022-01-23 14:45:44.183 - keylime.cloudverifier - INFO - Existing CA certificate found in /var/lib/keylime/cv_ca, not generating a new one
2022-01-23 14:45:44.185 - tornado.general - INFO - Starting 1 processes
2022-01-23 14:45:44.187 - keylime.cloudverifier - INFO - Starting service for revocation notifications on port 8992
2022-01-23 14:45:52.340 - tornado.access - INFO - 200 POST /v1.0/agents/d432fbb3-d2f1-4a97-9ef7-75bd81c00000 (127.0.0.1) 18.30ms
2022-01-23 14:45:52.341 - keylime.cloudverifier - INFO - POST returning 200 response for adding agent id: d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py:852: SAWarning: TypeDecorator JSONPickleType() will not produce a cache key because the ``cache_ok`` attribute is not set to True.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this type object's state is safe to use in a cache key, or False to disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  agent_id=agent_db['agent_id']).update(agent_db)
2022-01-23 14:45:52.501 - keylime.registrar_client - WARNING - TLS is enabled.
2022-01-23 14:45:52.501 - keylime.registrar_client - INFO - Setting up client TLS...
/usr/lib/python3.10/site-packages/urllib3/connectionpool.py:1013: InsecureRequestWarning: Unverified HTTPS request is being made to host '127.0.0.1'. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/1.26.x/advanced-usage.html#ssl-warnings
  warnings.warn(
2022-01-23 14:45:52.568 - keylime.tpm - INFO - TPM2-TOOLS Version: 5.2
2022-01-23 14:45:52.579 - keylime.tpm - INFO - Checking IMA measurement list on agent: d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py:852: SAWarning: TypeDecorator JSONPickleType() will not produce a cache key because the ``cache_ok`` attribute is not set to True.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this type object's state is safe to use in a cache key, or False to disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  agent_id=agent_db['agent_id']).update(agent_db)
2022-01-23 14:45:52.902 - keylime.cloudverifier_common - WARNING - Non-fatal problem ocurred while attempting to evaluate agent attribute "mb_refstate" (('malformed node or string on line 1: <ast.Name object at 0x7f0b53fefca0>',)). Will just consider the value of this attribute to be "None"
2022-01-23 14:45:52.908 - tornado.access - INFO - 200 GET /v1.0/agents/d432fbb3-d2f1-4a97-9ef7-75bd81c00000 (127.0.0.1) 17.98ms
2022-01-23 14:45:54.210 - keylime.cloudverifier_common - WARNING - Non-fatal problem ocurred while attempting to evaluate agent attribute "mb_refstate" (('malformed node or string on line 1: <ast.Name object at 0x7f0b53fd3ca0>',)). Will just consider the value of this attribute to be "None"
2022-01-23 14:45:54.216 - tornado.access - INFO - 200 GET /v1.0/agents/d432fbb3-d2f1-4a97-9ef7-75bd81c00000 (127.0.0.1) 39.43ms
2022-01-23 14:45:54.732 - keylime.tpm - INFO - Checking IMA measurement list on agent: d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py:852: SAWarning: TypeDecorator JSONPickleType() will not produce a cache key because the ``cache_ok`` attribute is not set to True.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this type object's state is safe to use in a cache key, or False to disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  agent_id=agent_db['agent_id']).update(agent_db)
2022-01-23 14:45:55.500 - keylime.cloudverifier_common - WARNING - Non-fatal problem ocurred while attempting to evaluate agent attribute "mb_refstate" (('malformed node or string on line 1: <ast.Name object at 0x7f0b53fed150>',)). Will just consider the value of this attribute to be "None"
2022-01-23 14:45:55.506 - tornado.access - INFO - 200 GET /v1.0/agents/d432fbb3-d2f1-4a97-9ef7-75bd81c00000 (127.0.0.1) 16.63ms
2022-01-23 14:45:55.793 - tornado.application - ERROR - Uncaught exception GET /v1.0/agents/?verifier= (127.0.0.1)
HTTPServerRequest(protocol='https', host='127.0.0.1:8881', method='GET', uri='/v1.0/agents/?verifier=', version='HTTP/1.1', remote_ip='127.0.0.1')
Traceback (most recent call last):
  File "/usr/lib64/python3.10/site-packages/tornado/web.py", line 1702, in _execute
    result = method(*self.path_args, **self.path_kwargs)
  File "/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py", line 289, in get
    web_util.echo_json_response(self, 200, "Success", {
  File "/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/web_util.py", line 146, in echo_json_response
    handler.write(json_res)  # This will set the Content-Type header automatically to application/json
  File "/usr/lib64/python3.10/site-packages/tornado/web.py", line 843, in write
    chunk = escape.json_encode(chunk)
  File "/usr/lib64/python3.10/site-packages/tornado/escape.py", line 75, in json_encode
    return json.dumps(value).replace("</", "<\\/")
  File "/usr/lib64/python3.10/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/lib64/python3.10/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib64/python3.10/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/lib64/python3.10/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type Row is not JSON serializable
2022-01-23 14:45:55.794 - tornado.access - ERROR - 500 GET /v1.0/agents/?verifier= (127.0.0.1) 2.39ms
2022-01-23 14:45:56.811 - keylime.tpm - INFO - Checking IMA measurement list on agent: d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py:852: SAWarning: TypeDecorator JSONPickleType() will not produce a cache key because the ``cache_ok`` attribute is not set to True.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this type object's state is safe to use in a cache key, or False to disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  agent_id=agent_db['agent_id']).update(agent_db)
2022-01-23 14:45:58.893 - keylime.tpm - INFO - Checking IMA measurement list on agent: d432fbb3-d2f1-4a97-9ef7-75bd81c00000
/usr/local/lib/python3.10/site-packages/keylime-0.0.0-py3.10.egg/keylime/cloud_verifier_tornado.py:852: SAWarning: TypeDecorator JSONPickleType() will not produce a cache key because the ``cache_ok`` attribute is not set to True.  This can have significant performance implications including some performance degradations in comparison to prior SQLAlchemy versions.  Set this attribute to True if this type object's state is safe to use in a cache key, or False to disable this warning. (Background on this error at: https://sqlalche.me/e/14/cprf)
  agent_id=agent_db['agent_id']).update(agent_db)

Dumping the data first to a string caused the Content-Type header to be set
to application/x-www-form-urlencoded. Now if the request function gets a
dict, it is converted to a JSON string and the Content-Type header is set
to application/json.

Signed-off-by: Thore Sommer <mail@thson.de>
@THS-on
Copy link
Member Author

THS-on commented Jan 24, 2022

I've dropped the commit that changed echo_json_response. Your json module adds handling for SQLAlchemy objects which the internal does not provide.

@mpeters mpeters merged commit 4c916c1 into keylime:master Jan 24, 2022
@THS-on THS-on deleted the tenant-json-header branch February 6, 2022 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants