Subject: Re: [Netconf] how to handle XML parse errors

From: "Andy Bierman" <[email-masked]>
Date: Tue, 17 Aug 2010 14:02:08 -0700

So why do we even care?
If the programmer is so bad that XML is accepted without even changing angle brackets to character entities, then you get what you deserve.  Dropping the session makes it harder to keep trying the attack.
The admin might notice all the session notifications (if any).

Andy

----- Reply message -----
From: "Juergen Schoenwaelder" <[email-masked]>
Date: Tue, Aug 17, 2010 13:43
Subject: [Netconf] how to handle XML parse errors
To: "Andy Bierman" <[email-masked]>
Cc: "Ladislav Lhotka" <[email-masked]>, "Wes Hardaker" <[email-masked]>, "Kent Watsen" <[email-masked]>, "[email-masked]" <[email-masked]>

On Tue, Aug 17, 2010 at 10:12:19PM +0200, Andy Bierman wrote:

> If the hacker can see the XML then closing off the end tags
> correctly will be trivial.  If not, because the channel is
> encrypted, then it will not be so easy.

Assuming that the data model is not a hidden secret (and NETCONF
itself surely isn't), it boils down to guessing the correct namespace
prefixes, no? Even brute force testing might be doable since most
people choose short prefixes. So yes, closing the session raises the
bar a bit but not for someone seriously interested.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 17 Aug 2010 23:26:20 +0200

> So why do we even care?

Closing the session is a drastic measure but given the shortcomings of
the framing we have invented, this might be the most robust solution
overall, although likely a bit annoying on the cut'n'paste interface.

/js

---

From: Ladislav Lhotka <[email-masked]>
Date: Wed, 18 Aug 2010 06:56:35 +0200

> overall, although likely a bit annoying on the cut'n'paste interface.

I agree. Perhaps the server could just send an error message ("malformed
input") before closing the session.

Lada

-- 
Ladislav Lhotka, CESNET
PGP Key ID: E74E8C0C

---

From: Phil Shafer <[email-masked]>
Date: Wed, 18 Aug 2010 10:53:44 -0400

>overall, although likely a bit annoying on the cut'n'paste interface.

Any chance we can address the framing issue?  It would be nice to
have something real, workable, and robust here.

Thanks,
 Phil

---

From: Kent Watsen <[email-masked]>
Date: Wed, 18 Aug 2010 09:54:22 -0700

> have something real, workable, and robust here.

Isn't framing fundamentally supported by matching XML tags?  ]]>]]> is only there to support recovery, it's not intended to support framing - right?   

4742 says the ]]>]]> is to only to allow "resynchronization of the NETCONF exchange in the event of an XML syntax or parsing error."  My reading of that is that the parsing error has to happen first, then the agent resynchronizes on the next ]]>]]>.  While the parsing error may be caused due to an unexpected ]]>]]>, that seems like a very special case.  

Given this shift in emphasis, I'm comfortable in closing the session if and only if the parsing error is due to an unexpected ]]>]]>.

This text should go into 4742bis, not 4741bis

Thanks,
Kent

---

From: Phil Shafer <[email-masked]>
Date: Wed, 18 Aug 2010 13:01:00 -0400

Kent Watsen writes:
>Isn't framing fundamentally supported by matching XML tags?  ]]>]]> is only
> there to support recovery, it's not intended to support framing - right?

Matching tags means garbage-in-garbage-out.  Without a real framing
protocol you are trusting the content to tell you when the content
ends.  And one can't look for blindly for the string "</rpc>", since 
this could reasonably appear inside the message.  No one is excited
about the idea, but having real content framing would be goodness.

>Given this shift in emphasis, I'm comfortable in closing the session if and=
> only if the parsing error is due to an unexpected ]]>]]>.

FWIW, here's the original text from the junoscript draft (in 2002):

http://tools.ietf.org/id/draft-shafer-js-xml-api-00.txt

