relayd: DELETE with payload

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

relayd: DELETE with payload

Rivo Nurges-2
>Synopsis:      DELETE method with payload in relayd
>Category:      n/a
>Environment:
        System      : OpenBSD 6.0
        Details     : OpenBSD 6.0-current (GENERIC) #2254: Fri Sep  9 05:41:55 MDT 2016
                         [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
 
        Architecture: OpenBSD.amd64
        Machine     : amd64
>Description:
RFC 2616(obsoleted by RFC 7231) doesn't talk about payload of DELETE method.
RFC 7231 says:
   A payload within a DELETE request message has no defined semantics;
   sending a payload body on a DELETE request might cause some existing
   implementations to reject the request.

Which indirectly allows DELETE method to have payload.

At least Atlassian JIRA uses DELETE method with payload and will break if relayd forwards the request without payload.
 
>How-To-Repeat:
>Fix:
 
Following patch moves DELETE to the methods where relayd expects to have payload.

Index: usr.sbin/relayd/relay_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.63
diff -u -p -r1.63 relay_http.c
--- usr.sbin/relayd/relay_http.c 26 Sep 2016 16:25:16 -0000 1.63
+++ usr.sbin/relayd/relay_http.c 2 Mar 2017 16:14:04 -0000
@@ -418,7 +418,6 @@ relay_read_http(struct bufferevent *bev,
  cre->toread = TOREAD_UNLIMITED;
  bev->readcb = relay_read;
  break;
- case HTTP_METHOD_DELETE:
  case HTTP_METHOD_GET:
  case HTTP_METHOD_HEAD:
  /* WebDAV methods */
@@ -426,6 +425,7 @@ relay_read_http(struct bufferevent *bev,
  case HTTP_METHOD_MOVE:
  cre->toread = 0;
  break;
+ case HTTP_METHOD_DELETE:
  case HTTP_METHOD_OPTIONS:
  case HTTP_METHOD_POST:
  case HTTP_METHOD_PUT:
 
 
begin-base64 644 relayd_delete.diff
SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
b24gMS42MwpkaWZmIC11IC1wIC1yMS42MyByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
ZC9yZWxheV9odHRwLmMJMjYgU2VwIDIwMTYgMTY6MjU6MTYgLTAwMDAJMS42MworKysgdXNyLnNi
aW4vcmVsYXlkL3JlbGF5X2h0dHAuYwkyIE1hciAyMDE3IDE2OjE0OjA0IC0wMDAwCkBAIC00MTgs
NyArNDE4LDYgQEAgcmVsYXlfcmVhZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCQkJ
Y3JlLT50b3JlYWQgPSBUT1JFQURfVU5MSU1JVEVEOwogCQkJYmV2LT5yZWFkY2IgPSByZWxheV9y
ZWFkOwogCQkJYnJlYWs7Ci0JCWNhc2UgSFRUUF9NRVRIT0RfREVMRVRFOgogCQljYXNlIEhUVFBf
TUVUSE9EX0dFVDoKIAkJY2FzZSBIVFRQX01FVEhPRF9IRUFEOgogCQkvKiBXZWJEQVYgbWV0aG9k
cyAqLwpAQCAtNDI2LDYgKzQyNSw3IEBAIHJlbGF5X3JlYWRfaHR0cChzdHJ1Y3QgYnVmZmVyZXZl
bnQgKmJldiwKIAkJY2FzZSBIVFRQX01FVEhPRF9NT1ZFOgogCQkJY3JlLT50b3JlYWQgPSAwOwog
CQkJYnJlYWs7CisJCWNhc2UgSFRUUF9NRVRIT0RfREVMRVRFOgogCQljYXNlIEhUVFBfTUVUSE9E
X09QVElPTlM6CiAJCWNhc2UgSFRUUF9NRVRIT0RfUE9TVDoKIAkJY2FzZSBIVFRQX01FVEhPRF9Q
VVQ6Cg==
==== 
 
dmesg:
OpenBSD 6.0-current (GENERIC) #2254: Fri Sep  9 05:41:55 MDT 2016
    [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
real mem = 1056804864 (1007MB)
avail mem = 1020387328 (973MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf69d0 (9 entries)
bios0: vendor SeaBIOS version "1.9.1-1.fc24" date 04/01/2014
bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S5
acpi0: tables DSDT FACP APIC
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel Xeon E312xx (Sandy Bridge), 2200.33 MHz
cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SS,SSE3,PCLMUL,SSSE3,CX16,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,HV,NXE,PAGE1GB,LONG,LAHF,ARAT
cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 64b/line 16-way L2 cache
cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 1000MHz
ioapic0 at mainbus0: apid 0 pa 0xfec00000, version 11, 24 pins
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
"ACPI0006" at acpi0 not configured
"PNP0303" at acpi0 not configured
"PNP0F13" at acpi0 not configured
"PNP0700" at acpi0 not configured
"PNP0501" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"PNP0A06" at acpi0 not configured
"QEMU0002" at acpi0 not configured
"PNP0A06" at acpi0 not configured
pvbus0 at mainbus0: KVM
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel 82441FX" rev 0x02
pcib0 at pci0 dev 1 function 0 "Intel 82371SB ISA" rev 0x00
pciide0 at pci0 dev 1 function 1 "Intel 82371SB IDE" rev 0x00: DMA, channel 0 wired to compatibility, channel 1 wired to compatibility
atapiscsi0 at pciide0 channel 0 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0: <QEMU, QEMU DVD-ROM, 2.5+> ATAPI 5/cdrom removable
cd0(pciide0:0:0): using PIO mode 4, DMA mode 2
pciide0: channel 1 disabled (no drives)
piixpm0 at pci0 dev 1 function 3 "Intel 82371AB Power" rev 0x03: apic 0 int 9
iic0 at piixpm0
vga1 at pci0 dev 2 function 0 "Red Hat QXL Video" rev 0x04
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
virtio0 at pci0 dev 3 function 0 "Qumranet Virtio Network" rev 0x00
vio0 at virtio0: address 52:54:00:8c:e5:0e
virtio0: msix shared
virtio1 at pci0 dev 4 function 0 "Qumranet Virtio Network" rev 0x00
vio1 at virtio1: address 52:54:00:98:f0:af
virtio1: msix shared
azalia0 at pci0 dev 5 function 0 "Intel 82801FB HD Audio" rev 0x01: apic 0 int 10
azalia0: No codecs found
uhci0 at pci0 dev 6 function 0 "Intel 82801I USB" rev 0x03: apic 0 int 10
uhci1 at pci0 dev 6 function 1 "Intel 82801I USB" rev 0x03: apic 0 int 11
uhci2 at pci0 dev 6 function 2 "Intel 82801I USB" rev 0x03: apic 0 int 11
ehci0 at pci0 dev 6 function 7 "Intel 82801I USB" rev 0x03: apic 0 int 10
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 configuration 1 interface 0 "Intel EHCI root hub" rev 2.00/1.00 addr 1
virtio2 at pci0 dev 7 function 0 "Qumranet Virtio Console" rev 0x00
virtio2: no matching child driver; not configured
virtio3 at pci0 dev 8 function 0 "Qumranet Virtio Storage" rev 0x00
vioblk0 at virtio3
scsibus2 at vioblk0: 2 targets
sd0 at scsibus2 targ 0 lun 0: <VirtIO, Block Device, > SCSI3 0/direct fixed
sd0: 4096MB, 512 bytes/sector, 8388608 sectors
virtio3: msix shared
virtio4 at pci0 dev 9 function 0 "Qumranet Virtio Memory" rev 0x00
viomb0 at virtio4
virtio4: apic 0 int 10
isa0 at pcib0
isadma0 at isa0
fdc0 at isa0 port 0x3f0/6 irq 6 drq 2
fd0 at fdc0 drive 1: density unknown
com0 at isa0 port 0x3f8/8 irq 4: ns16550a, 16 byte fifo
com0: console
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pcppi0 at isa0 port 0x61
spkr0 at pcppi0
usb1 at uhci0: USB revision 1.0
uhub1 at usb1 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 addr 1
usb2 at uhci1: USB revision 1.0
uhub2 at usb2 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 addr 1
usb3 at uhci2: USB revision 1.0
uhub3 at usb3 configuration 1 interface 0 "Intel UHCI root hub" rev 1.00/1.00 addr 1
vscsi0 at root
scsibus3 at vscsi0: 256 targets
softraid0 at root
scsibus4 at softraid0: 256 targets
root on sd0a (2425b24d62d3fc96.a) swap on sd0b dump on sd0b

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: relayd: DELETE with payload

Sebastian Benoit-3
Rivo Nurges([hidden email]) on 2017.03.02 16:32:40 +0000:

> >Synopsis:?????????? DELETE method with payload in relayd
> >Category:?????????? n/a
> >Environment:
> ?????????????? System?????????? : OpenBSD 6.0
> ?????????????? Details???????? : OpenBSD 6.0-current (GENERIC) #2254: Fri Sep?? 9 05:41:55 MDT 2016
> ???????????????????????????????????????????????? [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
> ??
> ?????????????? Architecture: OpenBSD.amd64
> ?????????????? Machine???????? : amd64
> >Description:
> RFC 2616(obsoleted by RFC 7231) doesn't talk about payload of DELETE method.
> RFC 7231 says:
>    A payload within a DELETE request message has no defined semantics;
>    sending a payload body on a DELETE request might cause some existing
>    implementations to reject the request.
>
> Which indirectly allows DELETE method to have payload.
>
> At least Atlassian JIRA uses DELETE method with payload and will break if relayd forwards the request without payload.

Hi,

i thought i had fixed this in 2012, but apparently i never commited that
diff even though i had oks for it. And i remember we had discussions about
this in the past.

The question here is: do we need relayd to block this to protect whatever
application is behind it? Do we gain anything from blocking this request?

Anecdotal evidence(*) suggests that no one should rely on DELETE having a body.

Reyk?


(*)
http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request
https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Summary_table

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: relayd: DELETE with payload

Rivo Nurges-2
Hi!

At least Atlassian JIRA will break(thorws exception and returns 500) if relayd eats its json payload from DELETE.

Rivo

On 03/03/2017, 15:47, "Sebastian Benoit" <[hidden email]> wrote:

    Rivo Nurges([hidden email]) on 2017.03.02 16:32:40 +0000:
    > >Synopsis:?????????? DELETE method with payload in relayd
    > >Category:?????????? n/a
    > >Environment:
    > ?????????????? System?????????? : OpenBSD 6.0
    > ?????????????? Details???????? : OpenBSD 6.0-current (GENERIC) #2254: Fri Sep?? 9 05:41:55 MDT 2016
    > ???????????????????????????????????????????????? [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
    > ??
    > ?????????????? Architecture: OpenBSD.amd64
    > ?????????????? Machine???????? : amd64
    > >Description:
    > RFC 2616(obsoleted by RFC 7231) doesn't talk about payload of DELETE method.
    > RFC 7231 says:
    >    A payload within a DELETE request message has no defined semantics;
    >    sending a payload body on a DELETE request might cause some existing
    >    implementations to reject the request.
    >
    > Which indirectly allows DELETE method to have payload.
    >
    > At least Atlassian JIRA uses DELETE method with payload and will break if relayd forwards the request without payload.
   
    Hi,
   
    i thought i had fixed this in 2012, but apparently i never commited that
    diff even though i had oks for it. And i remember we had discussions about
    this in the past.
   
    The question here is: do we need relayd to block this to protect whatever
    application is behind it? Do we gain anything from blocking this request?
   
    Anecdotal evidence(*) suggests that no one should rely on DELETE having a body.
   
    Reyk?
   
   
    (*)
    http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request
    https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Summary_table
   

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: relayd: DELETE with payload

Rivo Nurges-2
Hi!

Friendly reminder:) I'd like to get rid of the local patch.

Rivo

On 03/03/2017, 16:01, "Rivo Nurges" <[hidden email]> wrote:

    Hi!
   
    At least Atlassian JIRA will break(thorws exception and returns 500) if relayd eats its json payload from DELETE.
   
    Rivo
   
    On 03/03/2017, 15:47, "Sebastian Benoit" <[hidden email]> wrote:
   
        Rivo Nurges([hidden email]) on 2017.03.02 16:32:40 +0000:
        > >Synopsis:?????????? DELETE method with payload in relayd
        > >Category:?????????? n/a
        > >Environment:
        > ?????????????? System?????????? : OpenBSD 6.0
        > ?????????????? Details???????? : OpenBSD 6.0-current (GENERIC) #2254: Fri Sep?? 9 05:41:55 MDT 2016
        > ???????????????????????????????????????????????? [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
        > ??
        > ?????????????? Architecture: OpenBSD.amd64
        > ?????????????? Machine???????? : amd64
        > >Description:
        > RFC 2616(obsoleted by RFC 7231) doesn't talk about payload of DELETE method.
        > RFC 7231 says:
        >    A payload within a DELETE request message has no defined semantics;
        >    sending a payload body on a DELETE request might cause some existing
        >    implementations to reject the request.
        >
        > Which indirectly allows DELETE method to have payload.
        >
        > At least Atlassian JIRA uses DELETE method with payload and will break if relayd forwards the request without payload.
       
        Hi,
       
        i thought i had fixed this in 2012, but apparently i never commited that
        diff even though i had oks for it. And i remember we had discussions about
        this in the past.
       
        The question here is: do we need relayd to block this to protect whatever
        application is behind it? Do we gain anything from blocking this request?
       
        Anecdotal evidence(*) suggests that no one should rely on DELETE having a body.
       
        Reyk?
       
       
        (*)
        http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request
        https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Summary_table
       
   
   

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: relayd: DELETE with payload

Reyk Floeter-2
On Fri, Mar 10, 2017 at 08:41:06PM +0000, Rivo Nurges wrote:
> Hi!
>
> Friendly reminder:) I'd like to get rid of the local patch.
>
> Rivo
>

Thanks, committed!  (to relayd and httpd)

Reyk

> On 03/03/2017, 16:01, "Rivo Nurges" <[hidden email]> wrote:
>
>     Hi!
>    
>     At least Atlassian JIRA will break(thorws exception and returns 500) if relayd eats its json payload from DELETE.
>    
>     Rivo
>    
>     On 03/03/2017, 15:47, "Sebastian Benoit" <[hidden email]> wrote:
>    
>         Rivo Nurges([hidden email]) on 2017.03.02 16:32:40 +0000:
>         > >Synopsis:?????????? DELETE method with payload in relayd
>         > >Category:?????????? n/a
>         > >Environment:
>         > ?????????????? System?????????? : OpenBSD 6.0
>         > ?????????????? Details???????? : OpenBSD 6.0-current (GENERIC) #2254: Fri Sep?? 9 05:41:55 MDT 2016
>         > ???????????????????????????????????????????????? [hidden email]:/usr/src/sys/arch/amd64/compile/GENERIC
>         > ??
>         > ?????????????? Architecture: OpenBSD.amd64
>         > ?????????????? Machine???????? : amd64
>         > >Description:
>         > RFC 2616(obsoleted by RFC 7231) doesn't talk about payload of DELETE method.
>         > RFC 7231 says:
>         >    A payload within a DELETE request message has no defined semantics;
>         >    sending a payload body on a DELETE request might cause some existing
>         >    implementations to reject the request.
>         >
>         > Which indirectly allows DELETE method to have payload.
>         >
>         > At least Atlassian JIRA uses DELETE method with payload and will break if relayd forwards the request without payload.
>        
>         Hi,
>        
>         i thought i had fixed this in 2012, but apparently i never commited that
>         diff even though i had oks for it. And i remember we had discussions about
>         this in the past.
>        
>         The question here is: do we need relayd to block this to protect whatever
>         application is behind it? Do we gain anything from blocking this request?
>        
>         Anecdotal evidence(*) suggests that no one should rely on DELETE having a body.
>        
>         Reyk?
>        
>        
>         (*)
>         http://stackoverflow.com/questions/299628/is-an-entity-body-allowed-for-an-http-delete-request
>         https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Summary_table
>        
>    
>    
>

--

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: relayd: DELETE with payload

Reyk Floeter-2
In reply to this post by Sebastian Benoit-3
On Fri, Mar 03, 2017 at 02:47:09PM +0100, Sebastian Benoit wrote:
> i thought i had fixed this in 2012, but apparently i never commited that
> diff even though i had oks for it. And i remember we had discussions about
> this in the past.
>

Well, if it is not in the tree, it does not exist :)
Another anedote: I once had a half-working driver for zyd(4), I called
it zy(4) but I never committed it, somebody else came and wrote a new
driver instead, and I accidentally deleted my code.

> The question here is: do we need relayd to block this to protect whatever
> application is behind it? Do we gain anything from blocking this request?
>

Yes, I want to keep relayd as strict as possible.  So we can change
and extend the list if there is enough evidence but I'm not going to
open it up completely.

> Anecdotal evidence(*) suggests that no one should rely on DELETE having a body.
>

OK, I fixed it with the proposed diff.

Reyk

Loading...