Repository: rthalley/dnspython

Issue:
Issues with IXFR requests against ee.
**Describe the bug**
I am trying to do an IXFR for `ee.` from `zone.internet.ee.`.
This works fine with dig, for example using `dig ixfr=1720121412 ee @zone.internet.ee.`
However, with dnspython, for some reason the first (and crucial) SOA record is missing and then it falls back to AXFR and then fails on some assertion/sanity check:

```
Traceback (most recent call last):
  File "/home/schanzen/dev/ascension/test.py", line 7, in <module>
    dns.query.inbound_xfr("195.43.86.175", zone, txn)
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/query.py", line 1665, in inbound_xfr
    for _ in _inbound_xfr(txn_manager, s, query, serial, timeout, expiration):
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/query.py", line 1470, in _inbound_xfr
    done = inbound.process_message(r)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/xfr.py", line 112, in process_message
    raise dns.exception.FormError("wrong question rdatatype")
dns.exception.FormError: wrong question rdatatype
```

(it sees that the question has type IXFR and expects AXFR now)

**To Reproduce**

Use this script:

```
import dns.xfr
import dns.query
import dns.zone

zone = dns.zone.Zone("ee")
txn, _ = dns.xfr.make_query (zone, serial=1720121412)
dns.query.inbound_xfr("195.43.86.175", zone, txn)
```

The IP is the result of `host zone.internet.ee`. The serial is picked from an old revision. The behaviour may change at some point if the serial is too old (then the server will probably actually fall back to AXFR and the above code will run fine).

**Context (please complete the following information):**
 - dnspython version 2.7.0
 - Python version 3.12.6
 - OS: Fedora Linux

Discussion:
@bwelling: This is definitely a bug

AXFR-style IXFR messages aren't a problem; the server is allowed to return AXFR-style responses to an IXFR query, and dnspython is correctly detecting this, and parsing the response as an AXFR.  The problem is that this is also causing the expected type in future question sections to be AXFR, which is wrong, as the query type was IXFR.

There aren't any tests which cover AXFR-style IXFR responses that span multiple messages, and probably should be.@schanzen: I am not entirely convinced that the bug is that dnspython incorrectly handles AXFR responses to IXFR queries (but it very well may be doing that incorrectly as well).
After all, the reponse *is* IXFR, which is why it fails: dnspython heuristically expects AXFR but the server claims it is sending an IXFR response.

When executing the dig command in the OP the first RR you get is the SOA.
Tracing what is happening in dnspython it seems as is the first SOA is either lost or the response is different from what dig outputs.
I have not looked at wireshark yet which would probably give a better picture wrt dig vs dnspython. In any case, I would expect my rather simple snippet to just work, and not error.@bwelling: I think your analysis is incorrect.  For one thing, there are no heuristics; dnspython is following the IXFR protocol.  See the first paragraph of https://datatracker.ietf.org/doc/html/rfc1995#section-4.

```
   If incremental zone transfer is not available, the entire zone is
   returned.  The first and the last RR of the response is the SOA
   record of the zone.  I.e. the behavior is the same as an AXFR
   response except the query type is IXFR.
```
This is the format of the response here, and dnspython is detecting this by noticing that the response begins with a single SOA.  As I said, the problem is that dnspython is incorrectly checking subsequent messages have type AXFR; it should be checking that they have type IXFR, to match the query.

I'm not sure why you think SOA records are being lost.  If you use `dns.query.xfr` instead of `dns.query.inbound_xfr` and print the messages, you'd see the first one prints correctly, and an exception will be thrown when the second one is processed.  If dnspython is modified to remove the incorrect type check, the transfer will complete successfully.

> In any case, I would expect my rather simple snippet to just work, and not error.

Yes, that's why I started with "This is definitely a bug."@schanzen: Sorry I mistakenly commented on the pull request instead of here. Anyway, your fix works for me for both issues, thanks!

What I still wonder is how I can as a caller determine if an IXFR or AXFR is performed on my transaction manager/zone.@schanzen: Ok I actually do sometimes(!) get the error I posted on the commit with `serial=None` after all:

