search

alexdalitz / dnsruby

Dnsruby is a feature-complete DNS(SEC) client for Ruby, as used by many of the world's largest DNS registries and the OpenDNSSEC project
Other
194 stars 77 forks source link

Implement ECDSAP256SHA256 (13) / ECDSAP384SHA384 (14) algorithms #163

Closed karlerikounapuu closed 4 years ago

karlerikounapuu commented 4 years ago

Makes it possible to validate DNSSEC data for algorithms 13 and 14. Has been tested with ECDSAP256SHA256, but alg 14 should follow the same logic.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.02%) to 77.566% when pulling dcedd01ed0f87f7f999846b60cdaefc803b2e38d on karlerikounapuu:master into 98f6230ec85ccf8d94d45ae775af84a6e1c3e7d4 on alexdalitz:master.

alexdalitz commented 4 years ago

Hi @karlerikounapuu -

Thank you very much for this patch!

I think it looks good - but would it please be possible to add a test case to show that it works OK?

Thanks!

Alex.

keithrbennett commented 4 years ago

Hello, @karlerikounapuu. I am not knowledgeable about DNS like you and @alexdalitz but I used dnsruby in my work for a few years and became active in helping with the code base in other ways. Thank you for your contribution to this. I'd like to raise a few questions as comments to the code changes, you and Alex can decide which if any should be addressed, ok?

karlerikounapuu commented 4 years ago

Hi @karlerikounapuu -

Thank you very much for this patch!

I think it looks good - but would it please be possible to add a test case to show that it works OK?

Thanks!

Alex.

Sure thing, sounds good to me. I aim to take care of it next day or so, had a pretty heavy day 🙂

alexdalitz commented 4 years ago

Yes, I think what you have done conforms to the existing pattern. I don’t think you need to address enhancing the pattern in this PR - we should probably get round to it at some point…

Thanks!

Alex.

On 30 Jun 2020, at 20:45, Karl Erik Õunapuu notifications@github.com wrote:

@karlerikounapuu commented on this pull request.

In lib/dnsruby/resource/DNSKEY.rb https://github.com/alexdalitz/dnsruby/pull/163#discussion_r447936591:

@@ -313,6 +313,8 @@ def public_key elsif [Algorithms.DSA, Algorithms.DSA_NSEC3_SHA1].include?(@algorithm) @public_key = dsa_key

  • elsif [Algorithms.ECDSAP256SHA256, Algorithms.ECDSAP384SHA384].include?(@algorithm) To me it seemed pretty straightforward choice at first sight as other algorithms were checked using the same pattern. But if you guys think it benefits code readability, I'm happy to do it 🙂

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/alexdalitz/dnsruby/pull/163#discussion_r447936591, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2WFWXTE35ALA5VDJ5KRO3RZI6EXANCNFSM4OL6OSNQ.

alexdalitz commented 4 years ago

Thank you!