2.5 Framing Protocol Errors

   When errors are detected in the framing protocol, the JUNOScript
   server closes the connection.  Although this behavior may seem
   severe, the server cannot trust that it will not discard something
   important during error recovery that may cause the script to fail its
   intended purpose.  Errors in RPC content are acceptable and are
   reported as errors, but failures of the framing protocol are fatal.

   This scenario is a result of having both framing and content in XML.
   A distinct framing protocol would not have this problem.  The choice
   was between a non-XML framing protocol and having the entire session
   form a component XML document.  Given our expectation of JUNOScript
   as a component of UNIX pipelines in shell scripts, we chose the
   latter.

Thanks,
 Phil

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 19 Aug 2010 09:07:07 +0200 (CEST)

> there to support recovery, it's not intended to support framing - right?

Logically, the transport layer is responsible for sending the contents
of each "transport frame" to the message layer.  With SSH, this
content is whatever data is found between the ]]>]]> markers.

The message layer takes this content and parses it as one XML
document.  If this parsing fails, the content is invalid.

It is not clear from 4741 what the server should do if the message is
invalid.  If you just read 4741, you might think that the only option
is to close the session, since there is no way to send an rpc-error
back unless an <rpc> was received.  On the other hand, if you also
read 4742, it talks about resynchronization in the event of an XML
syntax or parsing error.

If we want to change this now, we must also change 4742bis.  But the
]]>]]> is used for this specific purpose, so if we change this, should
we also remove ]]>]]>?

After thinking some more about this, I would actually prefer to not
change 4742, and update 4741 to say:

  If a server receives a message that is not well-formed XML, it MUST
  reply with an 'operation-failed' error.

FWIW, I have checked three existing implementations, and they all
generate an rpc-error and continue with the session.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 19 Aug 2010 10:04:56 +0200

> generate an rpc-error and continue with the session.

Not all messages received by a server are <rpc> messages. So what
about this:

>   If a server receives an <rpc> message that is not well-formed XML,
>   it MUST reply with an 'operation-failed' error.

It surely would be nice to have a more precise error for this since
operation-failed is a catch-all error and does not really indicate
that the XML was malformed. I have not tested it, but I would not be
surprised if different implementations return different errors.

I agree with I think Randy that the framing and resynchronization is a
transport issue and not a 4741 issue. If the framing does not allow
reliable resynchronization to the next frame, then the transport I
think should mandate to close the session. The question then still
remains whether 4742 provides reliable framing (or should it be
reliable enough framing).

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 19 Aug 2010 11:20:41 +0200 (CEST)

> >   it MUST reply with an 'operation-failed' error.

What do you want the server to do with other (illegal) messages then?

> reliable enough framing).

IMO it is reliable enough.  It is of course very primitive - scan for
]]>]]>, and when found, send the data to the higher layer.  The only
problem is well-known - the EOM marker can be part of a legal XML
document.  But we have discussed this before, and I do not want to
change this now.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 19 Aug 2010 12:51:22 +0200

> What do you want the server to do with other (illegal) messages then?

What is illegal? I can send a mal-formed <hello>. Other specifications
might add additional messages, like the <notification> message was
added to the stream of server -> client messages.

> document.  But we have discussed this before, and I do not want to

So a well-known problem is not a problem anymore... ;-)

/js

---

From: Ladislav Lhotka <[email-masked]>
Date: Thu, 19 Aug 2010 12:58:34 +0200

> What do you want the server to do with other (illegal) messages then?