```
Traceback (most recent call last):
  File "/home/schanzen/dev/ascension/test_error.py", line 7, in <module>
    dns.query.inbound_xfr("195.43.86.175", zone, txn)
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/query.py", line 1665, in inbound_xfr
    for _ in _inbound_xfr(txn_manager, s, query, serial, timeout, expiration):
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/query.py", line 1470, in _inbound_xfr
    done = inbound.process_message(r)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/schanzen/dev/ascension/.venv/lib64/python3.12/site-packages/dns/xfr.py", line 218, in process_message
    raise dns.exception.FormError("unexpected origin SOA in AXFR")
dns.exception.FormError: unexpected origin SOA in AXFR
```

So something the server returns when AXFR is requested makes dnspython unhappy still. But only sometimes?@rthalley: Sorry, just meant to merge Brian's changes not close the issue altogether.

Regardless of being initiated as an AXFR or an IXFR that falls back to AXFR, you can tell if you are getting a full zone transfer and not an incremental by looking at the transaction's `replacement` attribute, which will be `True`.

I will look into "unexpected origin SOA in AXFR"@schanzen: fwiw I noticed that the serial of the first and last SOA sometimes differ, probably becuase it changed internally. ``.ee`` seems to update the serial very frequently...@schanzen: I an not sure if it explicitly violates the spec

```
An AXFR response that is transferring the zone's contents will
   consist of a series (which could be a series of length 1) of DNS
   messages.  In such a series, the first message MUST begin with the
   SOA resource record of the zone, and the last message MUST conclude
   with the same SOA resource record.
```

of if that can still be considered the same SOA.@rthalley: Yes, I just replicated the issue with serial=None, and this is a problem with the authority.  A zone transfer is supposed to be a single coherent version, not a mix of versions (RFC 1035 section 6.3).  In the test I did, dnspython got a serial of 1729270216 at the start of the AXFR, and 1729270218 at the end, which is what triggers the error, as dnspython expects the AXFR to end with the same SOA with which it started.@rthalley: And in the text you quoted from RFC 5936, a change in the serial number is definitely not "the same SOA", though the standard could be clearer!@rthalley: Now closing as fixed.

Solution patch:
diff --git a/dns/xfr.py b/dns/xfr.py
index f1b8759..d17dd48 100644
--- a/dns/xfr.py
+++ b/dns/xfr.py
@@ -83,8 +83,13 @@ class Inbound:
         if rdtype == dns.rdatatype.IXFR:
             if serial is None:
                 raise ValueError("a starting serial must be supplied for IXFRs")
-        elif is_udp:
-            raise ValueError("is_udp specified for AXFR")
+            self.incremental = True
+        elif rdtype == dns.rdatatype.AXFR:
+            if is_udp:
+                raise ValueError("is_udp specified for AXFR")
+            self.incremental = False
+        else:
+            raise ValueError("rdtype is not IXFR or AXFR")
         self.serial = serial
         self.is_udp = is_udp
         (_, _, self.origin) = txn_manager.origin_information()
