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

Refactor for IO independent protocol implementation #1127

Merged
merged 105 commits into from
Jun 14, 2024

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented Mar 6, 2024

What

Refactor python-gvm to provide an I/O independent implementation of GMP and drop support for GMP < 22.4.

An IO independent protocol means it is possible to create data for requests. This data is represented as bytes. The bytes can be send via I/O and the response is returned again as bytes.

Currently this doesn't bring much value besides being able to split the actual request generating code into smaller peaces. But at the end it would allow to not only provide a sync implementation but also async implementation of GMP.

Additional Changes:

  • removed obsolete ifaces and ifaces_allow arguments from create_user and modify_user
  • removed obsolete OVALDef APIs
  • removed modify_scan_config method
  • removed thread arguments from create_override, modify_override, create_note and modify_note
  • removed SeverityLevel enum as it was used only for the thread arguments
  • full typing support
  • mypy runs without issues

Why

Abstract the request/response generating code from the actual transport of the protocol.

With this PR it is easier again to add version specific protocol code. It requires to add a new gvm.protocols.gmp.requests sub module for a new protocol version and to add a protocol class in a GMP version specific new sub module of gvm.protocols.gmp.

Checklist

  • Tests

@bjoernricks bjoernricks changed the title Refactor for independent protocol implementation Refactor for IO independent protocol implementation Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

Conventional Commits Report

Type Number
Removed 4
Changed 14
Added 25
Bug Fixes 2

🚀 Conventional commits found.

gvm/protocols/gmp/core/_connection.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_connection.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_connection.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_connection.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_connection.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_request.py Fixed Show fixed Hide fixed
gvm/protocols/gmp/core/_request.py Fixed Show fixed Hide fixed
gvm/connections/_connection.py Fixed Show fixed Hide fixed
gvm/connections/_connection.py Fixed Show fixed Hide fixed
gvm/connections/_connection.py Fixed Show fixed Hide fixed
gvm/connections/_connection.py Fixed Show fixed Hide fixed
gvm/connections/_connection.py Fixed Show fixed Hide fixed
gvm/connections/_ssh.py Dismissed Show dismissed Hide dismissed
@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch from 8b98e23 to 91f98e2 Compare March 11, 2024 15:53
gvm/protocols/core/_connection.py Dismissed Show dismissed Hide dismissed
gvm/protocols/core/_connection.py Dismissed Show dismissed Hide dismissed
gvm/protocols/core/_connection.py Dismissed Show dismissed Hide dismissed
gvm/protocols/core/_connection.py Dismissed Show dismissed Hide dismissed
gvm/protocols/core/_connection.py Dismissed Show dismissed Hide dismissed
gvm/protocols/core/_request.py Dismissed Show dismissed Hide dismissed
Copy link

github-actions bot commented Mar 22, 2024

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ❌ 1 package(s) with invalid SPDX license definitions
  • ✅ 0 package(s) with unknown licenses.
See the Details below.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA a771cbd.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

License Issues

poetry.lock

PackageVersionLicenseIssue Type
lxml-stubs0.5.1Apache-2.0 AND MIT AND NOASSERTIONInvalid SPDX License
Allowed Licenses: 0BSD, AGPL-3.0-or-later, GPL-3.0-or-later, LGPL-2.1, EPL-2.0, Python-2.0, GPL-2.0-or-later, GPL-2.0-only, GPL-3.0-or-later AND LGPL-2.1-only, GPL-3.0-or-later AND LGPL-3.0 AND LGPL-3.0-only, GPL-2.0 AND GPL-2.0-only AND GPL-2.0-or-later AND LGPL-2.1-or-later, MIT, ISC, Unlicense, Apache-2.0, BSD-3-Clause, BSD-2-Clause, BSD-2-Clause AND MIT, MPL-2.0, CC-BY-4.0, CC-BY-3.0, CC-BY-SA-4.0, CC0-1.0, BSD-2-Clause AND BSD-3-Clause, BSD-3-Clause AND BSD-3-Clause-Clear, MIT OR Apache-2.0, MIT AND Python-2.0, (Apache-2.0 AND BSD-3-Clause) OR (Apache-2.0 AND MIT), (MIT OR Apache-2.0) AND Unicode-DFS-2016, OFL-1.1, Apache-2.0 AND BSD-3-Clause AND MIT AND OFL-1.1, BlueOak-1.0.0, BSL-1.0, Python-2.0.1, MIT AND PSF-2.0, LGPL-2.0-only AND LGPL-2.1-or-later, CAL-1.0

OpenSSF Scorecard

