* [PATCH] nf_conntrack_sip: check for trailing spaces
@ 2016-10-19 8:24 Ulrich Weber
2016-10-20 18:17 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weber @ 2016-10-19 8:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: kaber
on SIP requests, so a fragmented TCP SIP packet starting with
INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
Content-Length: 0
will not bet interpreted as an INVITE request.
Confirm with RFC 3261
Request-Line = Method SP Request-URI SP SIP-Version CRLF
Signed-off-by: Ulrich Weber <ulrich.weber@riverbed.com>
---
net/netfilter/nf_conntrack_sip.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 7700556..e37f8da 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1436,9 +1436,11 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,
handler = &sip_handlers[i];
if (handler->request == NULL)
continue;
- if (*datalen < handler->len ||
+ if (*datalen < handler->len + 1 ||
strncasecmp(*dptr, handler->method, handler->len))
continue;
+ if ((*dptr)[handler->len] != ' ')
+ continue;
if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_CSEQ,
&matchoff, &matchlen) <= 0) {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nf_conntrack_sip: check for trailing spaces
2016-10-19 8:24 [PATCH] nf_conntrack_sip: check for trailing spaces Ulrich Weber
@ 2016-10-20 18:17 ` Pablo Neira Ayuso
2016-10-24 16:05 ` Ulrich Weber
0 siblings, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-10-20 18:17 UTC (permalink / raw)
To: Ulrich Weber; +Cc: netfilter-devel, kaber, Marco Angaroni
Hi Ulrich,
Cc'ing Marco Angaroni.
On Wed, Oct 19, 2016 at 10:24:02AM +0200, Ulrich Weber wrote:
> on SIP requests, so a fragmented TCP SIP packet starting with
> INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
> Content-Length: 0
>
> will not bet interpreted as an INVITE request.
>
> Confirm with RFC 3261
> Request-Line = Method SP Request-URI SP SIP-Version CRLF
Marco has missed your email, so I'm copying and pasting what he told
me wrt. to your patch:
My understanding is that Ulrich is referring to something like an
"Allow" header of a SIP message where the header name is part of a TCP
segment and the header value is part of the following TCP segment:
Allow: INVITE,ACK,CANCEL,OPTIONS,BYE
So I think the fix (check for a space after the method name) is
correct.
However maybe the check could be made more robust, because if you have
the fragmented "Allow" header containing the method names with a space
before comma (it's permitted by SIP grammar) like you see below, you
would again think that it's the first line of the SIP message:
Allow: INVITE , ACK , CANCEL , OPTIONS , BYE
Maybe it's worth checking two bytes after method name; the first byte
must be a space and the second must a alphanumeric character: the URI
following the method name has to start with "sip:" or "sips:" or some
other scheme, which starts for sure with an ALPHA character.
Request-Line = Method SP Request-URI SP SIP-Version CRLF
Request-URI = SIP-URI / SIPS-URI / absoluteURI
absoluteURI = scheme ":" ( hier-part / opaque-part )
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nf_conntrack_sip: check for trailing spaces
2016-10-20 18:17 ` Pablo Neira Ayuso
@ 2016-10-24 16:05 ` Ulrich Weber
0 siblings, 0 replies; 5+ messages in thread
From: Ulrich Weber @ 2016-10-24 16:05 UTC (permalink / raw)
To: Pablo Neira Ayuso, Ulrich Weber; +Cc: netfilter-devel, kaber, Marco Angaroni
Hi Pablo,
good point, will send a new version!
Thanks
Ulrich
On 20.10.2016 20:17, Pablo Neira Ayuso wrote:
> Hi Ulrich,
>
> Cc'ing Marco Angaroni.
>
> On Wed, Oct 19, 2016 at 10:24:02AM +0200, Ulrich Weber wrote:
>> on SIP requests, so a fragmented TCP SIP packet starting with
>> INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
>> Content-Length: 0
>>
>> will not bet interpreted as an INVITE request.
>>
>> Confirm with RFC 3261
>> Request-Line = Method SP Request-URI SP SIP-Version CRLF
>
> Marco has missed your email, so I'm copying and pasting what he told
> me wrt. to your patch:
>
> My understanding is that Ulrich is referring to something like an
> "Allow" header of a SIP message where the header name is part of a TCP
> segment and the header value is part of the following TCP segment:
>
> Allow: INVITE,ACK,CANCEL,OPTIONS,BYE
>
> So I think the fix (check for a space after the method name) is
> correct.
> However maybe the check could be made more robust, because if you have
> the fragmented "Allow" header containing the method names with a space
> before comma (it's permitted by SIP grammar) like you see below, you
> would again think that it's the first line of the SIP message:
>
> Allow: INVITE , ACK , CANCEL , OPTIONS , BYE
>
> Maybe it's worth checking two bytes after method name; the first byte
> must be a space and the second must a alphanumeric character: the URI
> following the method name has to start with "sip:" or "sips:" or some
> other scheme, which starts for sure with an ALPHA character.
>
> Request-Line = Method SP Request-URI SP SIP-Version CRLF
> Request-URI = SIP-URI / SIPS-URI / absoluteURI
> absoluteURI = scheme ":" ( hier-part / opaque-part )
> scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nf_conntrack_sip: check for trailing spaces
2016-10-18 9:13 Ulrich Weber
@ 2016-10-18 21:47 ` Pablo Neira Ayuso
0 siblings, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2016-10-18 21:47 UTC (permalink / raw)
To: Ulrich Weber; +Cc: netfilter, kaber
Hi Ulrich,
On Tue, Oct 18, 2016 at 11:13:18AM +0200, Ulrich Weber wrote:
> on SIP requests, so a fragmented TCP SIP packet starting with
> INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
> Content-Length: 0
>
> will not bet interpreted as an INVITE request.
>
> Confirm with RFC 3261
> Request-Line = Method SP Request-URI SP SIP-Version CRLF
Please, submit this to netfilter-devel@vger.kernel.org.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] nf_conntrack_sip: check for trailing spaces
@ 2016-10-18 9:13 Ulrich Weber
2016-10-18 21:47 ` Pablo Neira Ayuso
0 siblings, 1 reply; 5+ messages in thread
From: Ulrich Weber @ 2016-10-18 9:13 UTC (permalink / raw)
To: netfilter; +Cc: kaber
on SIP requests, so a fragmented TCP SIP packet starting with
INVITE,NOTIFY,OPTIONS,REFER,REGISTER,UPDATE,SUBSCRIBE
Content-Length: 0
will not bet interpreted as an INVITE request.
Confirm with RFC 3261
Request-Line = Method SP Request-URI SP SIP-Version CRLF
Signed-off-by: Ulrich Weber <ulrich.weber@riverbed.com>
---
net/netfilter/nf_conntrack_sip.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index 7700556..e37f8da 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1436,9 +1436,11 @@ static int process_sip_request(struct sk_buff *skb, unsigned int protoff,
handler = &sip_handlers[i];
if (handler->request == NULL)
continue;
- if (*datalen < handler->len ||
+ if (*datalen < handler->len + 1 ||
strncasecmp(*dptr, handler->method, handler->len))
continue;
+ if ((*dptr)[handler->len] != ' ')
+ continue;
if (ct_sip_get_header(ct, *dptr, 0, *datalen, SIP_HDR_CSEQ,
&matchoff, &matchlen) <= 0) {
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-24 16:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-19 8:24 [PATCH] nf_conntrack_sip: check for trailing spaces Ulrich Weber
2016-10-20 18:17 ` Pablo Neira Ayuso
2016-10-24 16:05 ` Ulrich Weber
-- strict thread matches above, loose matches on Subject: below --
2016-10-18 9:13 Ulrich Weber
2016-10-18 21:47 ` Pablo Neira Ayuso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.