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

rec: A SHA-384 DS should not trump a SHA-256 one, so only potentially zap SHA-1 #10908

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

omoerbeek
Copy link
Member

Short description

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

@omoerbeek omoerbeek added this to the rec-4.6.0 milestone Oct 27, 2021
@vcunat
Copy link

vcunat commented Jan 20, 2022

So GOST still zaps SHA-1? 🤔

(On a quick glance.) Not that I'd expect it to matter/happen in practice. Still, if that's so, it would be inconsistent with the comment you added.

@Habbie
Copy link
Member

Habbie commented Jan 20, 2022

We don't support GOST any more (even if some comments near this code still mention it) - but the comments could indeed be better.

@vcunat
Copy link

vcunat commented Jan 20, 2022

OK, but I was reading (possibly dead) code, not just comments: https://github.com/PowerDNS/pdns/blob/b995eb5/pdns/syncres.cc#L2616

@Habbie
Copy link
Member

Habbie commented Jan 20, 2022

Indeed, dead code, even if the compiler can't figure that out.

@vcunat
Copy link

vcunat commented Jan 20, 2022

Could call it "g(h)ost" code.

@vcunat
Copy link

vcunat commented Jan 20, 2022

Speaking of GOST DS, I'd expect a new one in the IANA registry soon (like 2022?). But that does not imply you implementing it. I expect that like us, any algorithm that you add in future will be considered significantly more secure than SHA1, so it makes sense to write the code like the diff in this PR.

For reference, my reason for looking into this: https://gitlab.nic.cz/knot/knot-resolver/-/merge_requests/1251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants