Loading ...
Sorry, an error occurred while loading the content.

Re: Make smtpd/Postscreen compatible with load balancers

Expand Messages
  • Wietse Venema
    ... Sorry, that might work for haproxy, but could complicate attempts to add support for other protocols. If the proxy talks to postscreen then that is where
    Message 1 of 17 , Jun 1, 2012
    • 0 Attachment
      Willy Tarreau:
      > I'm totally in sync with you on this. On the one hand, if it is as
      > trivial to make smtpd parse the PROXY line as it was for postscreen,
      > it can solve the problem by having postscreen not consume the first
      > line, which makes sense in that postscreen remains the first layer
      > analyser which doesn't mangle data on the connection. But I can also
      > understand that you're not necessarily interested in supporting a new
      > method of passing connection information.

      Sorry, that might work for haproxy, but could complicate attempts
      to add support for other protocols. If the proxy talks to postscreen
      then that is where its protocol will terminate.

      Wietse
    • Wietse Venema
      ... In an event-driven program such as postscreen, this code breaks when the proxy line arrives as multiple fragments. If the program does not drain proxy line
      Message 2 of 17 , Jun 7, 2012
      • 0 Attachment
        Willy Tarreau:
        > > >Regardless of command format details, if the proxy prepends a command
        > > >to the client's SMTP stream, then postscreen must use unbuffered
        > > >I/O to read that command. If buffering were turned on, the buffering
        > > >layer could read past the proxy's<CR><LF> and eat up part of the
        > > >client input kind-of like CVE-2011-0411.
        >
        > Precisely on this point there is an easier way, it consists in using
        > recv(MSG_PEEK). The big advantage is that you don't need to store the
        > temporary bytes you've read since they remain in the kernel's buffers.
        > So it more or less looks like this :
        >
        > len = recv(fd, trash, sizeof(trash), MSG_PEEK);
        > if (len == -1 && errno == EAGAIN)
        > return;
        >
        > lf = memchr(trash, '\n', len);
        > if (lf == NULL) {
        > if (len < trash) /* Huh?? */
        > return;
        > /* else abort the connection */
        > }

        In an event-driven program such as postscreen, this code breaks
        when the proxy line arrives as multiple fragments.

        If the program does not drain proxy line fragments from the kernel
        buffer, then the socket remains readable and the program will go
        into a read-notification loop until the entire line is received.

        This implies that the following suggestion is not valid for an
        event-driven program such as postscreen:

        > On the one hand, if it is as trivial to make smtpd parse the PROXY
        > line as it was for postscreen, it can solve the problem by having
        > postscreen not consume the first line, which makes sense in that
        > postscreen remains the first layer analyser which doesn't mangle
        > data on the connection.

        Either you need to update the protocol spec (require non-fragmented
        proxy lines) or provide a code example that doesn't go into a
        read-notification loop when the proxy line arrives as multiple
        fragments.

        Wietse
      • Willy Tarreau
        Hi Wietse, [ just subscribed to the list, I realized that our past conversation was dropped since I was not subscribed, never mind ] ... Indeed this is totally
        Message 3 of 17 , Jun 8, 2012
        • 0 Attachment
          Hi Wietse,

          [ just subscribed to the list, I realized that our past conversation
          was dropped since I was not subscribed, never mind ]

          On Thu, Jun 07, 2012 at 08:16:53PM -0400, Wietse Venema wrote:
          > Willy Tarreau:
          > > > >Regardless of command format details, if the proxy prepends a command
          > > > >to the client's SMTP stream, then postscreen must use unbuffered
          > > > >I/O to read that command. If buffering were turned on, the buffering
          > > > >layer could read past the proxy's<CR><LF> and eat up part of the
          > > > >client input kind-of like CVE-2011-0411.
          > >
          > > Precisely on this point there is an easier way, it consists in using
          > > recv(MSG_PEEK). The big advantage is that you don't need to store the
          > > temporary bytes you've read since they remain in the kernel's buffers.
          > > So it more or less looks like this :
          > >
          > > len = recv(fd, trash, sizeof(trash), MSG_PEEK);
          > > if (len == -1 && errno == EAGAIN)
          > > return;
          > >
          > > lf = memchr(trash, '\n', len);
          > > if (lf == NULL) {
          > > if (len < trash) /* Huh?? */
          > > return;
          > > /* else abort the connection */
          > > }
          >
          > In an event-driven program such as postscreen, this code breaks
          > when the proxy line arrives as multiple fragments.
          >
          > If the program does not drain proxy line fragments from the kernel
          > buffer, then the socket remains readable and the program will go
          > into a read-notification loop until the entire line is received.

          Indeed this is totally true. I would say that given the short size
          of the message the risk of this occurring is barely 0 explaining
          whit this has probably never hit anybody, but it would be sufficient
          that someone implements the protocol using a series of
          write(fd, buf++, 1) and you'd be spinning (even worse if it dies in
          the middle).

          I've seen implementations doing recv(MSG_PEEK) and reject connections
          from incomplete messages (again, almost zero risk but YMMV).

          In haproxy, I'm using the input buffer associated to the fd, so I
          don't have this problem. I'm basically doing a recv() on the buffer.
          I think it is equivalent to the VSTREAM you're using.

          In postscreen you don't want to do this since you don't want to
          consume any possible incoming data (btw you probably drop the
          connection if you get any data at this point). That said, if you
          have a buffer associated to the connection, then you can perform
          the first MSG_PEEK to check the pending data size and then a real
          recv() to only consume up to the end of line.

          But then doing so indeed invalidates the following suggestion.

          > This implies that the following suggestion is not valid for an
          > event-driven program such as postscreen:
          >
          > > On the one hand, if it is as trivial to make smtpd parse the PROXY
          > > line as it was for postscreen, it can solve the problem by having
          > > postscreen not consume the first line, which makes sense in that
          > > postscreen remains the first layer analyser which doesn't mangle
          > > data on the connection.


          > Either you need to update the protocol spec (require non-fragmented
          > proxy lines)

          I have mixed opinions on this. On the one hand, we can't really impose
          lower layers segmentation behaviour, so from a layering perspective, it
          is not correct. On the other hand, the use cases for the protocol are
          very specific. We're the very first segment over the connection so we
          are always allowed to send at least one MSS. Nobody should sanely use
          this proxy line on connections with an MSS lower than the 116 bytes a
          max line may be for long IPv6 addresses and ports.

          So indeed, I'm tempted to follow your suggestion, it will ease processing
          for everyone and ensure that nobody tries sending fragmented lines. We'd
          rely on a sane lower layer and declare other cases out of scope.

          > or provide a code example that doesn't go into a
          > read-notification loop when the proxy line arrives as multiple
          > fragments.

          With a buffer this problem does not happen, but it's the first case I'm
          facing this need with fd passing, which makes me scratch my head a lot.
          I really like the way you're plugging postscreen in front of smtpd, and
          I'd like to ensure we don't make it complex to keep this nice model.

          That's why I think that adding a sane requirement in the spec should be
          the most adequate solution. If in the mean time you get a smarter idea,
          do not hesitate to share it :-)

          I'll keep thinking about it a bit before updating it. I think I will also
          propose some generic code in the spec for both sides.

          Best regards,
          Willy
        • Wietse Venema
          Non-production release postfix-2.10-20120617-nonprod has support for up-stream proxy agents in postscreen(8) and smtpd(8). To enable, specify one of:
          Message 4 of 17 , Jun 17, 2012
          • 0 Attachment
            Non-production release postfix-2.10-20120617-nonprod has support
            for up-stream proxy agents in postscreen(8) and smtpd(8).

            To enable, specify one of:

            postscreen_upstream_proxy_protocol = haproxy
            smtpd_upstream_proxy_protocol = haproxy

            haproxy is not the only proxy agent that works with Postfix. Support
            for nginx with proxied SASL authentication is available in Postfix
            2.9 smtpd(8). This uses the XCLIENT protocol.

            Wietse
          • Willy Tarreau
            Hi Wietse, ... This is awesome work, it opens a wider interoperability between our components which are commonly placed close to each other in a number of
            Message 5 of 17 , Jun 17, 2012
            • 0 Attachment
              Hi Wietse,

              On Sun, Jun 17, 2012 at 08:25:12PM -0400, Wietse Venema wrote:
              > Non-production release postfix-2.10-20120617-nonprod has support
              > for up-stream proxy agents in postscreen(8) and smtpd(8).
              >
              > To enable, specify one of:
              >
              > postscreen_upstream_proxy_protocol = haproxy
              > smtpd_upstream_proxy_protocol = haproxy
              >
              > haproxy is not the only proxy agent that works with Postfix. Support
              > for nginx with proxied SASL authentication is available in Postfix
              > 2.9 smtpd(8). This uses the XCLIENT protocol.

              This is awesome work, it opens a wider interoperability between our
              components which are commonly placed close to each other in a number
              of infrastructures.

              I hope David (who originally asked for the feature) will quickly test
              and provide us with some feedback.

              I'll forward your mail to the haproxy mailing list, since I know there
              are a number of postfix users there too.

              Best regards,
              Willy
            • Wietse Venema
              ... After another week of testing, this is now released as a regular development release postfix-2.10-20120625. Wietse
              Message 6 of 17 , Jun 25, 2012
              • 0 Attachment
                > Non-production release postfix-2.10-20120617-nonprod has support
                > for up-stream proxy agents in postscreen(8) and smtpd(8).
                >
                > To enable, specify one of:
                >
                > postscreen_upstream_proxy_protocol = haproxy
                > smtpd_upstream_proxy_protocol = haproxy
                >
                > haproxy is not the only proxy agent that works with Postfix. Support
                > for nginx with proxied SASL authentication is available in Postfix
                > 2.9 smtpd(8). This uses the XCLIENT protocol.

                After another week of testing, this is now released as a regular
                development release postfix-2.10-20120625.

                Wietse
              Your message has been successfully submitted and would be delivered to recipients shortly.