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

Avoid exception when DNSKEY record references unknown signature algorithm #137

Merged
merged 1 commit into from
Apr 1, 2024

Conversation

cketti
Copy link
Contributor

@cketti cketti commented Mar 28, 2024

Using DnssecResolverApi.INSTANCE to resolve technikum-wien.at throws a NullPointerException. This is caused by the DNSKEY class not being able to handle unknown/unsupported algorithm bytes.

Stacktrace (MiniDNS 1.0.4):

java.lang.NullPointerException: Attempt to read from field 'byte org.minidns.constants.DnssecConstants$SignatureAlgorithm.number' on a null object reference in method 'void org.minidns.record.DNSKEY.<init>(short, byte, org.minidns.constants.DnssecConstants$SignatureAlgorithm, byte[])'
  at org.minidns.record.DNSKEY.<init>(DNSKEY.java:109)
  at org.minidns.record.DNSKEY.<init>(DNSKEY.java:105)
  at org.minidns.record.DNSKEY.parse(DNSKEY.java:90)
  at org.minidns.record.Record.parse(Record.java:376)
  at org.minidns.dnsmessage.DnsMessage.<init>(DnsMessage.java:414)
  at org.minidns.source.NetworkDataSource.queryUdp(NetworkDataSource.java:97)
  at org.minidns.source.NetworkDataSource.query(NetworkDataSource.java:60)
  at org.minidns.source.NetworkDataSource.query(NetworkDataSource.java:34)
  at org.minidns.AbstractDnsClient.query(AbstractDnsClient.java:250)
  at org.minidns.AbstractDnsClient.query(AbstractDnsClient.java:360)
  at org.minidns.DnsClient.query(DnsClient.java:157)
  at org.minidns.iterative.ReliableDnsClient.query(ReliableDnsClient.java:99)
  at org.minidns.AbstractDnsClient.query(AbstractDnsClient.java:188)
  at org.minidns.dnssec.DnssecClient.queryDnssec(DnssecClient.java:104)
  at org.minidns.dnssec.DnssecClient.queryDnssec(DnssecClient.java:100)
  at org.minidns.dnssec.DnssecClient.verifySignedRecords(DnssecClient.java:381)
  at org.minidns.dnssec.DnssecClient.verifySignatures(DnssecClient.java:321)
  at org.minidns.dnssec.DnssecClient.verifyAnswer(DnssecClient.java:159)
  at org.minidns.dnssec.DnssecClient.verify(DnssecClient.java:149)
  at org.minidns.dnssec.DnssecClient.performVerification(DnssecClient.java:115)
  at org.minidns.dnssec.DnssecClient.queryDnssec(DnssecClient.java:105)
  at org.minidns.hla.DnssecResolverApi.resolve(DnssecResolverApi.java:65)
  at org.minidns.hla.ResolverApi.resolve(ResolverApi.java:114)
  at org.minidns.hla.ResolverApi.resolve(ResolverApi.java:108)

See thunderbird/thunderbird-android#7739

The rest of the code seems to cope fine with DNSKEY.algorithm being null. With this change applied the name can be resolved without an exception.

@Flowdalic
Copy link
Collaborator

Flowdalic commented Mar 28, 2024

Thanks, much appreciated. Looks good to me.

And this looks like a candidate for the stable 1.0 branch. Could you change this PR so that it targets the 1.0 branch? This probably requires that you cherry-pick your commit on top of the 1.0 branch.

@cketti cketti force-pushed the unknown_signature_algorithm branch from 5bce77f to a5e45a2 Compare March 28, 2024 21:24
@cketti cketti changed the base branch from master to 1.0 March 28, 2024 21:25
@cketti cketti force-pushed the unknown_signature_algorithm branch from a5e45a2 to ba07d8d Compare March 28, 2024 21:28
@cketti
Copy link
Contributor Author

cketti commented Mar 28, 2024

Done. I also fixed the checkstyle error.

@Flowdalic Flowdalic merged commit ba07d8d into MiniDNS:1.0 Apr 1, 2024
@cketti cketti deleted the unknown_signature_algorithm branch April 1, 2024 22:50
@Flowdalic
Copy link
Collaborator

Fix included in MiniDNS 1.0.5. Thanks for your contribution.

@cketti
Copy link
Contributor Author

cketti commented Apr 2, 2024

Thank you! ❤️

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

Successfully merging this pull request may close these issues.

2 participants