@@ -103,8 +108,7 @@ class Inbound:
         Returns `True` if the transfer is complete, and `False` otherwise.
         """
         if self.txn is None:
-            replacement = self.rdtype == dns.rdatatype.AXFR
-            self.txn = self.txn_manager.writer(replacement)
+            self.txn = self.txn_manager.writer(not self.incremental)
         rcode = message.rcode()
         if rcode != dns.rcode.NOERROR:
             raise TransferError(rcode)
@@ -131,7 +135,7 @@ class Inbound:
                 raise dns.exception.FormError("first RRset is not an SOA")
             answer_index = 1
             self.soa_rdataset = rdataset.copy()  # pyright: ignore
-            if self.rdtype == dns.rdatatype.IXFR:
+            if self.incremental:
                 assert self.soa_rdataset is not None
                 soa = cast(dns.rdtypes.ANY.SOA.SOA, self.soa_rdataset[0])
                 if soa.serial == self.serial:
@@ -168,7 +172,7 @@ class Inbound:
                 #
                 # Every time we see an origin SOA delete_mode inverts
                 #
-                if self.rdtype == dns.rdatatype.IXFR:
+                if self.incremental:
                     self.delete_mode = not self.delete_mode
                 #
                 # If this SOA Rdataset is equal to the first we saw
@@ -177,8 +181,7 @@ class Inbound:
                 # part of the response.
                 #
                 if rdataset == self.soa_rdataset and (
-                    self.rdtype == dns.rdatatype.AXFR
-                    or (self.rdtype == dns.rdatatype.IXFR and self.delete_mode)
+                    (not self.incremental) or self.delete_mode
                 ):
                     #
                     # This is the final SOA
@@ -187,7 +190,7 @@ class Inbound:
                     if self.expecting_SOA:
                         # We got an empty IXFR sequence!
                         raise dns.exception.FormError("empty IXFR sequence")
-                    if self.rdtype == dns.rdatatype.IXFR and self.serial != soa.serial:
+                    if self.incremental and self.serial != soa.serial:
                         raise dns.exception.FormError("unexpected end of IXFR sequence")
                     self.txn.replace(name, rdataset)
                     self.txn.commit()
@@ -199,7 +202,7 @@ class Inbound:
                     #
                     self.expecting_SOA = False
                     soa = cast(dns.rdtypes.ANY.SOA.SOA, rdataset[0])
-                    if self.rdtype == dns.rdatatype.IXFR:
+                    if self.incremental:
                         if self.delete_mode:
                             # This is the start of an IXFR deletion set
                             if soa.serial != self.serial:
@@ -220,7 +223,7 @@ class Inbound:
                 # SOA RR, but saw something else, so this must be an
                 # AXFR response.
                 #
-                self.rdtype = dns.rdatatype.AXFR
+                self.incremental = False
                 self.expecting_SOA = False
                 self.delete_mode = False
                 self.txn.rollback()


Test patch:
diff --git a/tests/test_xfr.py b/tests/test_xfr.py
index 458cdf9..257397c 100644
--- a/tests/test_xfr.py
+++ b/tests/test_xfr.py
@@ -263,6 +263,30 @@ ns2 3600 IN A 10.0.0.2
 @ 3600 IN SOA foo bar 1 2 3 4 5
 """
 
+ixfr_axfr1 = """id 1
+opcode QUERY
+rcode NOERROR
+flags AA
+;QUESTION
+example. IN IXFR
+;ANSWER
+@ 3600 IN SOA foo bar 1 2 3 4 5
+@ 3600 IN NS ns1
+@ 3600 IN NS ns2
+"""
+ixfr_axfr2 = """id 1
+opcode QUERY
+rcode NOERROR
+flags AA
+;QUESTION
+example. IN IXFR
+;ANSWER
+bar.foo 300 IN MX 0 blaz.foo
+ns1 3600 IN A 10.0.0.1
+ns2 3600 IN A 10.0.0.2
+@ 3600 IN SOA foo bar 1 2 3 4 5
+"""
+
 
 def test_basic_axfr():
     z = dns.versioned.Zone("example.")
@@ -394,6 +418,19 @@ def test_ixfr_is_axfr():
     assert z == ez
 
 
+def test_ixfr_is_axfr_two_parts():
+    z = dns.versioned.Zone("example.")
+    m1 = dns.message.from_text(ixfr_axfr1, origin=z.origin, one_rr_per_rrset=True)
+    m2 = dns.message.from_text(ixfr_axfr2, origin=z.origin, one_rr_per_rrset=True)
+    with dns.xfr.Inbound(z, dns.rdatatype.IXFR, serial=0xFFFFFFFF) as xfr:
+        done = xfr.process_message(m1)
+        assert not done
+        done = xfr.process_message(m2)
+        assert done
+    ez = dns.zone.from_text(base, "example.")
+    assert z == ez
+
+
 def test_ixfr_requires_serial():
     z = dns.zone.from_text(base, "example.", zone_factory=dns.versioned.Zone)
     with pytest.raises(ValueError):