-
Notifications
You must be signed in to change notification settings - Fork 258
Rough key-verification implementation #787
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a rough key verification feature for Meshtastic, allowing manual key verification from Python. The implementation adds three new methods to handle the key verification process and corresponding command-line interface options.
- Adds three key verification methods: initiate verification, provide security number, and confirm verification
- Implements command-line arguments to access the key verification functionality
- Uses protobuf messages to handle the key verification admin protocol
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
File | Description |
---|---|
meshtastic/node.py | Adds three key verification methods to handle the verification workflow |
meshtastic/main.py | Adds command-line argument parsing and handlers for key verification operations |
else: | ||
onResponse = self.onAckNak | ||
return self._sendAdmin(a, onResponse=onResponse) | ||
def keyVerificationNumber(self, number: int, nonce: int,): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing comma after the last parameter in the function definition.
def keyVerificationNumber(self, number: int, nonce: int,): | |
def keyVerificationNumber(self, number: int, nonce: int): |
Copilot uses AI. Check for mistakes.
print(int(number)) | ||
print(int(nonce)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statements should be removed from production code. Consider using proper logging instead.
print(int(number)) | |
print(int(nonce)) | |
logging.debug(f"Key verification number: {int(number)}") | |
logging.debug(f"Key verification nonce: {int(nonce)}") |
Copilot uses AI. Check for mistakes.
print(int(number)) | ||
print(int(nonce)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statements should be removed from production code. Consider using proper logging instead.
print(int(number)) | |
print(int(nonce)) | |
logging.debug(f"Security number: {int(number)}") | |
logging.debug(f"Nonce: {int(nonce)}") |
Copilot uses AI. Check for mistakes.
else: | ||
onResponse = self.onAckNak | ||
return self._sendAdmin(a, onResponse=onResponse) | ||
def keyVerificationConfirm(self, nonce: int,): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the trailing comma after the last parameter in the function definition.
def keyVerificationConfirm(self, nonce: int,): | |
def keyVerificationConfirm(self, nonce: int): |
Copilot uses AI. Check for mistakes.
onResponse = self.onAckNak | ||
return self._sendAdmin(a, onResponse=onResponse) | ||
def keyVerificationConfirm(self, nonce: int,): | ||
print(int(nonce)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug print statements should be removed from production code. Consider using proper logging instead.
print(int(nonce)) | |
logging.debug(f"Nonce value: {int(nonce)}") |
Copilot uses AI. Check for mistakes.
p = admin_pb2.KeyVerificationAdmin() | ||
p.message_type = p.MessageType.INITIATE_VERIFICATION | ||
p.remote_nodenum = nodeId | ||
p.nonce = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded nonce value of 0 appears to be a magic number. Consider using a named constant or adding a comment explaining why 0 is used for initiation.
p.nonce = 0 | |
p.nonce = DEFAULT_NONCE |
Copilot uses AI. Check for mistakes.
) | ||
group.add_argument( | ||
"--key-verification-number", | ||
help="start key verification. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help text is incorrect for this argument. It should describe providing a security number, not starting key verification.
help="start key verification. " | |
help="Provide a security number for key verification. " |
Copilot uses AI. Check for mistakes.
) | ||
group.add_argument( | ||
"--key-verification-confirm", | ||
help="start key verification. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help text is incorrect for this argument. It should describe confirming key verification, not starting it.
help="start key verification. " | |
help="confirm key verification. " |
Copilot uses AI. Check for mistakes.
@@ -1961,6 +1992,12 @@ def initParser(): | |||
action="store_true", | |||
) | |||
|
|||
group.add_argument( | |||
"--key-verification-nonce", | |||
help="start key verification. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The help text is incorrect for this argument. It should describe providing a nonce value, not starting key verification.
help="start key verification. " | |
help="Provide a nonce value for key verification. " |
Copilot uses AI. Check for mistakes.
Probably not done the right way, but works to do manual key-verification from python. Needs a protobuf update to go with it.