Opened 2 weeks ago

Last modified 3 days ago

#33675 needs_review enhancement

Search microdescriptor files for relay ed25519 keys

Reported by: teor Owned by: anuradha1904
Priority: Medium Milestone:
Component: Core Tor/Chutney Version:
Severity: Normal Keywords: ipv6, prop311, outreachy-ipv6
Cc: teor, anuradha1904 Actual Points: 0.1
Parent ID: #33428 Points: 0.5
Reviewer: ahf Sponsor: Sponsor55-can

Description (last modified by teor)

Your code in #33428 needs to pass your local "make test-network-all", before you start this ticket.

We need to enable searching for ed25519 keys in relay microdescriptor files.

There are instructions and a draft search pattern here:
https://github.com/torproject/chutney/blob/master/lib/chutney/TorNet.py#L1325

Please open a new pull request for this ticket. Your branch should be based on the final version of #33428.

Before you push new changes to your pull request, your chutney code should pass:

  • "make test-network-all" on tor master
  • "make test-network-all" on tor maint-0.3.5

You can build a tor branch using these commands:

cd tor
git checkout -b <branch>
make

Where <branch> is master or maint-0.3.5

Child Tickets

Change History (21)

comment:1 Changed 2 weeks ago by teor

Description: modified (diff)

comment:2 Changed 2 weeks ago by anuradha1904

hey teor, should I do all the changes on the same branch by making commits for each issue solved or rather make different PRs, since all the issue assigned to me are in the same file.

comment:3 Changed 2 weeks ago by anuradha1904

Last edited 2 weeks ago by anuradha1904 (previous) (diff)

comment:4 Changed 2 weeks ago by nickm

Generally it's best to have one issue per PR; that way they can get reviewed and merged independently. (Reviewing two little PRs is usally easier than reviewing one big one.)

That said, if branches have to edit the same part of the file, then there might be merge conflicts. If that's the case, it's probably better to have one branch.

comment:5 Changed 2 weeks ago by anuradha1904

Thank you nickm. Also, Can you explain me what does "nickname" mean in function getNodeDirInfoStatusPattern? Thank you.

comment:6 Changed 2 weeks ago by nickm

It's a short string that identifies the node, typically something like "000a" or "006c"

comment:7 Changed 2 weeks ago by nickm

Err, sorry. That should have said 'something like "test000a" or "test006c"'.

comment:8 Changed 2 weeks ago by anuradha1904

Great, so for "md", all the code that I have to add is:-

elif md:
         return r'^id ed25519 ' + " "


Am I following the right way, because I checked in the cached-microdescs.new file, and the ed25519 key was just after the string "id ed25519" and there was no nickname like for desc and cons in the file cached-microdesc-consensus and cached-microdescs.new. teor, nickm , can you confirm ? If not, where am I wrong? Kindly guide me and Thank you.

comment:9 in reply to:  8 Changed 2 weeks ago by teor

Replying to anuradha1904:

Great, so for "md", all the code that I have to add is:-

elif md:
         return r'^id ed25519 ' + " "

Can you explain why you wrote this code?
Why do you think you need to use the nickname?

The code in the ticket description is:

        elif md:
            # Not yet implemented, see #33428
            # r'^id ed25519 " + ed25519_identity (end of line)
            # needs ed25519-identity from #30642
            # or the existing keys/ed25519_master_id_public_key

At https://github.com/torproject/chutney/blob/master/lib/chutney/TorNet.py#L1325

Am I following the right way, because I checked in the cached-microdescs.new file, and the ed25519 key was just after the string "id ed25519" and there was no nickname like for desc and cons in the file cached-microdesc-consensus and cached-microdescs.new. teor, nickm , can you confirm ? If not, where am I wrong? Kindly guide me and Thank you.

Regular expressions are patterns that are matched against each line. You can read a short introduction here:
https://docs.python.org/3/howto/regex.html

Remember how we just loaded ed25519-id in #33428 ?
https://github.com/torproject/chutney/pull/61/files#diff-5fe4ecd0da6560d24d20e8ce90281abfR792

Maybe it could be useful here.

comment:10 Changed 13 days ago by anuradha1904

Hey teor, I mentioned I don't think I need to use nickname because nickname returns value "test000a" or "test006c" which does not follow 'id ed25519' , However I did read the articles you mentioned, and found this code :-

elif md:
        return r'^id ed25519 ' + ed25519-id 

where ed25519-id is the environment variable I set up in my function? Am I following the correct way. Thank you.

comment:11 Changed 12 days ago by teor

That looks great!

But base64 contains "+" characters:
https://docs.python.org/3.5/library/base64.html#base64.b64encode

And "+" is a special character in regular expressions:
https://docs.python.org/3/library/re.html#regular-expression-syntax

So you'll need to use re.escape() on ed25519-id:
https://docs.python.org/3/library/re.html#re.escape

Please write some code, and test it using "make test-network-all".
Send us a pull request when you're done.

If you get stuck, send us:

  • a pull request with your draft code
  • the detailed logs from the ".log" files

And tell us what you've tried already.

comment:12 Changed 10 days ago by anuradha1904

Hey teor, I have added the PR , can you kindly check? Thank you very much. https://github.com/torproject/chutney/pull/62

comment:13 Changed 8 days ago by teor

Status: assignedneeds_revision

I have asked for some changes on the PR. And some more information about how you are running the tests.

comment:14 Changed 8 days ago by anuradha1904

teor: I have followed the steps provided by you. Kindly check if I following the wrong steps and suggest me steps that I should take further for improvement. Thank you very much for your help.

comment:15 Changed 6 days ago by teor

Hi, just letting you know that I've seen these changes, and I'll try to review them over the next few days.

comment:16 Changed 5 days ago by anuradha1904

Hey teor, Thank you very much for reviewing the changes.

Last edited 5 days ago by anuradha1904 (previous) (diff)

comment:17 Changed 5 days ago by teor

Status: needs_revisionneeds_review

comment:18 Changed 5 days ago by teor

Reviewer: teorahf

I have spoken with Alex, and he will review this ticket.

comment:19 Changed 4 days ago by anuradha1904

Hey teor, can I ask alex the question regarding this ticket then? Thank you very much.

comment:20 in reply to:  19 Changed 4 days ago by teor

Replying to anuradha1904:

Hey teor, can I ask alex the question regarding this ticket then? Thank you very much.

Please ask your questions on the ticket, and someone will answer them :-)

comment:21 Changed 3 days ago by anuradha1904

hey ahf, can you please review this Pull Request https://github.com/torproject/chutney/pull/62 . Thank you very much.

Note: See TracTickets for help on using tickets.