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

[CRT_SH] 🐛 don't treat invalid certificates from 3rd party as active #541

Merged
merged 2 commits into from
Mar 30, 2023

Conversation

zcrt
Copy link
Contributor

@zcrt zcrt commented Mar 21, 2023

Changes

  • Fixes the todo in CRT_SH to only yield valid certificates

Issue ticket number and link

#86

Proof

Trivial

Checklist for author(s):

  • All the commits in this PR are properly PGP-signed and verified;
  • This PR comes from a feature or hotfix branch, in line with our git branching strategy;
  • This PR is "bite-sized" and only focuses on a single issue, problem, or feature;
  • I am not reinventing the wheel: there is no high-quality library that already has this feature;
  • I have changed the example .env files if I added, removed, or changed any config options, and I have informed others that they need to modify their .env files if required;
  • I have performed a self-review of my own code;
  • I have commented my code, particularly in hard-to-understand areas;
  • I have made corresponding changes to the documentation, if necessary;
  • I have written unit, integration, and end-to-end tests for the change that I made;

If a non-trivial PR:

  • This PR is part of a milestone and has appropriate labels;
  • This PR is properly linked to the project board (either directly or via an issue);
  • I have added screenshots or some other proof that my code does what it is supposed to do;
## Checklist for functional reviewer(s):
- [ ] If a non-trivial PR: This PR is properly linked to an issue on the project board;
- [ ] I have checked out this branch, and successfully ran `make kat`;
- [ ] I have ran `make test-rf` and all end-to-end Robot Framework tests pass;
- [ ] I confirmed that the PR's advertised `feature` or `hotfix` works as intended;
- [ ] I confirmed that there are no unintended functional regressions in this branch;

### What works:
* _bullet point + screenshot (if useful) per tested functionality_

### What doesn't work:
* _bullet point + screenshot (if useful) per tested functionality_

### Bug or feature?:
* _bullet point + screenshot (if useful) if it is unclear whether something is a bug or an intended feature._
## Checklist for code reviewer(s):
- [ ] The code passes the CI tests and linters;
- [ ] The code does not bypass authentication or security mechanisms;
- [ ] The code does not introduce any dependency on a library that has not been properly vetted;
- [ ] The code does not violate Model-View-Template and our other architectural principles;
- [ ] The code contains docstrings, comments, and documentation where needed;
- [ ] The code prioritizes readability over performance where appropriate;
- [ ] The code conforms to our agreed coding standards.

@zcrt zcrt requested a review from a team as a code owner March 21, 2023 16:29
@zcrt
Copy link
Contributor Author

zcrt commented Mar 22, 2023

Now that I think about it, maybe it is even better to not yield any certificates from external sources at all, but only from the certificate serving place itself. If the external source is not up-to-date and there are problems with the certificate, KAT may incorrectly deduce that the certificate for website x is fine. I am curious about your opinions on this

@noamblitz
Copy link
Contributor

Im not sure if this is the responsibility of boefjes. I think it should be the responsibility of bits. If a boefje finds a cert it should be reflected in the graph

@dekkers
Copy link
Contributor

dekkers commented Mar 30, 2023

Commits are not signed.

@dekkers dekkers merged commit f71cc28 into minvws:main Mar 30, 2023
jpbruinsslot added a commit that referenced this pull request Mar 30, 2023
* main:
  Add sudo in Debian install manual (#153)
  Fix report translation (#609)
  Feature/add organization dashboard (#481)
  [CRT_SH] 🐛 don't treat invalid certificates from 3rd party as active (#541)
  Use PostgreSQL 15 in a single container (#546)
  [CRT_SH] 🐛 don't treat invalid certificates from 3rd party as active (#610)
  Update mula design documentation (#478)
  pass on underscore domains (#550)
  Feature: Add Subfinder Boefje (#516)
  add project discovery Nuclei boefje (#518)
  Remove loop from get random objects (#558)
jpbruinsslot added a commit that referenced this pull request Mar 30, 2023
* main:
  Add sudo in Debian install manual (#153)
  Fix report translation (#609)
  Feature/add organization dashboard (#481)
  [CRT_SH] 🐛 don't treat invalid certificates from 3rd party as active (#541)
  Use PostgreSQL 15 in a single container (#546)
  [CRT_SH] 🐛 don't treat invalid certificates from 3rd party as active (#610)
  Update mula design documentation (#478)
  pass on underscore domains (#550)
  Feature: Add Subfinder Boefje (#516)
  add project discovery Nuclei boefje (#518)
  Remove loop from get random objects (#558)
  Bump urllib3 from 1.26.14 to 1.26.15 (#576)
  Bump black from 22.3.0 to 23.1.0 (#578)
  Bump iniconfig from 1.1.1 to 2.0.0 (#579)
  Add indices for Bytes (#600)
  refactor(organization lists): unify access to organizations by user (#528)
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.

4 participants