search

theforeman / puppet-dns

Installs and manages an ISC BIND DNS server with zones
Apache License 2.0
18 stars 70 forks source link

create-rndc.key fails with bind9 9.13.0+ #189

Closed karelyatin closed 3 years ago

karelyatin commented 3 years ago

With https://github.com/isc-projects/bind9/commit/3a4f820d625c214cfb21f5e6d18ce9160d2a193b -r option is removed in confgen command, so now if -r /dev/urandom is passed to the command then it fails due to fatal("The -r option has been deprecated.").

I see it's used at two place in puppet-dns code:-

Also looks this parameter is not mandatory in earlier bind versions and confgen works without it too, but may be it was passed for some use case, Anyway it would be good to get it fixed for bind-9.13.0+.

Faced this while testing with bind-9.16.11 on a CentOS9 machine.

ekohl commented 3 years ago

I'm trying to figure out the behavior if the parameter is not specified on older versions but it's not entirely clear to me. It looks like it may default to /dev/random but I can't quite find it out. It also looks like it may not matter on modern kernels but most distros don't include kernel 5.6. Those that do are more likely to also ship a bind that doesn't accept the parameter.

Do you happen to know if it defaults to /dev/random or /dev/urandom if not passed?

For what it's worth, this was already present in the first commit (17794329beedfe1af4b3a7074a935cf0ddeaece4) which copied zleslie's dns module. Back in 2012 using /dev/random could be very slow and /dev/urandom was much better for most use cases.

karelyatin commented 3 years ago

@ekohl Before the commit that removed -r option, i see https://github.com/isc-projects/bind9/blob/74dd289a1c24e14d602cc3502b78247364565b32/lib/dns/openssl_link.c#L371-L401 is used to get randomdata if -r randomfile is not passed, so it looks neither /dev/random nor /dev/urandom binaries used by default. With With isc-projects/bind9@3a4f820 it removed -r option and added some more generator function to be used based on availability.

ekohl commented 3 years ago

That was also roughly my impression from trying to read the C code (which I'm not experienced with). Your reading is a bit better than mine. To me it looks like we can trust openssl to be present and recent enough on supported platforms to trust it. I'd be ok with dropping the parameter unconditionally.

Would you be willing to write a patch?

karelyatin commented 3 years ago

That was also roughly my impression from trying to read the C code (which I'm not experienced with). Your reading is a bit better than mine. To me it looks like we can trust openssl to be present and recent enough on supported platforms to trust it. I'd be ok with dropping the parameter unconditionally.

Would you be willing to write a patch?

Sure will send a PR in some time