[Samba] Bug in dhcp-dyndns.sh script, A_REC always singleton array

[Rowland Penny]

On Fri, 11 Aug 2023 14:03:01 +0200
Kasper Brandt via samba <samba at lists.samba.org> wrote:

> Hello
> I was directed to discuss this issue here. As I understand the issue
> with using the unquoted variable is that it expand globs unless
> noglob is set. E.g.
> 
> root at dy3:/# test="b*"
> root at dy3:/# a=($test)
> root at dy3:/# echo ${a[0]}
> bin
> 
> It does seem a bit hypothetical that the output of sambatool dns
> query ... for an A record should contain a glob, but for the sake of
> robustness it might still be a good idea to find a better solution.
> 
> For a single match the output looks like:
> root at dy3:/# /bin/samba-tool dns query "dy3" "poizan.dk" "PoiKodi" A
> "--use-kerberos=required" Name=, Records=1, Children=0
>     A: 10.13.37.10 (flags=f0, serial=1392054, ttl=900)
> 
> For multiple A records the output looks like:
> root at dy3:/# /bin/samba-tool dns query "dy3" "poizan.dk" "test" A
> "--use-kerberos=required" Name=, Records=2, Children=0
>     A: 10.13.37.100 (flags=f0, serial=1392062, ttl=3600)
>     A: 10.13.37.101 (flags=f0, serial=1392063, ttl=3600)
> 
> With the "| grep 'A:' | awk '{print $2}' " filtering these becomes
> respectively
> 
> 10.13.37.10
> 
> and
> 
> 10.13.37.100
> 10.13.37.101
> 
> So just splitting by lines should work.
> 
> Unfortunately just using mapfile with A_REC variable does not quite
> work as here strings always gets a newline at the end.
> 
> root at dy3:/# mapfile -t foo <<< ""
> root at dy3:/# echo ${#foo[@]}
> 1
> 
> I would then suggest instead redirecting the query command right away
> with process substitution
> 
> `mapfile -t `A_REC `< <`($SAMBATOOL dns query "${Server}" "${domain}"
> "${name}" A "$KTYPE" 2>/dev/null | grep 'A:' | awk '{print $2}') `
> `This seems to make it work as intended:
> 
> root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3"
> "poizan.dk" "doesnotexists" A "--use-kerberos=required" 2>/dev/null |
> grep 'A:' | awk '{print $2}') root at dy3:/# echo ${#A_REC[@]} 0
> root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3"
> "poizan.dk" "PoiKodi" A "--use-kerberos=required" 2>/dev/null | grep
> 'A:' | awk '{print $2}') root at dy3:/# echo ${#A_REC[@]} 1
> root at dy3:/# echo ${A_REC[0]}
> 10.13.37.10
> root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3"
> "poizan.dk" "test" A "--use-kerberos=required" 2>/dev/null | grep
> 'A:' | awk '{print $2}') root at dy3:/# echo ${#A_REC[@]} 2
> root at dy3:/# echo ${A_REC[0]}
> 10.13.37.100
> root at dy3:/# echo ${A_REC[1]}
> 10.13.37.101
> 
> - Kasper Brandt
> 
> On Fri, Aug 11, 2023, at 1:14 PM, Rowland Penny wrote:
> > On Fri, 11 Aug 2023 02:52:59 +0200
> > Kasper Brandt via samba-team <samba-team at lists.samba.org> wrote:
> > 
> > > Hello
> > > I encountered a bug in the dhcp-dyndns.sh script embedded in the
> > > site at
> > > https://wiki.samba.org/index.php/Configure_DHCP_to_update_DNS_records
> > > 
> > > Version 0.9.5 (introduced in this revision:
> > > https://wiki.samba.org/index.php?title=Configure_DHCP_to_update_DNS_records&oldid=18255)
> > > introduced the following
> > > 
> > > > # turn A_REC into an array
> > > > A_REC=("$A_REC")
> > > 
> > > (a bash array wasn't used in the previous version)
> > > This is however incorrect as this will always create an array of
> > > one element due to the quotes. The branch immediately after that
> > > should only be taken if the A record didn't exist is never taken.
> > > Instead the branch that should handle 1 existing record gets
> > > taken and something like
> > > > /bin/samba-tool dns delete "dy3" "poizan.dk" "PoiKodi" A ""
> > > > "--use-kerberos=required"
> > > gets executed (note the empty data field), which fails with an
> > > invalid parameter error.
> > > 
> > > The line should probably just have been
> > > > A_REC=($A_REC)
> > > which splits the string into array elements by whitespace.
> > > 
> > > Someone more familiar with the script might want to verify my
> > > fix, or I can just make the change myself if I get wiki access if
> > > that is acceptable? At least I am currently using the fix without
> > > issue.
> > > 
> > > - Kasper Brandt
> > 
> > Problem is, whilst it may be a bug, the suggested fix doesn't
> > appear to be the fix. If you do as Kasper suggests and remove the
> > quotation marks and then run shellcheck against the script, you get
> > this:
> > 
> > In dhcp-dyndns.sh.0.9.5 line 233:
> > A_REC=($A_REC)
> >                        ^-- SC2206: Quote to prevent word splitting,
> > or split robustly with mapfile or read -a.
> > 
> > Which is why I probably took the easy option and added the quotation
> > marks, looks like using a mapfile or read needs to be investigated.
> > 
> > Can I also point out that the last lines on that wikipage are these:
> > 
> > Any questions or problems, ask on the Samba mailing list.
> > 
> > Do not log a bug report on Samba bugzilla for any problems you have
> > with this set up, ask on the samba mailing list.
> > 
> > Rowland
> > 
> > 
> > 

Hi Kaspar, thanks for looking into this, when I said 'looks like using
a mapfile or read needs to be investigated.', I really meant that I
would look into it, so thanks for saving me the trouble :-)

It looks like the mapfile is the way to go, but have you run shellcheck
on the revised script ? 

If you have, please update the script on the wikipage again.

Rowand