PackageVersionScoreDetails
actions/greenbone/actions/mypy-python 3.*.* UnknownUnknown
pip/autohooks-plugin-mypy 23.10.0 UnknownUnknown
pip/lxml-stubs 0.5.1 UnknownUnknown
pip/mypy 1.10.0 🟢 5.7
Details
CheckScoreReason
Code-Review🟢 9Found 27/30 approved changesets -- score normalized to 9
Maintained🟢 1030 commit(s) and 5 issue activity found in the last 90 days -- score normalized to 10
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
Packaging⚠️ -1packaging workflow not detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Binary-Artifacts🟢 10no binaries found in the repo
Fuzzing⚠️ 0project is not fuzzed
Security-Policy⚠️ 0security policy file not detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
Vulnerabilities🟢 100 existing vulnerabilities detected
pip/types-paramiko 3.4.0.20240423 🟢 5.8
Details
CheckScoreReason
Code-Review🟢 10all changesets reviewed
Maintained🟢 1030 commit(s) and 21 issue activity found in the last 90 days -- score normalized to 10
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Packaging⚠️ -1packaging workflow not detected
Fuzzing⚠️ 0project is not fuzzed
Vulnerabilities🟢 100 existing vulnerabilities detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
Security-Policy⚠️ 0security policy file not detected
Binary-Artifacts🟢 10no binaries found in the repo
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
pip/defusedxml 0.7.1 🟢 5.9
Details
CheckScoreReason
Code-Review⚠️ 0Found 2/24 approved changesets -- score normalized to 0
Maintained⚠️ 00 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 0
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Binary-Artifacts🟢 10no binaries found in the repo
Security-Policy🟢 10security policy file detected
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
Fuzzing🟢 10project is fuzzed
Vulnerabilities🟢 100 existing vulnerabilities detected
Packaging🟢 10packaging workflow detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
pip/defusedxml >= 0.6 🟢 5.9
Details
CheckScoreReason
Code-Review⚠️ 0Found 2/24 approved changesets -- score normalized to 0
Maintained⚠️ 00 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 0
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Binary-Artifacts🟢 10no binaries found in the repo
Security-Policy🟢 10security policy file detected
Token-Permissions🟢 10GitHub workflow tokens follow principle of least privilege
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0
Branch-Protection⚠️ 0branch protection not enabled on development/release branches
Fuzzing🟢 10project is fuzzed
Vulnerabilities🟢 100 existing vulnerabilities detected
Packaging🟢 10packaging workflow detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0

Scanned Manifest Files

.github/workflows/ci.yml
  • greenbone/actions/mypy-python@3.*.*
poetry.lock
  • autohooks-plugin-mypy@23.10.0
  • lxml-stubs@0.5.1
  • mypy@1.10.0
  • types-paramiko@3.4.0.20240423
  • defusedxml@0.7.1
pyproject.toml
  • defusedxml@>= 0.6

gvm/protocols/gmp/_gmp224.py Fixed Show fixed Hide fixed
gvm/utils.py Dismissed Show dismissed Hide dismissed
@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch from 4c33cef to 8237b8d Compare May 21, 2024 14:08
@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch 7 times, most recently from 5e67903 to 4f55c34 Compare May 31, 2024 09:38
Copy link

codecov bot commented May 31, 2024

Codecov Report

Attention: Patch coverage is 95.88496% with 93 lines in your changes missing coverage. Please review.

Project coverage is 97.57%. Comparing base (0f946e6) to head (fa88c2a).

Current head fa88c2a differs from pull request most recent head a771cbd

Please upload reports for the commit a771cbd to get more accurate results.

Files Patch % Lines
gvm/connections/_ssh.py 78.48% 28 Missing and 6 partials ⚠️
gvm/connections/_debug.py 45.83% 13 Missing ⚠️
gvm/utils.py 75.60% 4 Missing and 6 partials ⚠️
gvm/connections/_connection.py 83.33% 6 Missing and 2 partials ⚠️
gvm/connections/_tls.py 85.71% 6 Missing ⚠️
gvm/protocols/_protocol.py 90.74% 4 Missing and 1 partial ⚠️
gvm/protocols/core/_connection.py 94.84% 0 Missing and 5 partials ⚠️
gvm/xml.py 93.10% 1 Missing and 3 partials ⚠️
gvm/errors.py 62.50% 1 Missing and 2 partials ⚠️
gvm/protocols/gmp/requests/v224/_scanners.py 96.77% 1 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1127      +/-   ##
==========================================
- Coverage   97.85%   97.57%   -0.28%     
==========================================
  Files          61       65       +4     
  Lines        4292     4541     +249     
  Branches     1047      831     -216     
==========================================
+ Hits         4200     4431     +231     
- Misses         72       74       +2     
- Partials       20       36      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch 4 times, most recently from 2ba6b61 to 431ca35 Compare June 5, 2024 14:32
@bjoernricks bjoernricks marked this pull request as ready for review June 5, 2024 14:35
@bjoernricks bjoernricks requested review from a team as code owners June 5, 2024 14:35
@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch from 431ca35 to fa88c2a Compare June 11, 2024 06:59
@greenbonebot greenbonebot enabled auto-merge (rebase) June 11, 2024 06:59
It seems lxml doesn't support type hints. Thus an additional package
needs to be installed.
The file can be str or bytes IO. For example sys.stdout is str IO
(TextIO).
Passing an empty string or None has to fail.
If None is allowed the argument must be typed as optional.
The module `gvm.protocols.gmp` should be the main entry point when using
python-gvm for GMP.
Allow code like `SomeEnum(OtherEnum.Foo)`. With this change it is
possible to reuse enum classes from older gmp versions in new gmp
classes.
Some methods and arguments got dropped and all methods create bytes data
that is send via a connection class.
We should not rename the methods which would result in requiring
adjustments in user code.
This also removes all code providing GMP 20.8 and 21.4.
Drop obsolete tests and adjust tests for enum imports.
While on it also add type hints and rename variables for consistent
naming style.
It seems Iterable.__name__ is not defined on Python 3.9.
Allow to version the request classes for specific GMP versions. With
this change we can add new requests that are only available for specific
GMP versions.
Update tests to just have 22.4 and 22.5.
gvm-cli passes a string to the osp or gmp class via send_command method.
Therefore still support this interface.
The types should be used from the protocol module or from the gmp class.
@bjoernricks bjoernricks force-pushed the refactor-for-independent-protocol-implementation branch from fa88c2a to a771cbd Compare June 14, 2024 08:24
@greenbonebot greenbonebot merged commit a81f64c into main Jun 14, 2024
18 of 19 checks passed
@greenbonebot greenbonebot deleted the refactor-for-independent-protocol-implementation branch June 14, 2024 08:24
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.

3 participants