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

Use libidn2 #133

Merged
7 commits merged into from Apr 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ before_install:
# quoting preserves newlines in the script and then avoid error if the
# script contains comments
- eval "$(curl https://travis-perl.github.io/init)"
- sudo apt-get install -y libidn11-dev
- sudo apt-get install -y libidn2-dev
- cpan-install --deps Devel::CheckLib Module::Install Module::Install::XSUtil

install:
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ RUN apk add --no-cache \
# Compile-time dependencies
build-base \
ldns-dev \
libidn-dev \
libidn2-dev \
make \
openssl-dev \
perl-app-cpanminus \
Expand Down Expand Up @@ -32,5 +32,5 @@ COPY --from=build /usr/local/lib/perl5/site_perl/Zonemaster /usr/local/lib/perl5
RUN apk add --no-cache \
# Run-time dependencies
ldns \
libidn \
libidn2 \
perl
9 changes: 4 additions & 5 deletions Makefile.PL
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,11 @@ else {
if ( $opt_idn ) {
print "Feature idn enabled\n";
check_lib_or_exit(
lib => 'idn',
header => 'idna.h',
lib => 'idn2',
header => 'idn2.h',
function =>
'if(strcmp(IDNA_ACE_PREFIX,"xn--")==0) return 0; else return 1;'
);
cc_libs 'idn';
'return IDN2_OK;');
cc_libs 'idn2';
cc_define '-DWE_CAN_HAZ_IDN';
}
else {
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Initially this module was named Net::LDNS.

Run-time dependencies:
* `openssl` (openssl >= 1.1.1 unless [Ed25519] is disabled)
* `libidn` (if [IDN] is enabled)
* `libidn2` (if [IDN] is enabled)
* `libldns` (if [Internal ldns] is disabled; libldns >= 1.7.0, or
libldns >= 1.7.1 if [Ed25519] is enabled)

Expand Down Expand Up @@ -139,7 +139,7 @@ Requires support for Ed25519 in both openssl and ldns.
Enabled by default.
Disable with `--no-idn`.

If the IDN feature is enabled, the GNU `libidn` library will be used to
If the IDN feature is enabled, the GNU `libidn2` library will be used to
add a simple function that converts strings from Perl's internal encoding
to IDNA domain name format.
In order to convert strings from whatever encoding you have to Perl's
Expand Down
2 changes: 1 addition & 1 deletion include/LDNS.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include <ldns/ldns.h>

#ifdef WE_CAN_HAZ_IDN
#include <idna.h>
#include <idn2.h>
#endif

/* ldns 1.6.17 does not have this in its header files, but it is in the published documentation and we need it */
Expand Down
4 changes: 2 additions & 2 deletions lib/Zonemaster/LDNS.pm
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ labels converted to A-labels unless they are already in ASCII.
Assumes that the strings have been converted to Perl's internal encoding before
it's called. Can be exported, but is not by default.

This function requires that GNU libidn was present when L<Zonemaster::LDNS> was
This function requires that GNU libidn2 was present when L<Zonemaster::LDNS> was
compiled. If not, calling C<to_idn> will result in an exception getting thrown.

=item has_idn()

Takes no arguments. Returns true if libidn was present at compilation, false if not.
Takes no arguments. Returns true if libidn2 was present at compilation, false if not.

=item has_gost()

Expand Down
8 changes: 4 additions & 4 deletions src/LDNS.xs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ to_idn(...)

if (SvPOK(ST(i)))
{
status = idna_to_ascii_8z(SvPVutf8_nolen(obj), &out, IDNA_ALLOW_UNASSIGNED);
if (status == IDNA_SUCCESS)
status = idn2_to_ascii_8z(SvPVutf8_nolen(obj), &out, IDN2_ALLOW_UNASSIGNED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flags we se are important. Is IDN2_ALLOW_UNASSIGNED really what we want here? The features that we select are important. This has to be investigated first.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IDNA_ALLOW_UNASSIGNED has been used since the beginning of the to_idn() function. I've just tried to keep the same flag thinking it would give the same behavior. Are you saying that there is a change in behavior?

if (status == IDN2_OK)
{
SV *new = newSVpv(out,0);
SvUTF8_on(new); /* We know the string is plain ASCII, so let Perl know too */
Expand All @@ -28,12 +28,12 @@ to_idn(...)
}
else
{
croak("Error: %s\n", idna_strerror(status));
croak("Error: %s\n", idn2_strerror(status));
}
}
}
#else
croak("libidn not installed");
croak("libidn2 not installed");
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion t/idn.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use utf8;
BEGIN { use_ok( "Zonemaster::LDNS" => qw[:all] ) }

no warnings 'uninitialized';
if (exception {to_idn("whatever")} =~ /libidn not installed/) {
if (exception {to_idn("whatever")} =~ /libidn2 not installed/) {
ok(!has_idn(), 'No IDN');
done_testing;
exit;
Expand Down
22 changes: 10 additions & 12 deletions t/rr.t
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ subtest 'DNSKEY' => sub {
isa_ok( $rr, 'Zonemaster::LDNS::RR::DNSKEY' );
ok( $rr->flags == 256 or $rr->flags == 257 );
is( $rr->protocol, 3 );
# Alg 8 will replace 5. Now (December 2017) both are used.
ok( $rr->algorithm == 5 or $rr->algorithm == 8 );
# Alg 8 has replaced 5. Now (February 2022) only alg 8 is used.
ok( $rr->algorithm == 8 );
}
}
};
Expand All @@ -122,9 +122,9 @@ subtest 'RRSIG' => sub {
is( $rr->signer, 'se.' );
is( $rr->labels, 1 );
if ( $rr->typecovered eq 'DNSKEY' ) {
# .SE KSK should not change very often. 59407 will replace 59747.
# Now (December 2017) both are used.
ok( $rr->keytag == 59747 or $rr->keytag == 59407 );
# .SE KSK should not change very often. 59407 has replaced 59747.
# Now (February 2022) only 59407 is used.
ok( $rr->keytag == 59407 );
}
}
}
Expand Down Expand Up @@ -172,19 +172,17 @@ subtest 'DS' => sub {
my $pd = $se->query( 'nic.se', 'DS' );
plan skip_all => 'No response, cannot test' if not $pd;

# As of February 2022, new KSK with keytag 22643 and algo 13 is used
my $nic_key = Zonemaster::LDNS::RR->new(
'nic.se IN DNSKEY 257 3 5 AwEAAdhJAx197qFpGGXuQn8XH0tQpQSfjvLKMcreRvJyO+f3F3weIHR3 6E8DObolHFp+m1YkxsgnHYjUFN4E9sKa38ZXU0oHTSsB3adExJkINA/t INDlKrzUDn4cIbyUCqHNGe0et+lHmjmfZdj62GJlHgVmxizYkoBd7Rg0 wxzEOo7CA3ZadaHuqmVJ2HvqRCoe+5NDsYpnDia7WggvLTe0vorV6kDc u6d5N9AUPwBsR7YUkbetfXMtUebux71kHCGUJdmzp84MeDi9wXYIssjR oTC5wUF2H3I2Mnj5GqdyBwQCdj5otFbRAx3jiMD+ROxXJxOFdFq7fWi1 yPqUf1jpJ+8='
'nic.se IN DNSKEY 257 3 13 lkpZSlU70pd1LHrXqZttOAYKmX046YqYQg1aQJsv1y0xKr+qJS+3Ue1tM5VCYPU3lKuzq93nz0Lm/AV9jeoumQ=='
);
my $made = Zonemaster::LDNS::RR->new_from_string( 'nic.se IN NS a.ns.se' );
foreach my $rr ( $pd->answer ) {
isa_ok( $rr, 'Zonemaster::LDNS::RR::DS' );
is( $rr->keytag, 16696 );
is( $rr->algorithm, 5 );
is( $rr->keytag, 22643 );
is( $rr->algorithm, 13 );
ok( $rr->digtype == 1 or $rr->digtype == 2 );
ok(
$rr->hexdigest eq '40079ddf8d09e7f10bb248a69b6630478a28ef969dde399f95bc3b39f8cbacd7'
or $rr->hexdigest eq 'ef5d421412a5eaf1230071affd4f585e3b2b1a60'
);
ok( $rr->hexdigest eq 'aa0b38f6755c2777992a74935d50a2a3480effef1a60bf8643d12c307465c9da' );
ok( $rr->verify( $nic_key ), 'derived from expected DNSKEY' );
ok( !$rr->verify( $made ), 'does not match a non-DS non-DNSKEY record' );
}
Expand Down