Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Origin.isValid() incorrectly rejects chrome-extension scheme #2556

Closed
werdnum opened this issue Jan 15, 2024 · 5 comments
Closed

Origin.isValid() incorrectly rejects chrome-extension scheme #2556

werdnum opened this issue Jan 15, 2024 · 5 comments
Assignees
@tsegismont
Labels
bug
Milestone
4.5.6

Comments

@werdnum
Copy link

werdnum commented Jan 15, 2024

Version

4.4.6

Context

I was looking into zyclonite/nassh-relay#21, and when doing so, I noticed that an 'accept all' CorsHandler nonetheless rejects requests where Origin is set to chrome-extension://foobar

Do you have a reproducer?

https://github.com/zyclonite/nassh-relay

Steps to reproduce

  1. Run the project - docker run zyclonite/nassh-relay -p 8022
  2. Try to make a request to it - curl -H 'Origin: chrome-extension://foobar' http://localhost:8022/proxy?host=foo.bar.com

Notice in the logs:

Sep 25, 2023 5:58:44 AM io.vertx.ext.web.RoutingContext
SEVERE: Unhandled exception in router
java.lang.IllegalStateException: CORS Rejected - Invalid origin
        at io.vertx.ext.web.handler.impl.CorsHandlerImpl.handle(CorsHandlerImpl.java:252)
        at io.vertx.ext.web.handler.impl.CorsHandlerImpl.handle(CorsHandlerImpl.java:41)
        at io.vertx.ext.web.impl.RouteState.handleContext(RouteState.java:1286)
        at io.vertx.ext.web.impl.RoutingContextImplBase.iterateNext(RoutingContextImplBase.java:177)
        at io.vertx.ext.web.impl.RoutingContextImpl.next(RoutingContextImpl.java:144)
        at io.vertx.ext.web.impl.RouterImpl.handle(RouterImpl.java:68)
        at io.vertx.ext.web.impl.RouterImpl.handle(RouterImpl.java:37)
        at io.vertx.core.http.impl.Http1xServerRequestHandler.handle(Http1xServerRequestHandler.java:57)
        at io.vertx.core.http.impl.Http1xServerRequestHandler.handle(Http1xServerRequestHandler.java:30)
        at io.vertx.core.impl.EventLoopContext.emit(EventLoopContext.java:55)
        at io.vertx.core.impl.DuplicatedContext.emit(DuplicatedContext.java:179)
        at io.vertx.core.http.impl.Http1xServerConnection.handleMessage(Http1xServerConnection.java:174)
        at io.vertx.core.net.impl.ConnectionBase.read(ConnectionBase.java:159)
        at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:153)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
        at io.netty.channel.ChannelInboundHandlerAdapter.channelRead(ChannelInboundHandlerAdapter.java:93)
        at io.netty.handler.codec.http.websocketx.extensions.WebSocketServerExtensionHandler.onHttpRequestChannelRead(WebSocketServerExtensionHandler.java:160)
        at io.netty.handler.codec.http.websocketx.extensions.WebSocketServerExtensionHandler.channelRead(WebSocketServerExtensionHandler.java:83)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:442)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
        at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:346)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:318)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
        at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
        at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
        at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
        at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
        at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
        at java.base/java.lang.Thread.run(Thread.java:833)

Extra

See zyclonite/nassh-relay#21
@werdnum werdnum added the bug label Jan 15, 2024
@werdnum werdnum mentioned this issue Jan 15, 2024
Closed
@tsegismont tsegismont self-assigned this Jan 18, 2024
@tsegismont tsegismont added this to the 4.5.2 milestone Jan 18, 2024
@tsegismont
Copy link
Contributor

I'll take a look, thanks for the report

@tsegismont
Copy link
Contributor

@werdnum as a workaround, you can modify .addRelativeOrigin(".*") to .addRelativeOrigin("(https|chrome-extension)://.*")

@tsegismont
Copy link
Contributor

There are different behaviors in the CorsHandler implementation:

  • if all origins are accepted, the implementation check if the origin header is a valid Origin with Origin.isValid
  • if a static origin is configured, its format is validated by the Origin constructor; then the origin header value is implicitly validated when comparing the strings with Origin.sameOrigin
  • if a relative origin is configured, we just trust the pattern provided by the user

@pmlopes I believe we should update the code so that:

  • CorsHandler checks if the origin header value is a valid origin, even when tested against a relative origin pattern
  • Origin supports chrome-extensions

Sounds good to you?

@vietj vietj modified the milestones: 4.5.2, 4.5.3 Jan 30, 2024
@vietj
Copy link
Contributor

vietj commented Feb 5, 2024

@pmlopes ping :-)

@vietj vietj modified the milestones: 4.5.3, 4.5.4 Feb 6, 2024
@vietj vietj modified the milestones: 4.5.4, 4.5.5 Feb 22, 2024
@vietj vietj modified the milestones: 4.5.5, 4.5.6 Mar 14, 2024
tsegismont added a commit to tsegismont/vertx-web that referenced this issue Mar 14, 2024
@tsegismont
CORS: support Chrome extensions
b49f298 
See vert-x3#2556

This commit makes it possible to use star origin in CORS handler setup with a Chrome extension client app.
Chrome Extensions use an origin of the follow form: chrome-extension://<extension-id>
An extension id is 32 chars long and contains chars from 'a' to 'p' (0->9 + A->F translated to a->p).

Signed-off-by: Thomas Segismont <[email protected]>
@tsegismont tsegismont mentioned this issue Mar 14, 2024
Merged
tsegismont added a commit to tsegismont/vertx-web that referenced this issue Mar 20, 2024
@tsegismont
CORS: support Chrome extensions
28be1f0 
See vert-x3#2556

This commit makes it possible to use star origin in CORS handler setup with a Chrome extension client app.
Chrome Extensions use an origin of the follow form: chrome-extension://<extension-id>
An extension id is 32 chars long and contains chars from 'a' to 'p' (0->9 + A->F translated to a->p).

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit that referenced this issue Mar 20, 2024
@tsegismont
CORS: support Chrome extensions (#2581)
8882049 
* CORS: support Chrome extensions

See #2556

This commit makes it possible to use star origin in CORS handler setup with a Chrome extension client app.
Chrome Extensions use an origin of the follow form: chrome-extension://<extension-id>
An extension id is 32 chars long and contains chars from 'a' to 'p' (0->9 + A->F translated to a->p).

Signed-off-by: Thomas Segismont <[email protected]>

* Review updates

Signed-off-by: Thomas Segismont <[email protected]>

---------

Signed-off-by: Thomas Segismont <[email protected]>
tsegismont added a commit that referenced this issue Mar 20, 2024
@tsegismont
CORS: support Chrome extensions (#2581)
69c27c0 
* CORS: support Chrome extensions

See #2556

This commit makes it possible to use star origin in CORS handler setup with a Chrome extension client app.
Chrome Extensions use an origin of the follow form: chrome-extension://<extension-id>
An extension id is 32 chars long and contains chars from 'a' to 'p' (0->9 + A->F translated to a->p).

Signed-off-by: Thomas Segismont <[email protected]>

* Review updates

Signed-off-by: Thomas Segismont <[email protected]>

---------

Signed-off-by: Thomas Segismont <[email protected]>
@tsegismont
Copy link
Contributor

Fixed by 69c27c0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsegismont tsegismont

Labels
bug
Milestone
4.5.6
Development

No branches or pull requests
3 participants
@vietj @werdnum @tsegismont