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

Dev additions #18

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Dev additions #18

wants to merge 36 commits into from

Conversation

mattkrau
Copy link
Collaborator

As talked about in the weekly meeting, rebased to master:

Adds following functionalities:

  • Scans for Subresource Integrity Check
  • Additional scans for serverleaks
  • Reading of GeneratorTags

@mattkrau mattkrau requested a review from hprid February 27, 2019 16:05
@hprid
Copy link
Member

hprid commented Feb 28, 2019

Thank you for your pull request. We discussed most of it already in person, but for the record here are the most important points:

  • Keep your code PEP8 compliant (with an exception of line length, which can be about 90).
  • Use different branches for different features and provide several pull requests. This helps to review individual changes and makes discussions easier, since every discussion belongs to one topic.
  • Resolve all conflicts and rebase your branch to the current master.

Regarding SRI

  • Do not add pseudo headers to the security headers. Instead, provide these information in the dictionary that is already returned for the CSP header.
  • Search for script/style tags and their integrity attribute and provide a list of resources that include and/or miss SRI. Include whether SRI failed (you can consider the block reason of a failed request, see Page.failed_request_log).
  • When searching for nodes, use the scripts_disabled context manager to avoid changes to the DOM while searching.

Regarding the generator tag

  • Remove code that is not used and rename variables that were copied from other codes and give them names matching the context.
  • You do not need to use the search function to search the page for "Generator". Instead, you can use a more specific selector, e.g., meta[name="generator" i]. This will return all nodes you are interested in
  • When search for nodes, use the scripts_disabled context manager to avoid changes to the DOM while searching.

Regarding additional serverleaks

  • _match_package_file uses "binary or" in the loop instead of "logical or". You could replace this loop with the any function together with an generator expression, i.e., return any(target in content for target in targets). Use the beauty of Python :-)

mattkrau added 20 commits April 17, 2019 13:49
…omedevtools. Trying POC to confirm hypothesis.
+
Started reworking result from list to dict
Added "all-sri-active-valid" key (defaulting to false) to quickly show whether all scripts/styles require sri and/or are both active and/or valid
… flag #enable-experimental-web-platform-features is enabled, it correctly throws an error if a script / style has no integrity-hash.

This is now accounted for.
+ all-sri-active-valid is now evaluated and set correctly
- Reworked / removed unused code.
- Speed up the module
- Corrected behaviour if no errors were found
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.

2 participants