There may not be any message to refer to - it could be complete
rubbish, so IMO it would be better to return a special error ("parse
error", "malformed frame" or similar), without any message-id.

Lada
  
> _______________________________________________
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

-- 
Ladislav Lhotka, CESNET
PGP Key ID: E74E8C0C

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 19 Aug 2010 13:08:41 +0200 (CEST)

Ladislav Lhotka <[email-masked]> wrote:
> rubbish

Right.  Except for the error tag, do you approve the proposed text:

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Thu, 19 Aug 2010 13:34:49 +0200

>     reply with an 'operation-failed' error.

My understanding is that "message" is a well-formed <rpc> document in
the Message layer. On the other hand, the term "frame" is not used in
4741bis, so I don't know.

BTW, what happens if the client continues feeding data and never sends
"]]>]]>"? Shouldn't there be some maximum frame length?

Lada

-- 
Ladislav Lhotka, CESNET
PGP Key ID: E74E8C0C

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 19 Aug 2010 13:53:18 +0200

> 4741bis, so I don't know.

Quoting 4741bis:

            Layer                 Example
       +-------------+      +-----------------+      +----------------+
   (4) |   Content   |      |  Configuration  |      |  Notification  |
       |             |      |      data       |      |      data      |
       +-------------+      +-----------------+      +----------------+
              |                       |                      |
       +-------------+      +-----------------+              |
   (3) | Operations  |      |  <edit-config>  |              |
       |             |      |                 |              |
       +-------------+      +-----------------+              |
              |                       |                      |
       +-------------+      +-----------------+      +----------------+
   (2) |  Messages   |      |     <rpc>,      |      | <notification> |
       |             |      |   <rpc-reply>   |      |                |
       +-------------+      +-----------------+      +----------------+
              |                       |                      |
       +-------------+      +-----------------------------------------+
   (1) |   Secure    |      |  SSH, TLS, BEEP/TLS, SOAP/HTTP/TLS, ... |
       | Transports  |      |                                         |
       +-------------+      +-----------------------------------------+
 
> "]]>]]>"? Shouldn't there be some maximum frame length?

There is a too-big error:

 error-tag:      too-big
 error-type:     transport, rpc, protocol, application
 error-severity: error
 error-info:     none
 Description:    The request or response (that would be generated) is
                 too large for the implementation to handle.

I assume request means <rpc> message and response means <rpc-reply>
message. So if you send a too big <hello> or <notification>, the
behaviour will be implementation dependent (the notification will
likely be dropped and a server likely reacts with closing the session
on an insane hello).

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 19 Aug 2010 13:55:12 +0200 (CEST)

Ladislav Lhotka <[email-masked]> wrote:
> the Message layer.

Sorry, here's the complete change:

OLD:

   All NETCONF messages MUST be well-formed XML, encoded in UTF-8.

NEW:

   All NETCONF messages MUST be well-formed XML, encoded in UTF-8.  If
   a peer receives a message that is not well-formed XML, it MUST
   reply with an 'operation-failed' error.

> "]]>]]>"? Shouldn't there be some maximum frame length?

The max frame length has to be implemenation specific.  There is
already a 'too-big' error tag to handle this case.

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 19 Aug 2010 14:40:05 +0200

>    reply with an 'operation-failed' error.

Sorry, how do I reply to a <notification> or <hello> message with an
'operation-failed' error?

/js

---

From: Andy Bierman <[email-masked]>
Date: Thu, 19 Aug 2010 06:21:08 -0700

> 'operation-failed' error?

The session should be dropped.

<hello> received in the wrong state:

    /* only process this message in hello wait state */
    if (scb->state != SES_ST_HELLO_WAIT) {
        if (LOGINFO) {
            log_info("\nagt_hello dropped, wrong state "
                     "(%d) for session %d",
                     scb->state,
                     scb->sid);
        }
        mytotals->inBadHellos++;
        agt_ses_request_close(scb->sid,
                              scb->sid,
                              SES_TR_BAD_HELLO);
        return;
    }

Yuma logs a message, bumps a counter, and drops the session.
A sysSessionEnd notification will also go out with an error code
such as 'bad-hello'.

The same sort of thing is done if the 'top' element is not registered
so it is unknown.

Unless there is an <rpc> received,there won't be an <rpc-reply>.
Mandating exact errors is all very nice, but they are only for <rpc>.

Andy