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

set line length to 120 #1735

Merged
merged 2 commits into from
Oct 27, 2024
Merged

set line length to 120 #1735

merged 2 commits into from
Oct 27, 2024

Conversation

oroulet
Copy link
Member

@oroulet oroulet commented Oct 26, 2024

No description provided.

@@ -173,7 +183,9 @@
if security_policy.host_certificate:
chunk.SecurityHeader.SenderCertificate = security_policy.host_certificate
if security_policy.peer_certificate:
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(security_policy.peer_certificate).digest()
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(
security_policy.peer_certificate

Check failure

Code scanning / CodeQL

Use of a broken or weak cryptographic hashing algorithm on sensitive data High

Sensitive data (certificate)
is used in a hashing algorithm (SHA1) that is insecure.

Copilot Autofix AI 29 days ago

To fix the problem, we need to replace the use of the SHA-1 hashing algorithm with a stronger algorithm, such as SHA-256. This change will ensure that the certificate's hash is more secure against collision attacks.

  • General Fix: Replace the hashlib.sha1 function with hashlib.sha256.
  • Detailed Fix: Specifically, change the line where hashlib.sha1 is used to hashlib.sha256 in the message_to_chunks method.
  • Files/Regions/Lines to Change: The change will be made in the file asyncua/common/connection.py on line 187.
  • Requirements: Ensure that the hashlib module is imported (already present in the code).
Suggested changeset 1
asyncua/common/connection.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/asyncua/common/connection.py b/asyncua/common/connection.py
--- a/asyncua/common/connection.py
+++ b/asyncua/common/connection.py
@@ -185,3 +185,3 @@
             if security_policy.peer_certificate:
-                chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(
+                chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha256(
                     security_policy.peer_certificate
EOF
@@ -185,3 +185,3 @@
if security_policy.peer_certificate:
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha1(
chunk.SecurityHeader.ReceiverCertificateThumbPrint = hashlib.sha256(
security_policy.peer_certificate
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@@ -31,6 +31,7 @@ jobs:
uv tool install pre-commit
- name: Lint with ruff
run: |
uvx ruff format --diff
Copy link
Contributor

@rth rth Oct 27, 2024

Choose a reason for hiding this comment

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

It might be better to include formatting in pre-commit, so it would also run it locally when comitting which is more convenient see https://github.com/astral-sh/ruff-pre-commit?tab=readme-ov-file#using-ruff-with-pre-commit

Copy link
Member Author

Choose a reason for hiding this comment

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

I have 0 ideas how to add that to pre-commit. So you can do it.
But I feel it is a bit strange to use pre-commit to do linting. Isnt't much clearer to have the check we do in plain test here?

@oroulet oroulet merged commit 54895d2 into master Oct 27, 2024
5 of 6 checks passed
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