fix: INTERCOM messages RSA #44
Conversation
these were still using the removed pgpy
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces a significant cryptographic transformation in the hivemind bus client, transitioning from PGP to RSA encryption. The changes primarily affect message serialization, encryption, and decryption processes in both Changes
Sequence DiagramsequenceDiagram
participant Sender
participant MessageBus
participant Receiver
Sender->>MessageBus: emit_intercom(message, pubkey)
MessageBus->>MessageBus: Encrypt with RSA
MessageBus->>MessageBus: Sign message
MessageBus->>Receiver: Send encrypted message
Receiver->>Receiver: Decrypt with RSA key
Receiver->>Receiver: Verify signature
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
hivemind_bus_client/client.py (1)
497-508: Newemit_intercommethod implementing RSA encryption and signing.
- The RSA-based code properly encrypts and signs the message.
- Consider adding error handling for missing or invalid private keys.
- Consider clarifying docstrings about the expected type(s) of
pubkey.hivemind_bus_client/protocol.py (1)
281-284: Consider implementing signature verification.
The commented-out code suggests a plan to verify RSA signatures. Verifying signatures helps ensure message authenticity and integrity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hivemind_bus_client/client.py(3 hunks)hivemind_bus_client/protocol.py(2 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
hivemind_bus_client/protocol.py
279-279: Local variable signature is assigned to but never used
Remove assignment to unused variable signature
(F841)
🔇 Additional comments (8)
hivemind_bus_client/client.py (4)
3-3: Import usage is consistent with RSA-based encryption.
No issues found with the usage of hexlify for hex-encoding the ciphertext and signature.
7-8: New cryptographic imports.
Using pybase64 and Cryptodome.PublicKey.RSA aligns with the revised RSA encryption approach. Please ensure these dependencies are installed and compatible with your runtime environment.
14-15: Additional imports from ovos_utils.
These appear consistent with the rest of the codebase and introduce no apparent conflicts.
23-23: Removal of pgpy references in favor of RSA methods.
Importing encrypt_RSA, load_RSA_key, and sign_RSA completes the transition away from PGP.
hivemind_bus_client/protocol.py (4)
1-1: Hex-decoding for RSA ciphertext.
No issues with introducing unhexlify; it matches the message flow for RSA decryption.
3-3: Addition of Optional import.
Usage is consistent with existing type hints.
14-15: Transition to RSA-based decryption.
Imports for decrypt_RSA and load_RSA_key confirm that references to pgpy have been removed.
285-287: Error handling when decrypting.
Consider catching exceptions if private key loading or decryption fails. This can ensure graceful handling of invalid or corrupted keys.
hivemind_bus_client/protocol.py
Outdated
| ciphertext = unhexlify(pload["ciphertext"]) | ||
| signature = unhexlify(pload["signature"]) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unused signature variable.
As flagged by static analysis, signature is assigned but never utilized. You can remove it if you’re not performing signature verification:
- signature = unhexlify(pload["signature"])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ciphertext = unhexlify(pload["ciphertext"]) | |
| signature = unhexlify(pload["signature"]) | |
| ciphertext = unhexlify(pload["ciphertext"]) | |
🧰 Tools
🪛 Ruff (0.8.2)
279-279: Local variable signature is assigned to but never used
Remove assignment to unused variable signature
(F841)
these were still using the removed pgpy
these were still using the removed pgpy, missed in #42
Summary by CodeRabbit
New Features
Security Improvements
Technical Updates
pgpylibrary with RSA-based cryptographic methods