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

Fix: 4xx and 5xx responses from API Gateway cause Invicti findings (DataBiosphere/azul-private#154) #6385

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions .github/pull_request_template.md.template.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
OrderedSet,
)
from azul.strings import (
back_quote as bq,
join_grammatically,
)
from azul.template import (
Expand Down Expand Up @@ -225,10 +226,6 @@ def shared_deploy_target(self, target_branch: str) -> str:
return 'apply' + iif(self.shared_deploy_is_two_phase(target_branch), '_keep_unused')


def bq(s):
return '`' + s + '`'


def main():
path = Path(sys.argv[1])
for t in T:
Expand Down
5 changes: 4 additions & 1 deletion scripts/convert_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
from azul.files import (
write_file_atomically,
)
from azul.strings import (
single_quote as sq,
)


class Variable(NamedTuple):
Expand Down Expand Up @@ -144,7 +147,7 @@ def sub(m: re.Match):
return '{{' + m[1] + '}}'

value = re.sub(r'\$?{([^}]+)}|\$([_A-Za-z][_A-Za-z0-9]*)', sub, value)
return f"'{value}'"
return sq(value)


if __name__ == '__main__':
Expand Down
9 changes: 4 additions & 5 deletions src/azul/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
Union,
)

from azul import (
require,
from azul.strings import (
back_quote as bq,
)

BigQueryValue = Union[int, float, bool, str, bytes, datetime, None]
Expand Down Expand Up @@ -42,10 +42,9 @@ def backtick(table_name: str) -> str:
>>> backtick('foo-2.bar`s.my_table')
Traceback (most recent call last):
...
azul.RequirementError: foo-2.bar`s.my_table
azul.RequirementError: ('`', 'must not occur in', 'foo-2.bar`s.my_table')
"""
if table_name_re.fullmatch(table_name):
return table_name
else:
require('`' not in table_name, table_name)
return f'`{table_name}`'
return bq(table_name)
30 changes: 26 additions & 4 deletions src/azul/chalice.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@
copy_json,
json_head,
)
from azul.strings import (
join_words as jw,
single_quote as sq,
)
from azul.types import (
JSON,
LambdaContext,
Expand Down Expand Up @@ -189,15 +193,33 @@ def _api_gateway_context_middleware(self, event, get_response):
finally:
config.lambda_is_handling_api_gateway_request = False

hsts_max_age = 60 * 60 * 24 * 365 * 2

# Headers added to every response from the app, as well as canned 4XX and
# 5XX responses from API Gateway. Use of these headers addresses known
Copy link
Contributor

Choose a reason for hiding this comment

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

The non-semantic changes should be moved to a fixup that immediately follows the 3rd commit (where this paragraph was first added).

# security vulnerabilities.
#
security_headers = {
'Content-Security-Policy': jw('default-src', sq('self')),
'Referrer-Policy': 'strict-origin-when-cross-origin',
'Strict-Transport-Security': jw(f'max-age={hsts_max_age};',
'includeSubDomains;',
'preload'),
'X-Content-Type-Options': 'nosniff',
Fixed Show fixed Hide fixed
'X-Frame-Options': 'DENY',
'X-XSS-Protection': '1; mode=block'
}

def _security_headers_middleware(self, event, get_response):
"""
Add headers to the response
"""
response = get_response(event)
seconds = 60 * 60 * 24 * 365
response.headers['Strict-Transport-Security'] = f'max-age={seconds}; includeSubDomains'
response.headers['X-Content-Type-Options'] = 'nosniff'
response.headers['X-Frame-Options'] = 'DENY'
response.headers.update(self.security_headers)
# FIXME: Add a CSP header with a nonce value to text/html responses
# https://github.com/DataBiosphere/azul-private/issues/6
if response.headers.get('Content-Type') == 'text/html':
del response.headers['Content-Security-Policy']
view_function = self.routes[event.path][event.method].view_function
cache_control = getattr(view_function, 'cache_control')
response.headers['Cache-Control'] = cache_control
Expand Down
10 changes: 5 additions & 5 deletions src/azul/plugins/repository/tdr_hca/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
TDRBundleFQID,
TDRPlugin,
)
from azul.strings import (
single_quote as sq,
)
from azul.terra import (
TDRSourceRef,
TDRSourceSpec,
Expand Down Expand Up @@ -439,22 +442,19 @@ def _retrieve_entities(self,
table_name = backtick(self._full_table_name(source, entity_type))
entity_id_type = one(set(map(type, entity_ids)))

def quote(s):
return f"'{s}'"

if entity_type == 'links':
assert issubclass(entity_id_type, BundleFQID), entity_id_type
entity_ids = cast(set[BundleFQID], entity_ids)
where_columns = (pk_column, version_column)
where_values = (
(quote(fqid.uuid), f'TIMESTAMP({quote(fqid.version)})')
(sq(fqid.uuid), f'TIMESTAMP({sq(fqid.version)})')
for fqid in entity_ids
)
expected = {fqid.uuid for fqid in entity_ids}
else:
assert issubclass(entity_id_type, str), (entity_type, entity_id_type)
where_columns = (pk_column,)
where_values = ((quote(entity_id),) for entity_id in entity_ids)
where_values = ((sq(str(entity_id)),) for entity_id in entity_ids)
expected = entity_ids
query = f'''
SELECT {', '.join({pk_column, *non_pk_columns})}
Expand Down
6 changes: 4 additions & 2 deletions src/azul/service/manifest_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,9 @@
StorageObjectNotFound,
StorageService,
)
from azul.strings import (
double_quote as dq,
)
from azul.types import (
AnyJSON,
FlatJSON,
Expand Down Expand Up @@ -887,9 +890,8 @@ def _cmd_exe_quote(cls, s: str) -> str:
"""
Escape a string for insertion into a `cmd.exe` command line
"""
assert '"' not in s, s
assert '\\' not in s, s
return f'"{s}"'
return dq(s)

@classmethod
def command_lines(cls,
Expand Down
110 changes: 110 additions & 0 deletions src/azul/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
minmax,
)

from azul import (
reject,
)


def to_camel_case(text: str) -> str:
camel_cased = ''.join(part.title() for part in text.split('_'))
Expand Down Expand Up @@ -223,3 +227,109 @@ def longest_common_prefix(strings: Iterable[str]) -> Optional[str]:
if s2[i] != c:
return s1[:i]
return s1


def join_lines(*lines: str) -> str:
"""
Join the arguments with a newline character.

>>> join_lines()
''

>>> join_lines('a')
'a'

>>> join_lines('a', 'b')
'a\\nb'
"""
return '\n'.join(lines)


def join_words(*words: str) -> str:
"""
Join the arguments with a space character.

>>> join_words()
''

>>> join_words('a')
'a'

>>> join_words('a', 'b')
'a b'
"""
return ' '.join(words)


def delimit(s: str, delimiter: str) -> str:
"""
Prepend and append a delimiter to a string after ensuring that the former
does not occur in the latter.

>>> delimit('foo', "'")
"'foo'"

>>> delimit("foo's", "'")
Traceback (most recent call last):
...
azul.RequirementError: ("'", 'must not occur in', "foo's")
"""
reject(delimiter in s, delimiter, 'must not occur in', s)
return delimiter + s + delimiter


def back_quote(*words: str) -> str:
"""
Join the arguments with a space character and enclose the result in back
quotes. The arguments must not contain back quotes.

>>> back_quote()
'``'

>>> back_quote('foo', 'bar')
'`foo bar`'

>>> back_quote('foo`s')
Traceback (most recent call last):
...
azul.RequirementError: ('`', 'must not occur in', 'foo`s')
"""
return delimit(join_words(*words), '`')


def single_quote(*words: str) -> str:
"""
Join the arguments with a space character and enclose the result in single
quotes. The arguments must not contain single quotes.

>>> single_quote()
"''"

>>> single_quote('foo', 'bar')
"'foo bar'"

>>> single_quote("foo", "bar's")
Traceback (most recent call last):
...
azul.RequirementError: ("'", 'must not occur in', "foo bar's")
"""
return delimit(join_words(*words), "'")


def double_quote(*words: str) -> str:
"""
Join the arguments with a space character and enclose the result in double
quotes. The arguments must not contain double quotes.

>>> double_quote()
'""'

>>> double_quote('foo', 'bar')
'"foo bar"'

>>> double_quote('foo', 'b"a"r')
Traceback (most recent call last):
...
azul.RequirementError: ('"', 'must not occur in', 'foo b"a"r')
"""
return delimit(join_words(*words), '"')
28 changes: 27 additions & 1 deletion src/azul/terraform.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
config,
require,
)
from azul.chalice import (
AzulChaliceApp,
)
from azul.deployment import (
aws,
)
Expand Down Expand Up @@ -802,11 +805,34 @@ def tf_config(self, app_name):
# Setting this property using AWS API Gateway extensions to the OpenAPI
# specification works around this issue.
#
# We ran into similar difficulties when using Terraform to configure
# default responses for the API, so we use these extensions for that
# purpose, too.
#
openapi_spec = json.loads(locals[app_name])
rest_api = resources['aws_api_gateway_rest_api'][app_name]
assert 'minimum_compression_size' not in rest_api, rest_api
openapi_spec = json.loads(locals[app_name])
key = 'x-amazon-apigateway-minimum-compression-size'
openapi_spec[key] = config.minimum_compression_size
assert 'aws_api_gateway_gateway_response' not in resources, resources
openapi_spec['x-amazon-apigateway-gateway-responses'] = {
f'DEFAULT_{response_type}': {
'responseParameters': {
# Static value response header parameters must be enclosed
# within a pair of single quotes.
#
# https://docs.aws.amazon.com/apigateway/latest/developerguide/request-response-data-mappings.html#mapping-response-parameters
# https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-swagger-extensions-gateway-responses.html
#
# Note that azul.strings.single_quote() is not used here
# since API Gateway allows internal single quotes in the
# value, which that function would prohibit.
#
f'gatewayresponse.header.{k}': f"'{v}'"
for k, v in AzulChaliceApp.security_headers.items()
}
} for response_type in ['4XX', '5XX']
}
locals[app_name] = json.dumps(openapi_spec)

return {
Expand Down
Loading
Loading