You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Connor Penhale <CP...@perforce.com> on 2020/04/02 18:00:55 UTC
[DISCUSS] KIP-587 Suppress detailed responses for handled exceptions
in security-sensitive environments
Hello All!
I’ve created the following KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
The PR that originated this discussion, is here: https://github.com/apache/kafka/pull/8355 It is based on 2.0, but I would be working on Kafka Connect in 2.6 to get this behavior changed to the community’s preference.
Looking forward to working with everyone!
Thanks!
Connor
---
Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/)
Perforce (https://www.perforce.com/)
Support: +1 866.399.6736
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Chris,
I agree that Connect shouldn't obtusely refuse operator-helpful information on known, handled exceptions. The situation you describe here feels like a great example. In cases like this, I would suggest printing the message, and in the field that would contain the stack trace, replacing that message with the pre-defined string that says Connect is running in a mode that doesn't print stack traces. This way, the schema is not changed. In places where the message would include the stack trace along with information about the fault, omitting the stack trace or replacing it with the pre-defined string would work just fine for me.
Thanks!
Connor
On 5/13/20, 12:30 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
Hi Connor,
I think this is really close but have one more thought. Uncaught exceptions
in the REST API are different from exceptions that come about when tasks or
connectors fail, and can be used for different purposes. Stack traces in
500 errors are probably only useful for the administrator of the Connect
cluster. However, if a user has tried to create a connector and sees that
it or one of its tasks has failed, a brief message about the cause of
failure might actually be pretty helpful, and if they can't get any
information on why the connector or task failed, then they're essentially
at the mercy of the Connect cluster administrator for figuring out what
caused the failure. Would it be alright to include the exception message,
but not the entire stack trace, in the response for requests to view the
status of a connector or task?
Cheers,
Chris
On Wed, May 6, 2020 at 12:07 PM Connor Penhale <CP...@perforce.com>
wrote:
> Hi Chris,
>
> Apologies for the name confusion! I've been working with the my customer
> sponsor over the last few weeks, and we finally have an answer regarding
> "only exceptions or all responses." This organization is really interested
> in removing stack traces from all responses, which will expand the scope of
> this KIP a bit. I'm going to update the wiki entry, and then would it be
> reasonable to call for a vote?
>
> Thanks!
> Connor
>
> On 4/17/20, 3:53 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
>
> Hi Connor,
>
> That's great, but I think you may have mistaken Colin for me :)
>
> One more thing that should be addressed--the "public interfaces"
> section
> isn't just for Java interfaces, it's for any changes to any public
> part of
> Kafka that users and external developers interact with. As far as
> Connect
> is concerned, this includes (but is not limited to) the REST API and
> worker
> configuration properties, so it might be worth briefly summarizing the
> scope of your proposed changes in that section (something like "We
> plan on
> adding a new worker config named <name> that will affect the REST API
> under
> <conditions>".
>
> Cheers,
>
> Chris
>
> On Wed, Apr 15, 2020 at 1:00 PM Connor Penhale <CP...@perforce.com>
> wrote:
>
> > Hi Chris,
> >
> > I can ask the customer if they can disclose any additional
> information. I
> > provided the information around "PCI-DSS" to give the community a
> flavor of
> > the type of environment the customer was operating in. The current
> mode is
> > /not/ insecure, I would agree with this. I would be willing to agree
> that
> > my customer has particular security audit requirements that go above
> and
> > beyond what most environments would consider reasonable. Are you
> > comfortable with that language?
> >
> > " enable.rest.response.stack.traces" works great for me!
> >
> > I created a new class in the example PR because I wanted the highest
> > chance of not gunking up the works by stepping on toes in an
> important
> > class. I figured I'd be reducing risk by creating an alternative
> > implementing class. In retrospect, and now that I'm getting a
> first-hand
> > look at Kafka's community process, that is probably unnecessary.
> > Additionally, I would agree with your statement that we should
> modify the
> > existing ExceptionMapper to avoid behavior divergence in subsequent
> > releases and ensure this feature's particular scope is easy to
> maintain.
> >
> > Thanks!
> > Connor
> >
> > On 4/15/20, 1:17 PM, "Colin McCabe" <cm...@apache.org> wrote:
> >
> > Hi Connor,
> >
> > I still would like to hear more about whether this feature is
> required
> > for PCI-DSS or any other security certification. Nobody I talked to
> seemed
> > to think that it was-- if there are certifications that would
> require this,
> > it would be nice to know. However, I don't object to implementing
> this as
> > long as we don't imply that the current mode is insecure.
> >
> > What do you think about using
> "enable.rest.response.stack.traces" as
> > the config name? It seems like that makes it clearer that it's a
> boolean
> > value.
> >
> > It's not really necessary to describe the internal
> implementation in
> > the KIP, but since you mentioned it, it's probably worth considering
> using
> > the current ExceptionMapper class with a different configuration
> rather
> > than creating a new one.
> >
> > best,
> > Colin
> >
> >
> > On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> > > Hi Chris!
> > >
> > > RE: SSL, indeed, the issue is not that the information is not
> > > encrypted, but that there is no authorization layer.
> > >
> > > I'll be sure to edit the KIP as we continue discussion!
> > >
> > > RE: the 200 response you highlighted, great catch! I'll work
> with my
> > > customer and get back to you on their audit team's intention!
> I'm
> > > fairly certain I know the answer, but I need to be sure before
> I
> > speak
> > > for them.
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/8/20, 11:27 PM, "Christopher Egerton" <
> chrise@confluent.io>
> > wrote:
> > >
> > > Hi Connor,
> > >
> > > Just a few more remarks!
> > >
> > > I noticed that you said "Kafka Connect was passing these
> > exceptions without
> > > authentication." For what it's worth, the Connect REST API
> can
> > be secured
> > > with TLS out-of-the-box by configuring the worker with the
> > various ssl.*
> > > properties, but that doesn't provide any kind of
> authorization
> > layer to
> > > provide levels of security depending who the user is. Just
> > pointing out in
> > > case this helps with your use case.
> > >
> > > As far as editing the KIP based on discussion goes--it's
> not only
> > > acceptable, it's expected :) Ideally, the KIP should be
> kept
> > up-to-date to
> > > the point where, were it to be accepted at any moment, it
> would
> > accurately
> > > reflect the changes that would then be made to Kafka. This
> can
> > be relaxed
> > > if there's rapid iteration or items that are still up for
> > discussion, but
> > > as soon as things settle down it should be updated.
> > >
> > > As far as item 4 goes, my question was about exceptions
> that
> > aren't handled
> > > by the ExceptionMapper, but which are returned as part of
> the
> > response body
> > > when querying the status of a connector or task that has
> failed
> > by querying
> > > the /connectors/{name}/status or
> > /connectors/{name}/tasks/{taskId}/status
> > > endpoints. Even if the request is successful and results
> in an
> > HTTP 200
> > > response, the body might contain a stack trace if the
> connector
> > or any of
> > > its tasks have failed.
> > >
> > > For example, I ran an instance of the FileStreamSource
> connector
> > named
> > > "file-source" locally and instructed it to consume from a
> file
> > that it
> > > lacked permissions to read. When I queried the status of
> that
> > connector by
> > > issuing a request to /connectors/file-source/status, I got
> back
> > the
> > > following response:
> > >
> > > {
> > > "name": "file-source",
> > > "connector": {
> > > "state": "RUNNING",
> > > "worker_id": "192.168.86.21:8083"
> > > },
> > > "tasks": [
> > > {
> > > "id": 0,
> > > "state": "FAILED",
> > > "worker_id": "192.168.86.21:8083",
> > > "trace":
> "org.apache.kafka.connect.errors.ConnectException:
> > > java.nio.file.AccessDeniedException: test.txt\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
> > >
> > >
> >
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> > >
> java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> > > java.lang.Thread.run(Thread.java:748)\nCaused by:
> > > java.nio.file.AccessDeniedException: test.txt\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> > > java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> > > java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
> > >
> > >
> >
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> > > java.nio.file.Files.newInputStream(Files.java:152)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> > > 9 more\n"
> > > }
> > > ],
> > > "type": "source"
> > > }
> > >
> > > Note the "trace" field in the first element of the "tasks"
> field
> > of the
> > > response: this was the stack trace for the exception that
> caused
> > the task
> > > to fail during execution, which has nothing to do with the
> > success or
> > > failure of the REST request I issued to the
> > /connectors/file-source/status
> > > endpoint.
> > >
> > > I was wondering if you wanted to include these kinds of
> stack
> > traces as
> > > part of the KIP, as opposed to uncaught exceptions that
> result
> > in a 500
> > > error from the REST API.
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <
> > CPenhale@perforce.com> wrote:
> > >
> > > > Hi All!
> > > >
> > > > Is there any additional feedback that the community can
> provide
> > > me on the
> > > > KIP? Has anyone else run into requirements like this, or
> maybe
> > my
> > > customer
> > > > is the only one :)? If the scope looks good, is it time
> to
> > call a
> > > vote? Or
> > > > should I start porting my 2.0 code to 2.6 to show
> examples?
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > On 4/6/20, 9:03 AM, "Connor Penhale" <
> CPenhale@perforce.com>
> > > wrote:
> > > >
> > > > Hi Colin,
> > > >
> > > > We did not find a specific security vulnerability.
> Our
> > > customer had
> > > > auditors in their environment, and they identified Kafka
> > Connect
> > > as out of
> > > > compliance with their particular standards, something
> that
> > > happens all the
> > > > time for REST-based applications. What these security
> auditors
> > > expected
> > > > Kafka Connect to be able to do was tune the response. As
> Kafka
> > > Connect
> > > > could not provide this functionality, I'm proposing this
> KIP.
> > > Does that
> > > > make sense? Should I still go through the process of a
> security
> > > disclosure?
> > > >
> > > > Our particular need was around suppressing
> exceptions in
> > the
> > > "public"
> > > > response, as Kafka Connect was passing these exceptions
> without
> > > > authentication, they became a public endpoint upon which
> the
> > > auditors could
> > > > fuzz, and show it being out of compliance. Keeping these
> > > exceptions in the
> > > > logs, as proposed in the KIP, makes sense to me as an
> operator.
> > > >
> > > > I only mention PCI-DSS as this was the kind of
> environment
> > my
> > > customer
> > > > had that was making the request for being able to tune
> the
> > > response.
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > ---
> > > > Connor Penhale | Enterprise Architect, OpenLogic (
> > > > https://openlogic.com/)
> > > > Perforce (https://www.perforce.com/)
> > > > Support: +1 866.399.6736
> > > >
> > > >
> > > > On 4/3/20, 3:24 PM, "Colin McCabe" <
> cmccabe@apache.org>
> > wrote:
> > > >
> > > > Also, if you do find a security issue, the
> process to
> > > follow is
> > > > here: https://kafka.apache.org/project-security.html .
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Apr 3, 2020, at 14:20, Colin McCabe
> wrote:
> > > > > Hi Connor,
> > > > >
> > > > > If we are putting security-sensitive
> information into
> > > REST
> > > > responses,
> > > > > that is a bug that needs to be fixed, not
> worked
> > around
> > > with a
> > > > > configuration option. Do you have an example
> of
> > > > security-sensitive
> > > > > information appearing in the exception text?
> Why do
> > > you feel
> > > > that
> > > > > PCI-DSS requires this change?
> > > > >
> > > > > By the way, the same concern applies to log
> messages.
> > > We do not
> > > > log
> > > > > sensitive information such as passwords to the
> log4j
> > > output. If
> > > > you
> > > > > know of that happening somewhere, please file
> a bug
> > so
> > > it can be
> > > > fixed.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale
> wrote:
> > > > > > Hi Chris!
> > > > > >
> > > > > > Thanks for your feedback! I'll number my
> responses
> > to
> > > your
> > > > questions / thoughts.
> > > > > >
> > > > > > 1. Apologies on that lack of clarity! I
> settled on
> > > "Detailed
> > > > exception
> > > > > > information has been suppressed. Please see
> logs."
> > > > > > (
> > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34
> > ).
> > > > Should I update the KIP to reflect what I've already
> thought
> > > about? It's my
> > > > first one, not sure what the process should be for
> editing.
> > > > > >
> > > > > > 2. I was unaware of the REST extensions!
> I'll see
> > if
> > > I can
> > > > implement
> > > > > > the same behavior as a REST extension. I
> agree that
> > > the KIP
> > > > still has
> > > > > > merit, regardless of the feasibility of the
> > > extension, but in
> > > > regards
> > > > > > to the 5th thought, this might make that
> decision
> > > easier.
> > > > > >
> > > > > > 3. I agree with your suggestion here.
> Absolutely
> > > ready to take
> > > > the
> > > > > > community feedback on what makes sense here.
> > > > > >
> > > > > > 4. I should note that while I emphasized
> uncaught
> > > exceptions,
> > > > I mean
> > > > > > all exceptions handled by the
> ExceptionMapper,
> > > including
> > > > > > ConnectRestExceptions. An example of this is
> here:
> > > > > >
> > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > > > >
> > > > > > 5. I didn't know how specific I should get
> if I had
> > > already
> > > > taken a
> > > > > > stab at implementing! I'm happy to edit this
> in
> > > whatever way
> > > > we want to
> > > > > > go about it.
> > > > > >
> > > > > > Let me know if anyone has any other
> questions or
> > > feedback!
> > > > > >
> > > > > >
> > > > > > Thanks!
> > > > > > Connor
> > > > > >
> > > > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> > > <ch...@confluent.io>
> > > > wrote:
> > > > > >
> > > > > > Hi Connor,
> > > > > >
> > > > > > Great stuff! I generally like being able
> to see
> > > the stack
> > > > trace of an
> > > > > > exception directly via the REST API but
> can
> > > definitely
> > > > understand the
> > > > > > security concerns here. I've got a few
> > > questions/remarks
> > > > about the KIP and
> > > > > > would be interested in your thoughts:
> > > > > >
> > > > > > 1. The KIP mentions a
> > > SUPRESSED_EXCEPTION_MESSAGE, but
> > > > doesn't actually
> > > > > > outline what this message would actually
> be.
> > It'd
> > > be great
> > > > to see the
> > > > > > actual message in the KIP since people
> may have
> > > thoughts
> > > > on what it should
> > > > > > be and want to comment on it as part of
> this
> > > discussion.
> > > > > >
> > > > > > 2. In the "Rejected Alternatives"
> section, an
> > > Nginx proxy
> > > > is
> > > > > > mentioned as
> > > > > > one possible way to filter out stack
> traces
> > from
> > > the REST
> > > > API. It
> > > > > > seems
> > > > > > like a Connect REST extension (
> > > > > >
> > > > > >
> > > >
> > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > > > )
> > > > > > would be a better alternative than an
> Nginx
> > > proxy; had you
> > > > > > considered
> > > > > > utilizing one? I still think this KIP is
> > > worthwhile and a
> > > > REST
> > > > > > extension
> > > > > > shouldn't be necessary in order to lock
> down
> > the
> > > REST API
> > > > this way,
> > > > > > but it
> > > > > > might be worth calling out as an
> alternative
> > and
> > > perhaps
> > > > even a
> > > > > > workaround
> > > > > > in cases where users are stuck on a given
> > version
> > > of
> > > > Connect and
> > > > > > can't
> > > > > > upgrade to 2.6 (or whichever version
> this KIP
> > > lands on)
> > > > any time
> > > > > > soon.
> > > > > >
> > > > > > 3. The
> > > "error.rest.response.message.detail.enabled"
> > > > property is a bit of a
> > > > > > mouthful; it'd be great if we could come
> up
> > with
> > > something
> > > > more succinct.
> > > > > > What do you think about something like
> > > > "rest.response.stack.traces"?
> > > > > >
> > > > > > 4. The KIP is targeted at stack traces
> for
> > > uncaught
> > > > exceptions, but it's
> > > > > > also possible that stack traces get
> exposed in
> > > the REST
> > > > API when querying
> > > > > > the status of a connector or one of its
> tasks.
> > > Was this
> > > > intentional? If so,
> > > > > > it'd be great to call out why that kind
> of
> > > filtering is
> > > > not required in the
> > > > > > "Rejected Alternatives" section, and if
> not,
> > it's
> > > probably
> > > > not too late to
> > > > > > consider modifying the KIP to cover
> those cases
> > > as well.
> > > > > >
> > > > > > 5. The KIP mentions creating a new,
> separate
> > > exception
> > > > mapper class. This
> > > > > > seems like more of an implementation
> detail and
> > > something
> > > > that can be
> > > > > > decided on during code review; unless
> it's
> > > critical to the
> > > > functionality
> > > > > > that the KIP aims to accomplish, I'd
> suggest
> > > leaving that
> > > > part out since it
> > > > > > shouldn't affect the impact on users of
> the
> > > Connect
> > > > framework.
> > > > > >
> > > > > > Thanks for the KIP, looking forward to
> seeing
> > > this happen!
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Chris
> > > > > >
> > > > > > On Thu, Apr 2, 2020 at 11:01 AM Connor
> Penhale
> > <
> > > > CPenhale@perforce.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hello All!
> > > > > > >
> > > > > > > I’ve created the following KIP:
> > > > > > >
> > > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > > > >
> > > > > > > The PR that originated this
> discussion, is
> > here:
> > > > > > >
> https://github.com/apache/kafka/pull/8355
> > It
> > > is based
> > > > on 2.0,
> > > > > > but I
> > > > > > > would be working on Kafka Connect in
> 2.6 to
> > get
> > > this
> > > > behavior
> > > > > > changed to
> > > > > > > the community’s preference.
> > > > > > >
> > > > > > > Looking forward to working with
> everyone!
> > > > > > >
> > > > > > > Thanks!
> > > > > > > Connor
> > > > > > > ---
> > > > > > > Connor Penhale | Enterprise Architect,
> > OpenLogic
> > > > > > (https://openlogic.com/)
> > > > > > > Perforce (https://www.perforce.com/)
> > > > > > > Support: +1 866.399.6736
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > This e-mail may contain information
> that is
> > > privileged or
> > > > > > confidential. If
> > > > > > > you are not the intended recipient,
> please
> > > delete the
> > > > e-mail and
> > > > > > any
> > > > > > > attachments and notify us immediately.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > CAUTION: This email originated from
> outside of
> > the
> > > > organization. Do
> > > > > > not click on links or open attachments
> unless you
> > > recognize
> > > > the sender
> > > > > > and know the content is safe.
> > > > > >
> > > > > >
> > > > > >
> > > > > > This e-mail may contain information that is
> > > privileged or
> > > > confidential.
> > > > > > If you are not the intended recipient, please
> > delete
> > > the
> > > > e-mail and any
> > > > > > attachments and notify us immediately.
> > > > > >
> > > > > >
> > > >
> > > >
> > > > CAUTION: This email originated from outside of
> the
> > > organization.
> > > > Do not click on links or open attachments unless you
> recognize
> > > the sender
> > > > and know the content is safe.
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is
> privileged or
> > > > confidential. If you are not the intended recipient,
> please
> > > delete the
> > > > e-mail and any attachments and notify us immediately.
> > > >
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is privileged or
> > > confidential. If
> > > > you are not the intended recipient, please delete the
> e-mail
> > and
> > > any
> > > > attachments and notify us immediately.
> > > >
> > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> organization.
> > Do
> > > not click on links or open attachments unless you recognize the
> > sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential.
> > > If you are not the intended recipient, please delete the
> e-mail and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization.
> Do
> > not click on links or open attachments unless you recognize the
> sender and
> > know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential. If
> > you are not the intended recipient, please delete the e-mail and any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender and
> know the content is safe.
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Connor,
I think this is really close but have one more thought. Uncaught exceptions
in the REST API are different from exceptions that come about when tasks or
connectors fail, and can be used for different purposes. Stack traces in
500 errors are probably only useful for the administrator of the Connect
cluster. However, if a user has tried to create a connector and sees that
it or one of its tasks has failed, a brief message about the cause of
failure might actually be pretty helpful, and if they can't get any
information on why the connector or task failed, then they're essentially
at the mercy of the Connect cluster administrator for figuring out what
caused the failure. Would it be alright to include the exception message,
but not the entire stack trace, in the response for requests to view the
status of a connector or task?
Cheers,
Chris
On Wed, May 6, 2020 at 12:07 PM Connor Penhale <CP...@perforce.com>
wrote:
> Hi Chris,
>
> Apologies for the name confusion! I've been working with the my customer
> sponsor over the last few weeks, and we finally have an answer regarding
> "only exceptions or all responses." This organization is really interested
> in removing stack traces from all responses, which will expand the scope of
> this KIP a bit. I'm going to update the wiki entry, and then would it be
> reasonable to call for a vote?
>
> Thanks!
> Connor
>
> On 4/17/20, 3:53 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
>
> Hi Connor,
>
> That's great, but I think you may have mistaken Colin for me :)
>
> One more thing that should be addressed--the "public interfaces"
> section
> isn't just for Java interfaces, it's for any changes to any public
> part of
> Kafka that users and external developers interact with. As far as
> Connect
> is concerned, this includes (but is not limited to) the REST API and
> worker
> configuration properties, so it might be worth briefly summarizing the
> scope of your proposed changes in that section (something like "We
> plan on
> adding a new worker config named <name> that will affect the REST API
> under
> <conditions>".
>
> Cheers,
>
> Chris
>
> On Wed, Apr 15, 2020 at 1:00 PM Connor Penhale <CP...@perforce.com>
> wrote:
>
> > Hi Chris,
> >
> > I can ask the customer if they can disclose any additional
> information. I
> > provided the information around "PCI-DSS" to give the community a
> flavor of
> > the type of environment the customer was operating in. The current
> mode is
> > /not/ insecure, I would agree with this. I would be willing to agree
> that
> > my customer has particular security audit requirements that go above
> and
> > beyond what most environments would consider reasonable. Are you
> > comfortable with that language?
> >
> > " enable.rest.response.stack.traces" works great for me!
> >
> > I created a new class in the example PR because I wanted the highest
> > chance of not gunking up the works by stepping on toes in an
> important
> > class. I figured I'd be reducing risk by creating an alternative
> > implementing class. In retrospect, and now that I'm getting a
> first-hand
> > look at Kafka's community process, that is probably unnecessary.
> > Additionally, I would agree with your statement that we should
> modify the
> > existing ExceptionMapper to avoid behavior divergence in subsequent
> > releases and ensure this feature's particular scope is easy to
> maintain.
> >
> > Thanks!
> > Connor
> >
> > On 4/15/20, 1:17 PM, "Colin McCabe" <cm...@apache.org> wrote:
> >
> > Hi Connor,
> >
> > I still would like to hear more about whether this feature is
> required
> > for PCI-DSS or any other security certification. Nobody I talked to
> seemed
> > to think that it was-- if there are certifications that would
> require this,
> > it would be nice to know. However, I don't object to implementing
> this as
> > long as we don't imply that the current mode is insecure.
> >
> > What do you think about using
> "enable.rest.response.stack.traces" as
> > the config name? It seems like that makes it clearer that it's a
> boolean
> > value.
> >
> > It's not really necessary to describe the internal
> implementation in
> > the KIP, but since you mentioned it, it's probably worth considering
> using
> > the current ExceptionMapper class with a different configuration
> rather
> > than creating a new one.
> >
> > best,
> > Colin
> >
> >
> > On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> > > Hi Chris!
> > >
> > > RE: SSL, indeed, the issue is not that the information is not
> > > encrypted, but that there is no authorization layer.
> > >
> > > I'll be sure to edit the KIP as we continue discussion!
> > >
> > > RE: the 200 response you highlighted, great catch! I'll work
> with my
> > > customer and get back to you on their audit team's intention!
> I'm
> > > fairly certain I know the answer, but I need to be sure before
> I
> > speak
> > > for them.
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/8/20, 11:27 PM, "Christopher Egerton" <
> chrise@confluent.io>
> > wrote:
> > >
> > > Hi Connor,
> > >
> > > Just a few more remarks!
> > >
> > > I noticed that you said "Kafka Connect was passing these
> > exceptions without
> > > authentication." For what it's worth, the Connect REST API
> can
> > be secured
> > > with TLS out-of-the-box by configuring the worker with the
> > various ssl.*
> > > properties, but that doesn't provide any kind of
> authorization
> > layer to
> > > provide levels of security depending who the user is. Just
> > pointing out in
> > > case this helps with your use case.
> > >
> > > As far as editing the KIP based on discussion goes--it's
> not only
> > > acceptable, it's expected :) Ideally, the KIP should be
> kept
> > up-to-date to
> > > the point where, were it to be accepted at any moment, it
> would
> > accurately
> > > reflect the changes that would then be made to Kafka. This
> can
> > be relaxed
> > > if there's rapid iteration or items that are still up for
> > discussion, but
> > > as soon as things settle down it should be updated.
> > >
> > > As far as item 4 goes, my question was about exceptions
> that
> > aren't handled
> > > by the ExceptionMapper, but which are returned as part of
> the
> > response body
> > > when querying the status of a connector or task that has
> failed
> > by querying
> > > the /connectors/{name}/status or
> > /connectors/{name}/tasks/{taskId}/status
> > > endpoints. Even if the request is successful and results
> in an
> > HTTP 200
> > > response, the body might contain a stack trace if the
> connector
> > or any of
> > > its tasks have failed.
> > >
> > > For example, I ran an instance of the FileStreamSource
> connector
> > named
> > > "file-source" locally and instructed it to consume from a
> file
> > that it
> > > lacked permissions to read. When I queried the status of
> that
> > connector by
> > > issuing a request to /connectors/file-source/status, I got
> back
> > the
> > > following response:
> > >
> > > {
> > > "name": "file-source",
> > > "connector": {
> > > "state": "RUNNING",
> > > "worker_id": "192.168.86.21:8083"
> > > },
> > > "tasks": [
> > > {
> > > "id": 0,
> > > "state": "FAILED",
> > > "worker_id": "192.168.86.21:8083",
> > > "trace":
> "org.apache.kafka.connect.errors.ConnectException:
> > > java.nio.file.AccessDeniedException: test.txt\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
> > >
> > >
> >
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> > >
> java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
> > >
> > >
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> > > java.lang.Thread.run(Thread.java:748)\nCaused by:
> > > java.nio.file.AccessDeniedException: test.txt\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
> > >
> > >
> >
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> > > java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> > > java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
> > >
> > >
> >
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> > > java.nio.file.Files.newInputStream(Files.java:152)\n\tat
> > >
> > >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> > > 9 more\n"
> > > }
> > > ],
> > > "type": "source"
> > > }
> > >
> > > Note the "trace" field in the first element of the "tasks"
> field
> > of the
> > > response: this was the stack trace for the exception that
> caused
> > the task
> > > to fail during execution, which has nothing to do with the
> > success or
> > > failure of the REST request I issued to the
> > /connectors/file-source/status
> > > endpoint.
> > >
> > > I was wondering if you wanted to include these kinds of
> stack
> > traces as
> > > part of the KIP, as opposed to uncaught exceptions that
> result
> > in a 500
> > > error from the REST API.
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <
> > CPenhale@perforce.com> wrote:
> > >
> > > > Hi All!
> > > >
> > > > Is there any additional feedback that the community can
> provide
> > > me on the
> > > > KIP? Has anyone else run into requirements like this, or
> maybe
> > my
> > > customer
> > > > is the only one :)? If the scope looks good, is it time
> to
> > call a
> > > vote? Or
> > > > should I start porting my 2.0 code to 2.6 to show
> examples?
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > On 4/6/20, 9:03 AM, "Connor Penhale" <
> CPenhale@perforce.com>
> > > wrote:
> > > >
> > > > Hi Colin,
> > > >
> > > > We did not find a specific security vulnerability.
> Our
> > > customer had
> > > > auditors in their environment, and they identified Kafka
> > Connect
> > > as out of
> > > > compliance with their particular standards, something
> that
> > > happens all the
> > > > time for REST-based applications. What these security
> auditors
> > > expected
> > > > Kafka Connect to be able to do was tune the response. As
> Kafka
> > > Connect
> > > > could not provide this functionality, I'm proposing this
> KIP.
> > > Does that
> > > > make sense? Should I still go through the process of a
> security
> > > disclosure?
> > > >
> > > > Our particular need was around suppressing
> exceptions in
> > the
> > > "public"
> > > > response, as Kafka Connect was passing these exceptions
> without
> > > > authentication, they became a public endpoint upon which
> the
> > > auditors could
> > > > fuzz, and show it being out of compliance. Keeping these
> > > exceptions in the
> > > > logs, as proposed in the KIP, makes sense to me as an
> operator.
> > > >
> > > > I only mention PCI-DSS as this was the kind of
> environment
> > my
> > > customer
> > > > had that was making the request for being able to tune
> the
> > > response.
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > ---
> > > > Connor Penhale | Enterprise Architect, OpenLogic (
> > > > https://openlogic.com/)
> > > > Perforce (https://www.perforce.com/)
> > > > Support: +1 866.399.6736
> > > >
> > > >
> > > > On 4/3/20, 3:24 PM, "Colin McCabe" <
> cmccabe@apache.org>
> > wrote:
> > > >
> > > > Also, if you do find a security issue, the
> process to
> > > follow is
> > > > here: https://kafka.apache.org/project-security.html .
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Apr 3, 2020, at 14:20, Colin McCabe
> wrote:
> > > > > Hi Connor,
> > > > >
> > > > > If we are putting security-sensitive
> information into
> > > REST
> > > > responses,
> > > > > that is a bug that needs to be fixed, not
> worked
> > around
> > > with a
> > > > > configuration option. Do you have an example
> of
> > > > security-sensitive
> > > > > information appearing in the exception text?
> Why do
> > > you feel
> > > > that
> > > > > PCI-DSS requires this change?
> > > > >
> > > > > By the way, the same concern applies to log
> messages.
> > > We do not
> > > > log
> > > > > sensitive information such as passwords to the
> log4j
> > > output. If
> > > > you
> > > > > know of that happening somewhere, please file
> a bug
> > so
> > > it can be
> > > > fixed.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale
> wrote:
> > > > > > Hi Chris!
> > > > > >
> > > > > > Thanks for your feedback! I'll number my
> responses
> > to
> > > your
> > > > questions / thoughts.
> > > > > >
> > > > > > 1. Apologies on that lack of clarity! I
> settled on
> > > "Detailed
> > > > exception
> > > > > > information has been suppressed. Please see
> logs."
> > > > > > (
> > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34
> > ).
> > > > Should I update the KIP to reflect what I've already
> thought
> > > about? It's my
> > > > first one, not sure what the process should be for
> editing.
> > > > > >
> > > > > > 2. I was unaware of the REST extensions!
> I'll see
> > if
> > > I can
> > > > implement
> > > > > > the same behavior as a REST extension. I
> agree that
> > > the KIP
> > > > still has
> > > > > > merit, regardless of the feasibility of the
> > > extension, but in
> > > > regards
> > > > > > to the 5th thought, this might make that
> decision
> > > easier.
> > > > > >
> > > > > > 3. I agree with your suggestion here.
> Absolutely
> > > ready to take
> > > > the
> > > > > > community feedback on what makes sense here.
> > > > > >
> > > > > > 4. I should note that while I emphasized
> uncaught
> > > exceptions,
> > > > I mean
> > > > > > all exceptions handled by the
> ExceptionMapper,
> > > including
> > > > > > ConnectRestExceptions. An example of this is
> here:
> > > > > >
> > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > > > >
> > > > > > 5. I didn't know how specific I should get
> if I had
> > > already
> > > > taken a
> > > > > > stab at implementing! I'm happy to edit this
> in
> > > whatever way
> > > > we want to
> > > > > > go about it.
> > > > > >
> > > > > > Let me know if anyone has any other
> questions or
> > > feedback!
> > > > > >
> > > > > >
> > > > > > Thanks!
> > > > > > Connor
> > > > > >
> > > > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> > > <ch...@confluent.io>
> > > > wrote:
> > > > > >
> > > > > > Hi Connor,
> > > > > >
> > > > > > Great stuff! I generally like being able
> to see
> > > the stack
> > > > trace of an
> > > > > > exception directly via the REST API but
> can
> > > definitely
> > > > understand the
> > > > > > security concerns here. I've got a few
> > > questions/remarks
> > > > about the KIP and
> > > > > > would be interested in your thoughts:
> > > > > >
> > > > > > 1. The KIP mentions a
> > > SUPRESSED_EXCEPTION_MESSAGE, but
> > > > doesn't actually
> > > > > > outline what this message would actually
> be.
> > It'd
> > > be great
> > > > to see the
> > > > > > actual message in the KIP since people
> may have
> > > thoughts
> > > > on what it should
> > > > > > be and want to comment on it as part of
> this
> > > discussion.
> > > > > >
> > > > > > 2. In the "Rejected Alternatives"
> section, an
> > > Nginx proxy
> > > > is
> > > > > > mentioned as
> > > > > > one possible way to filter out stack
> traces
> > from
> > > the REST
> > > > API. It
> > > > > > seems
> > > > > > like a Connect REST extension (
> > > > > >
> > > > > >
> > > >
> > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > > > )
> > > > > > would be a better alternative than an
> Nginx
> > > proxy; had you
> > > > > > considered
> > > > > > utilizing one? I still think this KIP is
> > > worthwhile and a
> > > > REST
> > > > > > extension
> > > > > > shouldn't be necessary in order to lock
> down
> > the
> > > REST API
> > > > this way,
> > > > > > but it
> > > > > > might be worth calling out as an
> alternative
> > and
> > > perhaps
> > > > even a
> > > > > > workaround
> > > > > > in cases where users are stuck on a given
> > version
> > > of
> > > > Connect and
> > > > > > can't
> > > > > > upgrade to 2.6 (or whichever version
> this KIP
> > > lands on)
> > > > any time
> > > > > > soon.
> > > > > >
> > > > > > 3. The
> > > "error.rest.response.message.detail.enabled"
> > > > property is a bit of a
> > > > > > mouthful; it'd be great if we could come
> up
> > with
> > > something
> > > > more succinct.
> > > > > > What do you think about something like
> > > > "rest.response.stack.traces"?
> > > > > >
> > > > > > 4. The KIP is targeted at stack traces
> for
> > > uncaught
> > > > exceptions, but it's
> > > > > > also possible that stack traces get
> exposed in
> > > the REST
> > > > API when querying
> > > > > > the status of a connector or one of its
> tasks.
> > > Was this
> > > > intentional? If so,
> > > > > > it'd be great to call out why that kind
> of
> > > filtering is
> > > > not required in the
> > > > > > "Rejected Alternatives" section, and if
> not,
> > it's
> > > probably
> > > > not too late to
> > > > > > consider modifying the KIP to cover
> those cases
> > > as well.
> > > > > >
> > > > > > 5. The KIP mentions creating a new,
> separate
> > > exception
> > > > mapper class. This
> > > > > > seems like more of an implementation
> detail and
> > > something
> > > > that can be
> > > > > > decided on during code review; unless
> it's
> > > critical to the
> > > > functionality
> > > > > > that the KIP aims to accomplish, I'd
> suggest
> > > leaving that
> > > > part out since it
> > > > > > shouldn't affect the impact on users of
> the
> > > Connect
> > > > framework.
> > > > > >
> > > > > > Thanks for the KIP, looking forward to
> seeing
> > > this happen!
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Chris
> > > > > >
> > > > > > On Thu, Apr 2, 2020 at 11:01 AM Connor
> Penhale
> > <
> > > > CPenhale@perforce.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hello All!
> > > > > > >
> > > > > > > I’ve created the following KIP:
> > > > > > >
> > > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > > > >
> > > > > > > The PR that originated this
> discussion, is
> > here:
> > > > > > >
> https://github.com/apache/kafka/pull/8355
> > It
> > > is based
> > > > on 2.0,
> > > > > > but I
> > > > > > > would be working on Kafka Connect in
> 2.6 to
> > get
> > > this
> > > > behavior
> > > > > > changed to
> > > > > > > the community’s preference.
> > > > > > >
> > > > > > > Looking forward to working with
> everyone!
> > > > > > >
> > > > > > > Thanks!
> > > > > > > Connor
> > > > > > > ---
> > > > > > > Connor Penhale | Enterprise Architect,
> > OpenLogic
> > > > > > (https://openlogic.com/)
> > > > > > > Perforce (https://www.perforce.com/)
> > > > > > > Support: +1 866.399.6736
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > This e-mail may contain information
> that is
> > > privileged or
> > > > > > confidential. If
> > > > > > > you are not the intended recipient,
> please
> > > delete the
> > > > e-mail and
> > > > > > any
> > > > > > > attachments and notify us immediately.
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > CAUTION: This email originated from
> outside of
> > the
> > > > organization. Do
> > > > > > not click on links or open attachments
> unless you
> > > recognize
> > > > the sender
> > > > > > and know the content is safe.
> > > > > >
> > > > > >
> > > > > >
> > > > > > This e-mail may contain information that is
> > > privileged or
> > > > confidential.
> > > > > > If you are not the intended recipient, please
> > delete
> > > the
> > > > e-mail and any
> > > > > > attachments and notify us immediately.
> > > > > >
> > > > > >
> > > >
> > > >
> > > > CAUTION: This email originated from outside of
> the
> > > organization.
> > > > Do not click on links or open attachments unless you
> recognize
> > > the sender
> > > > and know the content is safe.
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is
> privileged or
> > > > confidential. If you are not the intended recipient,
> please
> > > delete the
> > > > e-mail and any attachments and notify us immediately.
> > > >
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is privileged or
> > > confidential. If
> > > > you are not the intended recipient, please delete the
> e-mail
> > and
> > > any
> > > > attachments and notify us immediately.
> > > >
> > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> organization.
> > Do
> > > not click on links or open attachments unless you recognize the
> > sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential.
> > > If you are not the intended recipient, please delete the
> e-mail and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization.
> Do
> > not click on links or open attachments unless you recognize the
> sender and
> > know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential. If
> > you are not the intended recipient, please delete the e-mail and any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender and
> know the content is safe.
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Chris,
Apologies for the name confusion! I've been working with the my customer sponsor over the last few weeks, and we finally have an answer regarding "only exceptions or all responses." This organization is really interested in removing stack traces from all responses, which will expand the scope of this KIP a bit. I'm going to update the wiki entry, and then would it be reasonable to call for a vote?
Thanks!
Connor
On 4/17/20, 3:53 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
Hi Connor,
That's great, but I think you may have mistaken Colin for me :)
One more thing that should be addressed--the "public interfaces" section
isn't just for Java interfaces, it's for any changes to any public part of
Kafka that users and external developers interact with. As far as Connect
is concerned, this includes (but is not limited to) the REST API and worker
configuration properties, so it might be worth briefly summarizing the
scope of your proposed changes in that section (something like "We plan on
adding a new worker config named <name> that will affect the REST API under
<conditions>".
Cheers,
Chris
On Wed, Apr 15, 2020 at 1:00 PM Connor Penhale <CP...@perforce.com>
wrote:
> Hi Chris,
>
> I can ask the customer if they can disclose any additional information. I
> provided the information around "PCI-DSS" to give the community a flavor of
> the type of environment the customer was operating in. The current mode is
> /not/ insecure, I would agree with this. I would be willing to agree that
> my customer has particular security audit requirements that go above and
> beyond what most environments would consider reasonable. Are you
> comfortable with that language?
>
> " enable.rest.response.stack.traces" works great for me!
>
> I created a new class in the example PR because I wanted the highest
> chance of not gunking up the works by stepping on toes in an important
> class. I figured I'd be reducing risk by creating an alternative
> implementing class. In retrospect, and now that I'm getting a first-hand
> look at Kafka's community process, that is probably unnecessary.
> Additionally, I would agree with your statement that we should modify the
> existing ExceptionMapper to avoid behavior divergence in subsequent
> releases and ensure this feature's particular scope is easy to maintain.
>
> Thanks!
> Connor
>
> On 4/15/20, 1:17 PM, "Colin McCabe" <cm...@apache.org> wrote:
>
> Hi Connor,
>
> I still would like to hear more about whether this feature is required
> for PCI-DSS or any other security certification. Nobody I talked to seemed
> to think that it was-- if there are certifications that would require this,
> it would be nice to know. However, I don't object to implementing this as
> long as we don't imply that the current mode is insecure.
>
> What do you think about using "enable.rest.response.stack.traces" as
> the config name? It seems like that makes it clearer that it's a boolean
> value.
>
> It's not really necessary to describe the internal implementation in
> the KIP, but since you mentioned it, it's probably worth considering using
> the current ExceptionMapper class with a different configuration rather
> than creating a new one.
>
> best,
> Colin
>
>
> On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> > Hi Chris!
> >
> > RE: SSL, indeed, the issue is not that the information is not
> > encrypted, but that there is no authorization layer.
> >
> > I'll be sure to edit the KIP as we continue discussion!
> >
> > RE: the 200 response you highlighted, great catch! I'll work with my
> > customer and get back to you on their audit team's intention! I'm
> > fairly certain I know the answer, but I need to be sure before I
> speak
> > for them.
> >
> > Thanks!
> > Connor
> >
> > On 4/8/20, 11:27 PM, "Christopher Egerton" <ch...@confluent.io>
> wrote:
> >
> > Hi Connor,
> >
> > Just a few more remarks!
> >
> > I noticed that you said "Kafka Connect was passing these
> exceptions without
> > authentication." For what it's worth, the Connect REST API can
> be secured
> > with TLS out-of-the-box by configuring the worker with the
> various ssl.*
> > properties, but that doesn't provide any kind of authorization
> layer to
> > provide levels of security depending who the user is. Just
> pointing out in
> > case this helps with your use case.
> >
> > As far as editing the KIP based on discussion goes--it's not only
> > acceptable, it's expected :) Ideally, the KIP should be kept
> up-to-date to
> > the point where, were it to be accepted at any moment, it would
> accurately
> > reflect the changes that would then be made to Kafka. This can
> be relaxed
> > if there's rapid iteration or items that are still up for
> discussion, but
> > as soon as things settle down it should be updated.
> >
> > As far as item 4 goes, my question was about exceptions that
> aren't handled
> > by the ExceptionMapper, but which are returned as part of the
> response body
> > when querying the status of a connector or task that has failed
> by querying
> > the /connectors/{name}/status or
> /connectors/{name}/tasks/{taskId}/status
> > endpoints. Even if the request is successful and results in an
> HTTP 200
> > response, the body might contain a stack trace if the connector
> or any of
> > its tasks have failed.
> >
> > For example, I ran an instance of the FileStreamSource connector
> named
> > "file-source" locally and instructed it to consume from a file
> that it
> > lacked permissions to read. When I queried the status of that
> connector by
> > issuing a request to /connectors/file-source/status, I got back
> the
> > following response:
> >
> > {
> > "name": "file-source",
> > "connector": {
> > "state": "RUNNING",
> > "worker_id": "192.168.86.21:8083"
> > },
> > "tasks": [
> > {
> > "id": 0,
> > "state": "FAILED",
> > "worker_id": "192.168.86.21:8083",
> > "trace": "org.apache.kafka.connect.errors.ConnectException:
> > java.nio.file.AccessDeniedException: test.txt\n\tat
> >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
> >
> >
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> > java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
> >
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
> >
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> > java.lang.Thread.run(Thread.java:748)\nCaused by:
> > java.nio.file.AccessDeniedException: test.txt\n\tat
> >
> >
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
> >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
> >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
> >
> >
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> > java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> > java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
> >
> >
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> > java.nio.file.Files.newInputStream(Files.java:152)\n\tat
> >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> > 9 more\n"
> > }
> > ],
> > "type": "source"
> > }
> >
> > Note the "trace" field in the first element of the "tasks" field
> of the
> > response: this was the stack trace for the exception that caused
> the task
> > to fail during execution, which has nothing to do with the
> success or
> > failure of the REST request I issued to the
> /connectors/file-source/status
> > endpoint.
> >
> > I was wondering if you wanted to include these kinds of stack
> traces as
> > part of the KIP, as opposed to uncaught exceptions that result
> in a 500
> > error from the REST API.
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <
> CPenhale@perforce.com> wrote:
> >
> > > Hi All!
> > >
> > > Is there any additional feedback that the community can provide
> > me on the
> > > KIP? Has anyone else run into requirements like this, or maybe
> my
> > customer
> > > is the only one :)? If the scope looks good, is it time to
> call a
> > vote? Or
> > > should I start porting my 2.0 code to 2.6 to show examples?
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com>
> > wrote:
> > >
> > > Hi Colin,
> > >
> > > We did not find a specific security vulnerability. Our
> > customer had
> > > auditors in their environment, and they identified Kafka
> Connect
> > as out of
> > > compliance with their particular standards, something that
> > happens all the
> > > time for REST-based applications. What these security auditors
> > expected
> > > Kafka Connect to be able to do was tune the response. As Kafka
> > Connect
> > > could not provide this functionality, I'm proposing this KIP.
> > Does that
> > > make sense? Should I still go through the process of a security
> > disclosure?
> > >
> > > Our particular need was around suppressing exceptions in
> the
> > "public"
> > > response, as Kafka Connect was passing these exceptions without
> > > authentication, they became a public endpoint upon which the
> > auditors could
> > > fuzz, and show it being out of compliance. Keeping these
> > exceptions in the
> > > logs, as proposed in the KIP, makes sense to me as an operator.
> > >
> > > I only mention PCI-DSS as this was the kind of environment
> my
> > customer
> > > had that was making the request for being able to tune the
> > response.
> > >
> > > Thanks!
> > > Connor
> > >
> > > ---
> > > Connor Penhale | Enterprise Architect, OpenLogic (
> > > https://openlogic.com/)
> > > Perforce (https://www.perforce.com/)
> > > Support: +1 866.399.6736
> > >
> > >
> > > On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org>
> wrote:
> > >
> > > Also, if you do find a security issue, the process to
> > follow is
> > > here: https://kafka.apache.org/project-security.html .
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > > > Hi Connor,
> > > >
> > > > If we are putting security-sensitive information into
> > REST
> > > responses,
> > > > that is a bug that needs to be fixed, not worked
> around
> > with a
> > > > configuration option. Do you have an example of
> > > security-sensitive
> > > > information appearing in the exception text? Why do
> > you feel
> > > that
> > > > PCI-DSS requires this change?
> > > >
> > > > By the way, the same concern applies to log messages.
> > We do not
> > > log
> > > > sensitive information such as passwords to the log4j
> > output. If
> > > you
> > > > know of that happening somewhere, please file a bug
> so
> > it can be
> > > fixed.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > > > Hi Chris!
> > > > >
> > > > > Thanks for your feedback! I'll number my responses
> to
> > your
> > > questions / thoughts.
> > > > >
> > > > > 1. Apologies on that lack of clarity! I settled on
> > "Detailed
> > > exception
> > > > > information has been suppressed. Please see logs."
> > > > > (
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34
> ).
> > > Should I update the KIP to reflect what I've already thought
> > about? It's my
> > > first one, not sure what the process should be for editing.
> > > > >
> > > > > 2. I was unaware of the REST extensions! I'll see
> if
> > I can
> > > implement
> > > > > the same behavior as a REST extension. I agree that
> > the KIP
> > > still has
> > > > > merit, regardless of the feasibility of the
> > extension, but in
> > > regards
> > > > > to the 5th thought, this might make that decision
> > easier.
> > > > >
> > > > > 3. I agree with your suggestion here. Absolutely
> > ready to take
> > > the
> > > > > community feedback on what makes sense here.
> > > > >
> > > > > 4. I should note that while I emphasized uncaught
> > exceptions,
> > > I mean
> > > > > all exceptions handled by the ExceptionMapper,
> > including
> > > > > ConnectRestExceptions. An example of this is here:
> > > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > > >
> > > > > 5. I didn't know how specific I should get if I had
> > already
> > > taken a
> > > > > stab at implementing! I'm happy to edit this in
> > whatever way
> > > we want to
> > > > > go about it.
> > > > >
> > > > > Let me know if anyone has any other questions or
> > feedback!
> > > > >
> > > > >
> > > > > Thanks!
> > > > > Connor
> > > > >
> > > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> > <ch...@confluent.io>
> > > wrote:
> > > > >
> > > > > Hi Connor,
> > > > >
> > > > > Great stuff! I generally like being able to see
> > the stack
> > > trace of an
> > > > > exception directly via the REST API but can
> > definitely
> > > understand the
> > > > > security concerns here. I've got a few
> > questions/remarks
> > > about the KIP and
> > > > > would be interested in your thoughts:
> > > > >
> > > > > 1. The KIP mentions a
> > SUPRESSED_EXCEPTION_MESSAGE, but
> > > doesn't actually
> > > > > outline what this message would actually be.
> It'd
> > be great
> > > to see the
> > > > > actual message in the KIP since people may have
> > thoughts
> > > on what it should
> > > > > be and want to comment on it as part of this
> > discussion.
> > > > >
> > > > > 2. In the "Rejected Alternatives" section, an
> > Nginx proxy
> > > is
> > > > > mentioned as
> > > > > one possible way to filter out stack traces
> from
> > the REST
> > > API. It
> > > > > seems
> > > > > like a Connect REST extension (
> > > > >
> > > > >
> > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > > )
> > > > > would be a better alternative than an Nginx
> > proxy; had you
> > > > > considered
> > > > > utilizing one? I still think this KIP is
> > worthwhile and a
> > > REST
> > > > > extension
> > > > > shouldn't be necessary in order to lock down
> the
> > REST API
> > > this way,
> > > > > but it
> > > > > might be worth calling out as an alternative
> and
> > perhaps
> > > even a
> > > > > workaround
> > > > > in cases where users are stuck on a given
> version
> > of
> > > Connect and
> > > > > can't
> > > > > upgrade to 2.6 (or whichever version this KIP
> > lands on)
> > > any time
> > > > > soon.
> > > > >
> > > > > 3. The
> > "error.rest.response.message.detail.enabled"
> > > property is a bit of a
> > > > > mouthful; it'd be great if we could come up
> with
> > something
> > > more succinct.
> > > > > What do you think about something like
> > > "rest.response.stack.traces"?
> > > > >
> > > > > 4. The KIP is targeted at stack traces for
> > uncaught
> > > exceptions, but it's
> > > > > also possible that stack traces get exposed in
> > the REST
> > > API when querying
> > > > > the status of a connector or one of its tasks.
> > Was this
> > > intentional? If so,
> > > > > it'd be great to call out why that kind of
> > filtering is
> > > not required in the
> > > > > "Rejected Alternatives" section, and if not,
> it's
> > probably
> > > not too late to
> > > > > consider modifying the KIP to cover those cases
> > as well.
> > > > >
> > > > > 5. The KIP mentions creating a new, separate
> > exception
> > > mapper class. This
> > > > > seems like more of an implementation detail and
> > something
> > > that can be
> > > > > decided on during code review; unless it's
> > critical to the
> > > functionality
> > > > > that the KIP aims to accomplish, I'd suggest
> > leaving that
> > > part out since it
> > > > > shouldn't affect the impact on users of the
> > Connect
> > > framework.
> > > > >
> > > > > Thanks for the KIP, looking forward to seeing
> > this happen!
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Chris
> > > > >
> > > > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale
> <
> > > CPenhale@perforce.com>
> > > > > wrote:
> > > > >
> > > > > > Hello All!
> > > > > >
> > > > > > I’ve created the following KIP:
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > > >
> > > > > > The PR that originated this discussion, is
> here:
> > > > > > https://github.com/apache/kafka/pull/8355
> It
> > is based
> > > on 2.0,
> > > > > but I
> > > > > > would be working on Kafka Connect in 2.6 to
> get
> > this
> > > behavior
> > > > > changed to
> > > > > > the community’s preference.
> > > > > >
> > > > > > Looking forward to working with everyone!
> > > > > >
> > > > > > Thanks!
> > > > > > Connor
> > > > > > ---
> > > > > > Connor Penhale | Enterprise Architect,
> OpenLogic
> > > > > (https://openlogic.com/)
> > > > > > Perforce (https://www.perforce.com/)
> > > > > > Support: +1 866.399.6736
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > This e-mail may contain information that is
> > privileged or
> > > > > confidential. If
> > > > > > you are not the intended recipient, please
> > delete the
> > > e-mail and
> > > > > any
> > > > > > attachments and notify us immediately.
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > CAUTION: This email originated from outside of
> the
> > > organization. Do
> > > > > not click on links or open attachments unless you
> > recognize
> > > the sender
> > > > > and know the content is safe.
> > > > >
> > > > >
> > > > >
> > > > > This e-mail may contain information that is
> > privileged or
> > > confidential.
> > > > > If you are not the intended recipient, please
> delete
> > the
> > > e-mail and any
> > > > > attachments and notify us immediately.
> > > > >
> > > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> > organization.
> > > Do not click on links or open attachments unless you recognize
> > the sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > > confidential. If you are not the intended recipient, please
> > delete the
> > > e-mail and any attachments and notify us immediately.
> > >
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential. If
> > > you are not the intended recipient, please delete the e-mail
> and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization.
> Do
> > not click on links or open attachments unless you recognize the
> sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential.
> > If you are not the intended recipient, please delete the e-mail and
> any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender and
> know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Connor,
That's great, but I think you may have mistaken Colin for me :)
One more thing that should be addressed--the "public interfaces" section
isn't just for Java interfaces, it's for any changes to any public part of
Kafka that users and external developers interact with. As far as Connect
is concerned, this includes (but is not limited to) the REST API and worker
configuration properties, so it might be worth briefly summarizing the
scope of your proposed changes in that section (something like "We plan on
adding a new worker config named <name> that will affect the REST API under
<conditions>".
Cheers,
Chris
On Wed, Apr 15, 2020 at 1:00 PM Connor Penhale <CP...@perforce.com>
wrote:
> Hi Chris,
>
> I can ask the customer if they can disclose any additional information. I
> provided the information around "PCI-DSS" to give the community a flavor of
> the type of environment the customer was operating in. The current mode is
> /not/ insecure, I would agree with this. I would be willing to agree that
> my customer has particular security audit requirements that go above and
> beyond what most environments would consider reasonable. Are you
> comfortable with that language?
>
> " enable.rest.response.stack.traces" works great for me!
>
> I created a new class in the example PR because I wanted the highest
> chance of not gunking up the works by stepping on toes in an important
> class. I figured I'd be reducing risk by creating an alternative
> implementing class. In retrospect, and now that I'm getting a first-hand
> look at Kafka's community process, that is probably unnecessary.
> Additionally, I would agree with your statement that we should modify the
> existing ExceptionMapper to avoid behavior divergence in subsequent
> releases and ensure this feature's particular scope is easy to maintain.
>
> Thanks!
> Connor
>
> On 4/15/20, 1:17 PM, "Colin McCabe" <cm...@apache.org> wrote:
>
> Hi Connor,
>
> I still would like to hear more about whether this feature is required
> for PCI-DSS or any other security certification. Nobody I talked to seemed
> to think that it was-- if there are certifications that would require this,
> it would be nice to know. However, I don't object to implementing this as
> long as we don't imply that the current mode is insecure.
>
> What do you think about using "enable.rest.response.stack.traces" as
> the config name? It seems like that makes it clearer that it's a boolean
> value.
>
> It's not really necessary to describe the internal implementation in
> the KIP, but since you mentioned it, it's probably worth considering using
> the current ExceptionMapper class with a different configuration rather
> than creating a new one.
>
> best,
> Colin
>
>
> On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> > Hi Chris!
> >
> > RE: SSL, indeed, the issue is not that the information is not
> > encrypted, but that there is no authorization layer.
> >
> > I'll be sure to edit the KIP as we continue discussion!
> >
> > RE: the 200 response you highlighted, great catch! I'll work with my
> > customer and get back to you on their audit team's intention! I'm
> > fairly certain I know the answer, but I need to be sure before I
> speak
> > for them.
> >
> > Thanks!
> > Connor
> >
> > On 4/8/20, 11:27 PM, "Christopher Egerton" <ch...@confluent.io>
> wrote:
> >
> > Hi Connor,
> >
> > Just a few more remarks!
> >
> > I noticed that you said "Kafka Connect was passing these
> exceptions without
> > authentication." For what it's worth, the Connect REST API can
> be secured
> > with TLS out-of-the-box by configuring the worker with the
> various ssl.*
> > properties, but that doesn't provide any kind of authorization
> layer to
> > provide levels of security depending who the user is. Just
> pointing out in
> > case this helps with your use case.
> >
> > As far as editing the KIP based on discussion goes--it's not only
> > acceptable, it's expected :) Ideally, the KIP should be kept
> up-to-date to
> > the point where, were it to be accepted at any moment, it would
> accurately
> > reflect the changes that would then be made to Kafka. This can
> be relaxed
> > if there's rapid iteration or items that are still up for
> discussion, but
> > as soon as things settle down it should be updated.
> >
> > As far as item 4 goes, my question was about exceptions that
> aren't handled
> > by the ExceptionMapper, but which are returned as part of the
> response body
> > when querying the status of a connector or task that has failed
> by querying
> > the /connectors/{name}/status or
> /connectors/{name}/tasks/{taskId}/status
> > endpoints. Even if the request is successful and results in an
> HTTP 200
> > response, the body might contain a stack trace if the connector
> or any of
> > its tasks have failed.
> >
> > For example, I ran an instance of the FileStreamSource connector
> named
> > "file-source" locally and instructed it to consume from a file
> that it
> > lacked permissions to read. When I queried the status of that
> connector by
> > issuing a request to /connectors/file-source/status, I got back
> the
> > following response:
> >
> > {
> > "name": "file-source",
> > "connector": {
> > "state": "RUNNING",
> > "worker_id": "192.168.86.21:8083"
> > },
> > "tasks": [
> > {
> > "id": 0,
> > "state": "FAILED",
> > "worker_id": "192.168.86.21:8083",
> > "trace": "org.apache.kafka.connect.errors.ConnectException:
> > java.nio.file.AccessDeniedException: test.txt\n\tat
> >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
> >
> >
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
> >
> >
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> > java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
> >
> >
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
> >
> >
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> > java.lang.Thread.run(Thread.java:748)\nCaused by:
> > java.nio.file.AccessDeniedException: test.txt\n\tat
> >
> >
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
> >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
> >
> >
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
> >
> >
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> > java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> > java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
> >
> >
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> > java.nio.file.Files.newInputStream(Files.java:152)\n\tat
> >
> >
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> > 9 more\n"
> > }
> > ],
> > "type": "source"
> > }
> >
> > Note the "trace" field in the first element of the "tasks" field
> of the
> > response: this was the stack trace for the exception that caused
> the task
> > to fail during execution, which has nothing to do with the
> success or
> > failure of the REST request I issued to the
> /connectors/file-source/status
> > endpoint.
> >
> > I was wondering if you wanted to include these kinds of stack
> traces as
> > part of the KIP, as opposed to uncaught exceptions that result
> in a 500
> > error from the REST API.
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <
> CPenhale@perforce.com> wrote:
> >
> > > Hi All!
> > >
> > > Is there any additional feedback that the community can provide
> > me on the
> > > KIP? Has anyone else run into requirements like this, or maybe
> my
> > customer
> > > is the only one :)? If the scope looks good, is it time to
> call a
> > vote? Or
> > > should I start porting my 2.0 code to 2.6 to show examples?
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com>
> > wrote:
> > >
> > > Hi Colin,
> > >
> > > We did not find a specific security vulnerability. Our
> > customer had
> > > auditors in their environment, and they identified Kafka
> Connect
> > as out of
> > > compliance with their particular standards, something that
> > happens all the
> > > time for REST-based applications. What these security auditors
> > expected
> > > Kafka Connect to be able to do was tune the response. As Kafka
> > Connect
> > > could not provide this functionality, I'm proposing this KIP.
> > Does that
> > > make sense? Should I still go through the process of a security
> > disclosure?
> > >
> > > Our particular need was around suppressing exceptions in
> the
> > "public"
> > > response, as Kafka Connect was passing these exceptions without
> > > authentication, they became a public endpoint upon which the
> > auditors could
> > > fuzz, and show it being out of compliance. Keeping these
> > exceptions in the
> > > logs, as proposed in the KIP, makes sense to me as an operator.
> > >
> > > I only mention PCI-DSS as this was the kind of environment
> my
> > customer
> > > had that was making the request for being able to tune the
> > response.
> > >
> > > Thanks!
> > > Connor
> > >
> > > ---
> > > Connor Penhale | Enterprise Architect, OpenLogic (
> > > https://openlogic.com/)
> > > Perforce (https://www.perforce.com/)
> > > Support: +1 866.399.6736
> > >
> > >
> > > On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org>
> wrote:
> > >
> > > Also, if you do find a security issue, the process to
> > follow is
> > > here: https://kafka.apache.org/project-security.html .
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > > > Hi Connor,
> > > >
> > > > If we are putting security-sensitive information into
> > REST
> > > responses,
> > > > that is a bug that needs to be fixed, not worked
> around
> > with a
> > > > configuration option. Do you have an example of
> > > security-sensitive
> > > > information appearing in the exception text? Why do
> > you feel
> > > that
> > > > PCI-DSS requires this change?
> > > >
> > > > By the way, the same concern applies to log messages.
> > We do not
> > > log
> > > > sensitive information such as passwords to the log4j
> > output. If
> > > you
> > > > know of that happening somewhere, please file a bug
> so
> > it can be
> > > fixed.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > > > Hi Chris!
> > > > >
> > > > > Thanks for your feedback! I'll number my responses
> to
> > your
> > > questions / thoughts.
> > > > >
> > > > > 1. Apologies on that lack of clarity! I settled on
> > "Detailed
> > > exception
> > > > > information has been suppressed. Please see logs."
> > > > > (
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34
> ).
> > > Should I update the KIP to reflect what I've already thought
> > about? It's my
> > > first one, not sure what the process should be for editing.
> > > > >
> > > > > 2. I was unaware of the REST extensions! I'll see
> if
> > I can
> > > implement
> > > > > the same behavior as a REST extension. I agree that
> > the KIP
> > > still has
> > > > > merit, regardless of the feasibility of the
> > extension, but in
> > > regards
> > > > > to the 5th thought, this might make that decision
> > easier.
> > > > >
> > > > > 3. I agree with your suggestion here. Absolutely
> > ready to take
> > > the
> > > > > community feedback on what makes sense here.
> > > > >
> > > > > 4. I should note that while I emphasized uncaught
> > exceptions,
> > > I mean
> > > > > all exceptions handled by the ExceptionMapper,
> > including
> > > > > ConnectRestExceptions. An example of this is here:
> > > > >
> > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > > >
> > > > > 5. I didn't know how specific I should get if I had
> > already
> > > taken a
> > > > > stab at implementing! I'm happy to edit this in
> > whatever way
> > > we want to
> > > > > go about it.
> > > > >
> > > > > Let me know if anyone has any other questions or
> > feedback!
> > > > >
> > > > >
> > > > > Thanks!
> > > > > Connor
> > > > >
> > > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> > <ch...@confluent.io>
> > > wrote:
> > > > >
> > > > > Hi Connor,
> > > > >
> > > > > Great stuff! I generally like being able to see
> > the stack
> > > trace of an
> > > > > exception directly via the REST API but can
> > definitely
> > > understand the
> > > > > security concerns here. I've got a few
> > questions/remarks
> > > about the KIP and
> > > > > would be interested in your thoughts:
> > > > >
> > > > > 1. The KIP mentions a
> > SUPRESSED_EXCEPTION_MESSAGE, but
> > > doesn't actually
> > > > > outline what this message would actually be.
> It'd
> > be great
> > > to see the
> > > > > actual message in the KIP since people may have
> > thoughts
> > > on what it should
> > > > > be and want to comment on it as part of this
> > discussion.
> > > > >
> > > > > 2. In the "Rejected Alternatives" section, an
> > Nginx proxy
> > > is
> > > > > mentioned as
> > > > > one possible way to filter out stack traces
> from
> > the REST
> > > API. It
> > > > > seems
> > > > > like a Connect REST extension (
> > > > >
> > > > >
> > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > > )
> > > > > would be a better alternative than an Nginx
> > proxy; had you
> > > > > considered
> > > > > utilizing one? I still think this KIP is
> > worthwhile and a
> > > REST
> > > > > extension
> > > > > shouldn't be necessary in order to lock down
> the
> > REST API
> > > this way,
> > > > > but it
> > > > > might be worth calling out as an alternative
> and
> > perhaps
> > > even a
> > > > > workaround
> > > > > in cases where users are stuck on a given
> version
> > of
> > > Connect and
> > > > > can't
> > > > > upgrade to 2.6 (or whichever version this KIP
> > lands on)
> > > any time
> > > > > soon.
> > > > >
> > > > > 3. The
> > "error.rest.response.message.detail.enabled"
> > > property is a bit of a
> > > > > mouthful; it'd be great if we could come up
> with
> > something
> > > more succinct.
> > > > > What do you think about something like
> > > "rest.response.stack.traces"?
> > > > >
> > > > > 4. The KIP is targeted at stack traces for
> > uncaught
> > > exceptions, but it's
> > > > > also possible that stack traces get exposed in
> > the REST
> > > API when querying
> > > > > the status of a connector or one of its tasks.
> > Was this
> > > intentional? If so,
> > > > > it'd be great to call out why that kind of
> > filtering is
> > > not required in the
> > > > > "Rejected Alternatives" section, and if not,
> it's
> > probably
> > > not too late to
> > > > > consider modifying the KIP to cover those cases
> > as well.
> > > > >
> > > > > 5. The KIP mentions creating a new, separate
> > exception
> > > mapper class. This
> > > > > seems like more of an implementation detail and
> > something
> > > that can be
> > > > > decided on during code review; unless it's
> > critical to the
> > > functionality
> > > > > that the KIP aims to accomplish, I'd suggest
> > leaving that
> > > part out since it
> > > > > shouldn't affect the impact on users of the
> > Connect
> > > framework.
> > > > >
> > > > > Thanks for the KIP, looking forward to seeing
> > this happen!
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Chris
> > > > >
> > > > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale
> <
> > > CPenhale@perforce.com>
> > > > > wrote:
> > > > >
> > > > > > Hello All!
> > > > > >
> > > > > > I’ve created the following KIP:
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > > >
> > > > > > The PR that originated this discussion, is
> here:
> > > > > > https://github.com/apache/kafka/pull/8355
> It
> > is based
> > > on 2.0,
> > > > > but I
> > > > > > would be working on Kafka Connect in 2.6 to
> get
> > this
> > > behavior
> > > > > changed to
> > > > > > the community’s preference.
> > > > > >
> > > > > > Looking forward to working with everyone!
> > > > > >
> > > > > > Thanks!
> > > > > > Connor
> > > > > > ---
> > > > > > Connor Penhale | Enterprise Architect,
> OpenLogic
> > > > > (https://openlogic.com/)
> > > > > > Perforce (https://www.perforce.com/)
> > > > > > Support: +1 866.399.6736
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > This e-mail may contain information that is
> > privileged or
> > > > > confidential. If
> > > > > > you are not the intended recipient, please
> > delete the
> > > e-mail and
> > > > > any
> > > > > > attachments and notify us immediately.
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > CAUTION: This email originated from outside of
> the
> > > organization. Do
> > > > > not click on links or open attachments unless you
> > recognize
> > > the sender
> > > > > and know the content is safe.
> > > > >
> > > > >
> > > > >
> > > > > This e-mail may contain information that is
> > privileged or
> > > confidential.
> > > > > If you are not the intended recipient, please
> delete
> > the
> > > e-mail and any
> > > > > attachments and notify us immediately.
> > > > >
> > > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> > organization.
> > > Do not click on links or open attachments unless you recognize
> > the sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > > confidential. If you are not the intended recipient, please
> > delete the
> > > e-mail and any attachments and notify us immediately.
> > >
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential. If
> > > you are not the intended recipient, please delete the e-mail
> and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization.
> Do
> > not click on links or open attachments unless you recognize the
> sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential.
> > If you are not the intended recipient, please delete the e-mail and
> any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender and
> know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Chris,
I can ask the customer if they can disclose any additional information. I provided the information around "PCI-DSS" to give the community a flavor of the type of environment the customer was operating in. The current mode is /not/ insecure, I would agree with this. I would be willing to agree that my customer has particular security audit requirements that go above and beyond what most environments would consider reasonable. Are you comfortable with that language?
" enable.rest.response.stack.traces" works great for me!
I created a new class in the example PR because I wanted the highest chance of not gunking up the works by stepping on toes in an important class. I figured I'd be reducing risk by creating an alternative implementing class. In retrospect, and now that I'm getting a first-hand look at Kafka's community process, that is probably unnecessary. Additionally, I would agree with your statement that we should modify the existing ExceptionMapper to avoid behavior divergence in subsequent releases and ensure this feature's particular scope is easy to maintain.
Thanks!
Connor
On 4/15/20, 1:17 PM, "Colin McCabe" <cm...@apache.org> wrote:
Hi Connor,
I still would like to hear more about whether this feature is required for PCI-DSS or any other security certification. Nobody I talked to seemed to think that it was-- if there are certifications that would require this, it would be nice to know. However, I don't object to implementing this as long as we don't imply that the current mode is insecure.
What do you think about using "enable.rest.response.stack.traces" as the config name? It seems like that makes it clearer that it's a boolean value.
It's not really necessary to describe the internal implementation in the KIP, but since you mentioned it, it's probably worth considering using the current ExceptionMapper class with a different configuration rather than creating a new one.
best,
Colin
On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> Hi Chris!
>
> RE: SSL, indeed, the issue is not that the information is not
> encrypted, but that there is no authorization layer.
>
> I'll be sure to edit the KIP as we continue discussion!
>
> RE: the 200 response you highlighted, great catch! I'll work with my
> customer and get back to you on their audit team's intention! I'm
> fairly certain I know the answer, but I need to be sure before I speak
> for them.
>
> Thanks!
> Connor
>
> On 4/8/20, 11:27 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
>
> Hi Connor,
>
> Just a few more remarks!
>
> I noticed that you said "Kafka Connect was passing these exceptions without
> authentication." For what it's worth, the Connect REST API can be secured
> with TLS out-of-the-box by configuring the worker with the various ssl.*
> properties, but that doesn't provide any kind of authorization layer to
> provide levels of security depending who the user is. Just pointing out in
> case this helps with your use case.
>
> As far as editing the KIP based on discussion goes--it's not only
> acceptable, it's expected :) Ideally, the KIP should be kept up-to-date to
> the point where, were it to be accepted at any moment, it would accurately
> reflect the changes that would then be made to Kafka. This can be relaxed
> if there's rapid iteration or items that are still up for discussion, but
> as soon as things settle down it should be updated.
>
> As far as item 4 goes, my question was about exceptions that aren't handled
> by the ExceptionMapper, but which are returned as part of the response body
> when querying the status of a connector or task that has failed by querying
> the /connectors/{name}/status or /connectors/{name}/tasks/{taskId}/status
> endpoints. Even if the request is successful and results in an HTTP 200
> response, the body might contain a stack trace if the connector or any of
> its tasks have failed.
>
> For example, I ran an instance of the FileStreamSource connector named
> "file-source" locally and instructed it to consume from a file that it
> lacked permissions to read. When I queried the status of that connector by
> issuing a request to /connectors/file-source/status, I got back the
> following response:
>
> {
> "name": "file-source",
> "connector": {
> "state": "RUNNING",
> "worker_id": "192.168.86.21:8083"
> },
> "tasks": [
> {
> "id": 0,
> "state": "FAILED",
> "worker_id": "192.168.86.21:8083",
> "trace": "org.apache.kafka.connect.errors.ConnectException:
> java.nio.file.AccessDeniedException: test.txt\n\tat
>
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
>
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> java.lang.Thread.run(Thread.java:748)\nCaused by:
> java.nio.file.AccessDeniedException: test.txt\n\tat
>
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
>
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
>
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
>
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
>
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> java.nio.file.Files.newInputStream(Files.java:152)\n\tat
>
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> 9 more\n"
> }
> ],
> "type": "source"
> }
>
> Note the "trace" field in the first element of the "tasks" field of the
> response: this was the stack trace for the exception that caused the task
> to fail during execution, which has nothing to do with the success or
> failure of the REST request I issued to the /connectors/file-source/status
> endpoint.
>
> I was wondering if you wanted to include these kinds of stack traces as
> part of the KIP, as opposed to uncaught exceptions that result in a 500
> error from the REST API.
>
> Cheers,
>
> Chris
>
> On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <CP...@perforce.com> wrote:
>
> > Hi All!
> >
> > Is there any additional feedback that the community can provide
> me on the
> > KIP? Has anyone else run into requirements like this, or maybe my
> customer
> > is the only one :)? If the scope looks good, is it time to call a
> vote? Or
> > should I start porting my 2.0 code to 2.6 to show examples?
> >
> > Thanks!
> > Connor
> >
> > On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com>
> wrote:
> >
> > Hi Colin,
> >
> > We did not find a specific security vulnerability. Our
> customer had
> > auditors in their environment, and they identified Kafka Connect
> as out of
> > compliance with their particular standards, something that
> happens all the
> > time for REST-based applications. What these security auditors
> expected
> > Kafka Connect to be able to do was tune the response. As Kafka
> Connect
> > could not provide this functionality, I'm proposing this KIP.
> Does that
> > make sense? Should I still go through the process of a security
> disclosure?
> >
> > Our particular need was around suppressing exceptions in the
> "public"
> > response, as Kafka Connect was passing these exceptions without
> > authentication, they became a public endpoint upon which the
> auditors could
> > fuzz, and show it being out of compliance. Keeping these
> exceptions in the
> > logs, as proposed in the KIP, makes sense to me as an operator.
> >
> > I only mention PCI-DSS as this was the kind of environment my
> customer
> > had that was making the request for being able to tune the
> response.
> >
> > Thanks!
> > Connor
> >
> > ---
> > Connor Penhale | Enterprise Architect, OpenLogic (
> > https://openlogic.com/)
> > Perforce (https://www.perforce.com/)
> > Support: +1 866.399.6736
> >
> >
> > On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
> >
> > Also, if you do find a security issue, the process to
> follow is
> > here: https://kafka.apache.org/project-security.html .
> >
> > best,
> > Colin
> >
> >
> > On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > > Hi Connor,
> > >
> > > If we are putting security-sensitive information into
> REST
> > responses,
> > > that is a bug that needs to be fixed, not worked around
> with a
> > > configuration option. Do you have an example of
> > security-sensitive
> > > information appearing in the exception text? Why do
> you feel
> > that
> > > PCI-DSS requires this change?
> > >
> > > By the way, the same concern applies to log messages.
> We do not
> > log
> > > sensitive information such as passwords to the log4j
> output. If
> > you
> > > know of that happening somewhere, please file a bug so
> it can be
> > fixed.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > > Hi Chris!
> > > >
> > > > Thanks for your feedback! I'll number my responses to
> your
> > questions / thoughts.
> > > >
> > > > 1. Apologies on that lack of clarity! I settled on
> "Detailed
> > exception
> > > > information has been suppressed. Please see logs."
> > > > (
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34).
> > Should I update the KIP to reflect what I've already thought
> about? It's my
> > first one, not sure what the process should be for editing.
> > > >
> > > > 2. I was unaware of the REST extensions! I'll see if
> I can
> > implement
> > > > the same behavior as a REST extension. I agree that
> the KIP
> > still has
> > > > merit, regardless of the feasibility of the
> extension, but in
> > regards
> > > > to the 5th thought, this might make that decision
> easier.
> > > >
> > > > 3. I agree with your suggestion here. Absolutely
> ready to take
> > the
> > > > community feedback on what makes sense here.
> > > >
> > > > 4. I should note that while I emphasized uncaught
> exceptions,
> > I mean
> > > > all exceptions handled by the ExceptionMapper,
> including
> > > > ConnectRestExceptions. An example of this is here:
> > > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > >
> > > > 5. I didn't know how specific I should get if I had
> already
> > taken a
> > > > stab at implementing! I'm happy to edit this in
> whatever way
> > we want to
> > > > go about it.
> > > >
> > > > Let me know if anyone has any other questions or
> feedback!
> > > >
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> <ch...@confluent.io>
> > wrote:
> > > >
> > > > Hi Connor,
> > > >
> > > > Great stuff! I generally like being able to see
> the stack
> > trace of an
> > > > exception directly via the REST API but can
> definitely
> > understand the
> > > > security concerns here. I've got a few
> questions/remarks
> > about the KIP and
> > > > would be interested in your thoughts:
> > > >
> > > > 1. The KIP mentions a
> SUPRESSED_EXCEPTION_MESSAGE, but
> > doesn't actually
> > > > outline what this message would actually be. It'd
> be great
> > to see the
> > > > actual message in the KIP since people may have
> thoughts
> > on what it should
> > > > be and want to comment on it as part of this
> discussion.
> > > >
> > > > 2. In the "Rejected Alternatives" section, an
> Nginx proxy
> > is
> > > > mentioned as
> > > > one possible way to filter out stack traces from
> the REST
> > API. It
> > > > seems
> > > > like a Connect REST extension (
> > > >
> > > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > )
> > > > would be a better alternative than an Nginx
> proxy; had you
> > > > considered
> > > > utilizing one? I still think this KIP is
> worthwhile and a
> > REST
> > > > extension
> > > > shouldn't be necessary in order to lock down the
> REST API
> > this way,
> > > > but it
> > > > might be worth calling out as an alternative and
> perhaps
> > even a
> > > > workaround
> > > > in cases where users are stuck on a given version
> of
> > Connect and
> > > > can't
> > > > upgrade to 2.6 (or whichever version this KIP
> lands on)
> > any time
> > > > soon.
> > > >
> > > > 3. The
> "error.rest.response.message.detail.enabled"
> > property is a bit of a
> > > > mouthful; it'd be great if we could come up with
> something
> > more succinct.
> > > > What do you think about something like
> > "rest.response.stack.traces"?
> > > >
> > > > 4. The KIP is targeted at stack traces for
> uncaught
> > exceptions, but it's
> > > > also possible that stack traces get exposed in
> the REST
> > API when querying
> > > > the status of a connector or one of its tasks.
> Was this
> > intentional? If so,
> > > > it'd be great to call out why that kind of
> filtering is
> > not required in the
> > > > "Rejected Alternatives" section, and if not, it's
> probably
> > not too late to
> > > > consider modifying the KIP to cover those cases
> as well.
> > > >
> > > > 5. The KIP mentions creating a new, separate
> exception
> > mapper class. This
> > > > seems like more of an implementation detail and
> something
> > that can be
> > > > decided on during code review; unless it's
> critical to the
> > functionality
> > > > that the KIP aims to accomplish, I'd suggest
> leaving that
> > part out since it
> > > > shouldn't affect the impact on users of the
> Connect
> > framework.
> > > >
> > > > Thanks for the KIP, looking forward to seeing
> this happen!
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <
> > CPenhale@perforce.com>
> > > > wrote:
> > > >
> > > > > Hello All!
> > > > >
> > > > > I’ve created the following KIP:
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > >
> > > > > The PR that originated this discussion, is here:
> > > > > https://github.com/apache/kafka/pull/8355 It
> is based
> > on 2.0,
> > > > but I
> > > > > would be working on Kafka Connect in 2.6 to get
> this
> > behavior
> > > > changed to
> > > > > the community’s preference.
> > > > >
> > > > > Looking forward to working with everyone!
> > > > >
> > > > > Thanks!
> > > > > Connor
> > > > > ---
> > > > > Connor Penhale | Enterprise Architect, OpenLogic
> > > > (https://openlogic.com/)
> > > > > Perforce (https://www.perforce.com/)
> > > > > Support: +1 866.399.6736
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > This e-mail may contain information that is
> privileged or
> > > > confidential. If
> > > > > you are not the intended recipient, please
> delete the
> > e-mail and
> > > > any
> > > > > attachments and notify us immediately.
> > > > >
> > > > >
> > > >
> > > >
> > > > CAUTION: This email originated from outside of the
> > organization. Do
> > > > not click on links or open attachments unless you
> recognize
> > the sender
> > > > and know the content is safe.
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is
> privileged or
> > confidential.
> > > > If you are not the intended recipient, please delete
> the
> > e-mail and any
> > > > attachments and notify us immediately.
> > > >
> > > >
> >
> >
> > CAUTION: This email originated from outside of the
> organization.
> > Do not click on links or open attachments unless you recognize
> the sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> > confidential. If you are not the intended recipient, please
> delete the
> > e-mail and any attachments and notify us immediately.
> >
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential. If
> > you are not the intended recipient, please delete the e-mail and
> any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or confidential.
> If you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled exceptions in security-sensitive environments
Posted by Colin McCabe <cm...@apache.org>.
Hi Connor,
I still would like to hear more about whether this feature is required for PCI-DSS or any other security certification. Nobody I talked to seemed to think that it was-- if there are certifications that would require this, it would be nice to know. However, I don't object to implementing this as long as we don't imply that the current mode is insecure.
What do you think about using "enable.rest.response.stack.traces" as the config name? It seems like that makes it clearer that it's a boolean value.
It's not really necessary to describe the internal implementation in the KIP, but since you mentioned it, it's probably worth considering using the current ExceptionMapper class with a different configuration rather than creating a new one.
best,
Colin
On Mon, Apr 13, 2020, at 09:04, Connor Penhale wrote:
> Hi Chris!
>
> RE: SSL, indeed, the issue is not that the information is not
> encrypted, but that there is no authorization layer.
>
> I'll be sure to edit the KIP as we continue discussion!
>
> RE: the 200 response you highlighted, great catch! I'll work with my
> customer and get back to you on their audit team's intention! I'm
> fairly certain I know the answer, but I need to be sure before I speak
> for them.
>
> Thanks!
> Connor
>
> On 4/8/20, 11:27 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
>
> Hi Connor,
>
> Just a few more remarks!
>
> I noticed that you said "Kafka Connect was passing these exceptions without
> authentication." For what it's worth, the Connect REST API can be secured
> with TLS out-of-the-box by configuring the worker with the various ssl.*
> properties, but that doesn't provide any kind of authorization layer to
> provide levels of security depending who the user is. Just pointing out in
> case this helps with your use case.
>
> As far as editing the KIP based on discussion goes--it's not only
> acceptable, it's expected :) Ideally, the KIP should be kept up-to-date to
> the point where, were it to be accepted at any moment, it would accurately
> reflect the changes that would then be made to Kafka. This can be relaxed
> if there's rapid iteration or items that are still up for discussion, but
> as soon as things settle down it should be updated.
>
> As far as item 4 goes, my question was about exceptions that aren't handled
> by the ExceptionMapper, but which are returned as part of the response body
> when querying the status of a connector or task that has failed by querying
> the /connectors/{name}/status or /connectors/{name}/tasks/{taskId}/status
> endpoints. Even if the request is successful and results in an HTTP 200
> response, the body might contain a stack trace if the connector or any of
> its tasks have failed.
>
> For example, I ran an instance of the FileStreamSource connector named
> "file-source" locally and instructed it to consume from a file that it
> lacked permissions to read. When I queried the status of that connector by
> issuing a request to /connectors/file-source/status, I got back the
> following response:
>
> {
> "name": "file-source",
> "connector": {
> "state": "RUNNING",
> "worker_id": "192.168.86.21:8083"
> },
> "tasks": [
> {
> "id": 0,
> "state": "FAILED",
> "worker_id": "192.168.86.21:8083",
> "trace": "org.apache.kafka.connect.errors.ConnectException:
> java.nio.file.AccessDeniedException: test.txt\n\tat
>
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
>
> org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
>
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
> java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
> java.lang.Thread.run(Thread.java:748)\nCaused by:
> java.nio.file.AccessDeniedException: test.txt\n\tat
>
> sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
>
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
>
> sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
>
> sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
> java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
> java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
>
> java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
> java.nio.file.Files.newInputStream(Files.java:152)\n\tat
>
> org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
> 9 more\n"
> }
> ],
> "type": "source"
> }
>
> Note the "trace" field in the first element of the "tasks" field of the
> response: this was the stack trace for the exception that caused the task
> to fail during execution, which has nothing to do with the success or
> failure of the REST request I issued to the /connectors/file-source/status
> endpoint.
>
> I was wondering if you wanted to include these kinds of stack traces as
> part of the KIP, as opposed to uncaught exceptions that result in a 500
> error from the REST API.
>
> Cheers,
>
> Chris
>
> On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <CP...@perforce.com> wrote:
>
> > Hi All!
> >
> > Is there any additional feedback that the community can provide
> me on the
> > KIP? Has anyone else run into requirements like this, or maybe my
> customer
> > is the only one :)? If the scope looks good, is it time to call a
> vote? Or
> > should I start porting my 2.0 code to 2.6 to show examples?
> >
> > Thanks!
> > Connor
> >
> > On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com>
> wrote:
> >
> > Hi Colin,
> >
> > We did not find a specific security vulnerability. Our
> customer had
> > auditors in their environment, and they identified Kafka Connect
> as out of
> > compliance with their particular standards, something that
> happens all the
> > time for REST-based applications. What these security auditors
> expected
> > Kafka Connect to be able to do was tune the response. As Kafka
> Connect
> > could not provide this functionality, I'm proposing this KIP.
> Does that
> > make sense? Should I still go through the process of a security
> disclosure?
> >
> > Our particular need was around suppressing exceptions in the
> "public"
> > response, as Kafka Connect was passing these exceptions without
> > authentication, they became a public endpoint upon which the
> auditors could
> > fuzz, and show it being out of compliance. Keeping these
> exceptions in the
> > logs, as proposed in the KIP, makes sense to me as an operator.
> >
> > I only mention PCI-DSS as this was the kind of environment my
> customer
> > had that was making the request for being able to tune the
> response.
> >
> > Thanks!
> > Connor
> >
> > ---
> > Connor Penhale | Enterprise Architect, OpenLogic (
> > https://openlogic.com/)
> > Perforce (https://www.perforce.com/)
> > Support: +1 866.399.6736
> >
> >
> > On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
> >
> > Also, if you do find a security issue, the process to
> follow is
> > here: https://kafka.apache.org/project-security.html .
> >
> > best,
> > Colin
> >
> >
> > On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > > Hi Connor,
> > >
> > > If we are putting security-sensitive information into
> REST
> > responses,
> > > that is a bug that needs to be fixed, not worked around
> with a
> > > configuration option. Do you have an example of
> > security-sensitive
> > > information appearing in the exception text? Why do
> you feel
> > that
> > > PCI-DSS requires this change?
> > >
> > > By the way, the same concern applies to log messages.
> We do not
> > log
> > > sensitive information such as passwords to the log4j
> output. If
> > you
> > > know of that happening somewhere, please file a bug so
> it can be
> > fixed.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > > Hi Chris!
> > > >
> > > > Thanks for your feedback! I'll number my responses to
> your
> > questions / thoughts.
> > > >
> > > > 1. Apologies on that lack of clarity! I settled on
> "Detailed
> > exception
> > > > information has been suppressed. Please see logs."
> > > > (
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34).
> > Should I update the KIP to reflect what I've already thought
> about? It's my
> > first one, not sure what the process should be for editing.
> > > >
> > > > 2. I was unaware of the REST extensions! I'll see if
> I can
> > implement
> > > > the same behavior as a REST extension. I agree that
> the KIP
> > still has
> > > > merit, regardless of the feasibility of the
> extension, but in
> > regards
> > > > to the 5th thought, this might make that decision
> easier.
> > > >
> > > > 3. I agree with your suggestion here. Absolutely
> ready to take
> > the
> > > > community feedback on what makes sense here.
> > > >
> > > > 4. I should note that while I emphasized uncaught
> exceptions,
> > I mean
> > > > all exceptions handled by the ExceptionMapper,
> including
> > > > ConnectRestExceptions. An example of this is here:
> > > >
> >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > > >
> > > > 5. I didn't know how specific I should get if I had
> already
> > taken a
> > > > stab at implementing! I'm happy to edit this in
> whatever way
> > we want to
> > > > go about it.
> > > >
> > > > Let me know if anyone has any other questions or
> feedback!
> > > >
> > > >
> > > > Thanks!
> > > > Connor
> > > >
> > > > On 4/2/20, 3:58 PM, "Christopher Egerton"
> <ch...@confluent.io>
> > wrote:
> > > >
> > > > Hi Connor,
> > > >
> > > > Great stuff! I generally like being able to see
> the stack
> > trace of an
> > > > exception directly via the REST API but can
> definitely
> > understand the
> > > > security concerns here. I've got a few
> questions/remarks
> > about the KIP and
> > > > would be interested in your thoughts:
> > > >
> > > > 1. The KIP mentions a
> SUPRESSED_EXCEPTION_MESSAGE, but
> > doesn't actually
> > > > outline what this message would actually be. It'd
> be great
> > to see the
> > > > actual message in the KIP since people may have
> thoughts
> > on what it should
> > > > be and want to comment on it as part of this
> discussion.
> > > >
> > > > 2. In the "Rejected Alternatives" section, an
> Nginx proxy
> > is
> > > > mentioned as
> > > > one possible way to filter out stack traces from
> the REST
> > API. It
> > > > seems
> > > > like a Connect REST extension (
> > > >
> > > >
> >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> > )
> > > > would be a better alternative than an Nginx
> proxy; had you
> > > > considered
> > > > utilizing one? I still think this KIP is
> worthwhile and a
> > REST
> > > > extension
> > > > shouldn't be necessary in order to lock down the
> REST API
> > this way,
> > > > but it
> > > > might be worth calling out as an alternative and
> perhaps
> > even a
> > > > workaround
> > > > in cases where users are stuck on a given version
> of
> > Connect and
> > > > can't
> > > > upgrade to 2.6 (or whichever version this KIP
> lands on)
> > any time
> > > > soon.
> > > >
> > > > 3. The
> "error.rest.response.message.detail.enabled"
> > property is a bit of a
> > > > mouthful; it'd be great if we could come up with
> something
> > more succinct.
> > > > What do you think about something like
> > "rest.response.stack.traces"?
> > > >
> > > > 4. The KIP is targeted at stack traces for
> uncaught
> > exceptions, but it's
> > > > also possible that stack traces get exposed in
> the REST
> > API when querying
> > > > the status of a connector or one of its tasks.
> Was this
> > intentional? If so,
> > > > it'd be great to call out why that kind of
> filtering is
> > not required in the
> > > > "Rejected Alternatives" section, and if not, it's
> probably
> > not too late to
> > > > consider modifying the KIP to cover those cases
> as well.
> > > >
> > > > 5. The KIP mentions creating a new, separate
> exception
> > mapper class. This
> > > > seems like more of an implementation detail and
> something
> > that can be
> > > > decided on during code review; unless it's
> critical to the
> > functionality
> > > > that the KIP aims to accomplish, I'd suggest
> leaving that
> > part out since it
> > > > shouldn't affect the impact on users of the
> Connect
> > framework.
> > > >
> > > > Thanks for the KIP, looking forward to seeing
> this happen!
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <
> > CPenhale@perforce.com>
> > > > wrote:
> > > >
> > > > > Hello All!
> > > > >
> > > > > I’ve created the following KIP:
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > > >
> > > > > The PR that originated this discussion, is here:
> > > > > https://github.com/apache/kafka/pull/8355 It
> is based
> > on 2.0,
> > > > but I
> > > > > would be working on Kafka Connect in 2.6 to get
> this
> > behavior
> > > > changed to
> > > > > the community’s preference.
> > > > >
> > > > > Looking forward to working with everyone!
> > > > >
> > > > > Thanks!
> > > > > Connor
> > > > > ---
> > > > > Connor Penhale | Enterprise Architect, OpenLogic
> > > > (https://openlogic.com/)
> > > > > Perforce (https://www.perforce.com/)
> > > > > Support: +1 866.399.6736
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > This e-mail may contain information that is
> privileged or
> > > > confidential. If
> > > > > you are not the intended recipient, please
> delete the
> > e-mail and
> > > > any
> > > > > attachments and notify us immediately.
> > > > >
> > > > >
> > > >
> > > >
> > > > CAUTION: This email originated from outside of the
> > organization. Do
> > > > not click on links or open attachments unless you
> recognize
> > the sender
> > > > and know the content is safe.
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is
> privileged or
> > confidential.
> > > > If you are not the intended recipient, please delete
> the
> > e-mail and any
> > > > attachments and notify us immediately.
> > > >
> > > >
> >
> >
> > CAUTION: This email originated from outside of the
> organization.
> > Do not click on links or open attachments unless you recognize
> the sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or
> > confidential. If you are not the intended recipient, please
> delete the
> > e-mail and any attachments and notify us immediately.
> >
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential. If
> > you are not the intended recipient, please delete the e-mail and
> any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or confidential.
> If you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Chris!
RE: SSL, indeed, the issue is not that the information is not encrypted, but that there is no authorization layer.
I'll be sure to edit the KIP as we continue discussion!
RE: the 200 response you highlighted, great catch! I'll work with my customer and get back to you on their audit team's intention! I'm fairly certain I know the answer, but I need to be sure before I speak for them.
Thanks!
Connor
On 4/8/20, 11:27 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
Hi Connor,
Just a few more remarks!
I noticed that you said "Kafka Connect was passing these exceptions without
authentication." For what it's worth, the Connect REST API can be secured
with TLS out-of-the-box by configuring the worker with the various ssl.*
properties, but that doesn't provide any kind of authorization layer to
provide levels of security depending who the user is. Just pointing out in
case this helps with your use case.
As far as editing the KIP based on discussion goes--it's not only
acceptable, it's expected :) Ideally, the KIP should be kept up-to-date to
the point where, were it to be accepted at any moment, it would accurately
reflect the changes that would then be made to Kafka. This can be relaxed
if there's rapid iteration or items that are still up for discussion, but
as soon as things settle down it should be updated.
As far as item 4 goes, my question was about exceptions that aren't handled
by the ExceptionMapper, but which are returned as part of the response body
when querying the status of a connector or task that has failed by querying
the /connectors/{name}/status or /connectors/{name}/tasks/{taskId}/status
endpoints. Even if the request is successful and results in an HTTP 200
response, the body might contain a stack trace if the connector or any of
its tasks have failed.
For example, I ran an instance of the FileStreamSource connector named
"file-source" locally and instructed it to consume from a file that it
lacked permissions to read. When I queried the status of that connector by
issuing a request to /connectors/file-source/status, I got back the
following response:
{
"name": "file-source",
"connector": {
"state": "RUNNING",
"worker_id": "192.168.86.21:8083"
},
"tasks": [
{
"id": 0,
"state": "FAILED",
"worker_id": "192.168.86.21:8083",
"trace": "org.apache.kafka.connect.errors.ConnectException:
java.nio.file.AccessDeniedException: test.txt\n\tat
org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
java.lang.Thread.run(Thread.java:748)\nCaused by:
java.nio.file.AccessDeniedException: test.txt\n\tat
sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
java.nio.file.Files.newInputStream(Files.java:152)\n\tat
org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
9 more\n"
}
],
"type": "source"
}
Note the "trace" field in the first element of the "tasks" field of the
response: this was the stack trace for the exception that caused the task
to fail during execution, which has nothing to do with the success or
failure of the REST request I issued to the /connectors/file-source/status
endpoint.
I was wondering if you wanted to include these kinds of stack traces as
part of the KIP, as opposed to uncaught exceptions that result in a 500
error from the REST API.
Cheers,
Chris
On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <CP...@perforce.com> wrote:
> Hi All!
>
> Is there any additional feedback that the community can provide me on the
> KIP? Has anyone else run into requirements like this, or maybe my customer
> is the only one :)? If the scope looks good, is it time to call a vote? Or
> should I start porting my 2.0 code to 2.6 to show examples?
>
> Thanks!
> Connor
>
> On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com> wrote:
>
> Hi Colin,
>
> We did not find a specific security vulnerability. Our customer had
> auditors in their environment, and they identified Kafka Connect as out of
> compliance with their particular standards, something that happens all the
> time for REST-based applications. What these security auditors expected
> Kafka Connect to be able to do was tune the response. As Kafka Connect
> could not provide this functionality, I'm proposing this KIP. Does that
> make sense? Should I still go through the process of a security disclosure?
>
> Our particular need was around suppressing exceptions in the "public"
> response, as Kafka Connect was passing these exceptions without
> authentication, they became a public endpoint upon which the auditors could
> fuzz, and show it being out of compliance. Keeping these exceptions in the
> logs, as proposed in the KIP, makes sense to me as an operator.
>
> I only mention PCI-DSS as this was the kind of environment my customer
> had that was making the request for being able to tune the response.
>
> Thanks!
> Connor
>
> ---
> Connor Penhale | Enterprise Architect, OpenLogic (
> https://openlogic.com/)
> Perforce (https://www.perforce.com/)
> Support: +1 866.399.6736
>
>
> On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
>
> Also, if you do find a security issue, the process to follow is
> here: https://kafka.apache.org/project-security.html .
>
> best,
> Colin
>
>
> On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > Hi Connor,
> >
> > If we are putting security-sensitive information into REST
> responses,
> > that is a bug that needs to be fixed, not worked around with a
> > configuration option. Do you have an example of
> security-sensitive
> > information appearing in the exception text? Why do you feel
> that
> > PCI-DSS requires this change?
> >
> > By the way, the same concern applies to log messages. We do not
> log
> > sensitive information such as passwords to the log4j output. If
> you
> > know of that happening somewhere, please file a bug so it can be
> fixed.
> >
> > best,
> > Colin
> >
> >
> > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > Hi Chris!
> > >
> > > Thanks for your feedback! I'll number my responses to your
> questions / thoughts.
> > >
> > > 1. Apologies on that lack of clarity! I settled on "Detailed
> exception
> > > information has been suppressed. Please see logs."
> > > (
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34).
> Should I update the KIP to reflect what I've already thought about? It's my
> first one, not sure what the process should be for editing.
> > >
> > > 2. I was unaware of the REST extensions! I'll see if I can
> implement
> > > the same behavior as a REST extension. I agree that the KIP
> still has
> > > merit, regardless of the feasibility of the extension, but in
> regards
> > > to the 5th thought, this might make that decision easier.
> > >
> > > 3. I agree with your suggestion here. Absolutely ready to take
> the
> > > community feedback on what makes sense here.
> > >
> > > 4. I should note that while I emphasized uncaught exceptions,
> I mean
> > > all exceptions handled by the ExceptionMapper, including
> > > ConnectRestExceptions. An example of this is here:
> > >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > >
> > > 5. I didn't know how specific I should get if I had already
> taken a
> > > stab at implementing! I'm happy to edit this in whatever way
> we want to
> > > go about it.
> > >
> > > Let me know if anyone has any other questions or feedback!
> > >
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io>
> wrote:
> > >
> > > Hi Connor,
> > >
> > > Great stuff! I generally like being able to see the stack
> trace of an
> > > exception directly via the REST API but can definitely
> understand the
> > > security concerns here. I've got a few questions/remarks
> about the KIP and
> > > would be interested in your thoughts:
> > >
> > > 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but
> doesn't actually
> > > outline what this message would actually be. It'd be great
> to see the
> > > actual message in the KIP since people may have thoughts
> on what it should
> > > be and want to comment on it as part of this discussion.
> > >
> > > 2. In the "Rejected Alternatives" section, an Nginx proxy
> is
> > > mentioned as
> > > one possible way to filter out stack traces from the REST
> API. It
> > > seems
> > > like a Connect REST extension (
> > >
> > >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> )
> > > would be a better alternative than an Nginx proxy; had you
> > > considered
> > > utilizing one? I still think this KIP is worthwhile and a
> REST
> > > extension
> > > shouldn't be necessary in order to lock down the REST API
> this way,
> > > but it
> > > might be worth calling out as an alternative and perhaps
> even a
> > > workaround
> > > in cases where users are stuck on a given version of
> Connect and
> > > can't
> > > upgrade to 2.6 (or whichever version this KIP lands on)
> any time
> > > soon.
> > >
> > > 3. The "error.rest.response.message.detail.enabled"
> property is a bit of a
> > > mouthful; it'd be great if we could come up with something
> more succinct.
> > > What do you think about something like
> "rest.response.stack.traces"?
> > >
> > > 4. The KIP is targeted at stack traces for uncaught
> exceptions, but it's
> > > also possible that stack traces get exposed in the REST
> API when querying
> > > the status of a connector or one of its tasks. Was this
> intentional? If so,
> > > it'd be great to call out why that kind of filtering is
> not required in the
> > > "Rejected Alternatives" section, and if not, it's probably
> not too late to
> > > consider modifying the KIP to cover those cases as well.
> > >
> > > 5. The KIP mentions creating a new, separate exception
> mapper class. This
> > > seems like more of an implementation detail and something
> that can be
> > > decided on during code review; unless it's critical to the
> functionality
> > > that the KIP aims to accomplish, I'd suggest leaving that
> part out since it
> > > shouldn't affect the impact on users of the Connect
> framework.
> > >
> > > Thanks for the KIP, looking forward to seeing this happen!
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <
> CPenhale@perforce.com>
> > > wrote:
> > >
> > > > Hello All!
> > > >
> > > > I’ve created the following KIP:
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > >
> > > > The PR that originated this discussion, is here:
> > > > https://github.com/apache/kafka/pull/8355 It is based
> on 2.0,
> > > but I
> > > > would be working on Kafka Connect in 2.6 to get this
> behavior
> > > changed to
> > > > the community’s preference.
> > > >
> > > > Looking forward to working with everyone!
> > > >
> > > > Thanks!
> > > > Connor
> > > > ---
> > > > Connor Penhale | Enterprise Architect, OpenLogic
> > > (https://openlogic.com/)
> > > > Perforce (https://www.perforce.com/)
> > > > Support: +1 866.399.6736
> > > >
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is privileged or
> > > confidential. If
> > > > you are not the intended recipient, please delete the
> e-mail and
> > > any
> > > > attachments and notify us immediately.
> > > >
> > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> organization. Do
> > > not click on links or open attachments unless you recognize
> the sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> confidential.
> > > If you are not the intended recipient, please delete the
> e-mail and any
> > > attachments and notify us immediately.
> > >
> > >
>
>
> CAUTION: This email originated from outside of the organization.
> Do not click on links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or
> confidential. If you are not the intended recipient, please delete the
> e-mail and any attachments and notify us immediately.
>
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Connor,
Just a few more remarks!
I noticed that you said "Kafka Connect was passing these exceptions without
authentication." For what it's worth, the Connect REST API can be secured
with TLS out-of-the-box by configuring the worker with the various ssl.*
properties, but that doesn't provide any kind of authorization layer to
provide levels of security depending who the user is. Just pointing out in
case this helps with your use case.
As far as editing the KIP based on discussion goes--it's not only
acceptable, it's expected :) Ideally, the KIP should be kept up-to-date to
the point where, were it to be accepted at any moment, it would accurately
reflect the changes that would then be made to Kafka. This can be relaxed
if there's rapid iteration or items that are still up for discussion, but
as soon as things settle down it should be updated.
As far as item 4 goes, my question was about exceptions that aren't handled
by the ExceptionMapper, but which are returned as part of the response body
when querying the status of a connector or task that has failed by querying
the /connectors/{name}/status or /connectors/{name}/tasks/{taskId}/status
endpoints. Even if the request is successful and results in an HTTP 200
response, the body might contain a stack trace if the connector or any of
its tasks have failed.
For example, I ran an instance of the FileStreamSource connector named
"file-source" locally and instructed it to consume from a file that it
lacked permissions to read. When I queried the status of that connector by
issuing a request to /connectors/file-source/status, I got back the
following response:
{
"name": "file-source",
"connector": {
"state": "RUNNING",
"worker_id": "192.168.86.21:8083"
},
"tasks": [
{
"id": 0,
"state": "FAILED",
"worker_id": "192.168.86.21:8083",
"trace": "org.apache.kafka.connect.errors.ConnectException:
java.nio.file.AccessDeniedException: test.txt\n\tat
org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:116)\n\tat
org.apache.kafka.connect.runtime.WorkerSourceTask.poll(WorkerSourceTask.java:265)\n\tat
org.apache.kafka.connect.runtime.WorkerSourceTask.execute(WorkerSourceTask.java:232)\n\tat
org.apache.kafka.connect.runtime.WorkerTask.doRun(WorkerTask.java:177)\n\tat
org.apache.kafka.connect.runtime.WorkerTask.run(WorkerTask.java:227)\n\tat
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)\n\tat
java.util.concurrent.FutureTask.run(FutureTask.java:266)\n\tat
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)\n\tat
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)\n\tat
java.lang.Thread.run(Thread.java:748)\nCaused by:
java.nio.file.AccessDeniedException: test.txt\n\tat
sun.nio.fs.UnixException.translateToIOException(UnixException.java:84)\n\tat
sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)\n\tat
sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)\n\tat
sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)\n\tat
java.nio.file.Files.newByteChannel(Files.java:361)\n\tat
java.nio.file.Files.newByteChannel(Files.java:407)\n\tat
java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)\n\tat
java.nio.file.Files.newInputStream(Files.java:152)\n\tat
org.apache.kafka.connect.file.FileStreamSourceTask.poll(FileStreamSourceTask.java:82)\n\t...
9 more\n"
}
],
"type": "source"
}
Note the "trace" field in the first element of the "tasks" field of the
response: this was the stack trace for the exception that caused the task
to fail during execution, which has nothing to do with the success or
failure of the REST request I issued to the /connectors/file-source/status
endpoint.
I was wondering if you wanted to include these kinds of stack traces as
part of the KIP, as opposed to uncaught exceptions that result in a 500
error from the REST API.
Cheers,
Chris
On Wed, Apr 8, 2020 at 9:51 AM Connor Penhale <CP...@perforce.com> wrote:
> Hi All!
>
> Is there any additional feedback that the community can provide me on the
> KIP? Has anyone else run into requirements like this, or maybe my customer
> is the only one :)? If the scope looks good, is it time to call a vote? Or
> should I start porting my 2.0 code to 2.6 to show examples?
>
> Thanks!
> Connor
>
> On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com> wrote:
>
> Hi Colin,
>
> We did not find a specific security vulnerability. Our customer had
> auditors in their environment, and they identified Kafka Connect as out of
> compliance with their particular standards, something that happens all the
> time for REST-based applications. What these security auditors expected
> Kafka Connect to be able to do was tune the response. As Kafka Connect
> could not provide this functionality, I'm proposing this KIP. Does that
> make sense? Should I still go through the process of a security disclosure?
>
> Our particular need was around suppressing exceptions in the "public"
> response, as Kafka Connect was passing these exceptions without
> authentication, they became a public endpoint upon which the auditors could
> fuzz, and show it being out of compliance. Keeping these exceptions in the
> logs, as proposed in the KIP, makes sense to me as an operator.
>
> I only mention PCI-DSS as this was the kind of environment my customer
> had that was making the request for being able to tune the response.
>
> Thanks!
> Connor
>
> ---
> Connor Penhale | Enterprise Architect, OpenLogic (
> https://openlogic.com/)
> Perforce (https://www.perforce.com/)
> Support: +1 866.399.6736
>
>
> On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
>
> Also, if you do find a security issue, the process to follow is
> here: https://kafka.apache.org/project-security.html .
>
> best,
> Colin
>
>
> On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> > Hi Connor,
> >
> > If we are putting security-sensitive information into REST
> responses,
> > that is a bug that needs to be fixed, not worked around with a
> > configuration option. Do you have an example of
> security-sensitive
> > information appearing in the exception text? Why do you feel
> that
> > PCI-DSS requires this change?
> >
> > By the way, the same concern applies to log messages. We do not
> log
> > sensitive information such as passwords to the log4j output. If
> you
> > know of that happening somewhere, please file a bug so it can be
> fixed.
> >
> > best,
> > Colin
> >
> >
> > On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > > Hi Chris!
> > >
> > > Thanks for your feedback! I'll number my responses to your
> questions / thoughts.
> > >
> > > 1. Apologies on that lack of clarity! I settled on "Detailed
> exception
> > > information has been suppressed. Please see logs."
> > > (
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34).
> Should I update the KIP to reflect what I've already thought about? It's my
> first one, not sure what the process should be for editing.
> > >
> > > 2. I was unaware of the REST extensions! I'll see if I can
> implement
> > > the same behavior as a REST extension. I agree that the KIP
> still has
> > > merit, regardless of the feasibility of the extension, but in
> regards
> > > to the 5th thought, this might make that decision easier.
> > >
> > > 3. I agree with your suggestion here. Absolutely ready to take
> the
> > > community feedback on what makes sense here.
> > >
> > > 4. I should note that while I emphasized uncaught exceptions,
> I mean
> > > all exceptions handled by the ExceptionMapper, including
> > > ConnectRestExceptions. An example of this is here:
> > >
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> > >
> > > 5. I didn't know how specific I should get if I had already
> taken a
> > > stab at implementing! I'm happy to edit this in whatever way
> we want to
> > > go about it.
> > >
> > > Let me know if anyone has any other questions or feedback!
> > >
> > >
> > > Thanks!
> > > Connor
> > >
> > > On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io>
> wrote:
> > >
> > > Hi Connor,
> > >
> > > Great stuff! I generally like being able to see the stack
> trace of an
> > > exception directly via the REST API but can definitely
> understand the
> > > security concerns here. I've got a few questions/remarks
> about the KIP and
> > > would be interested in your thoughts:
> > >
> > > 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but
> doesn't actually
> > > outline what this message would actually be. It'd be great
> to see the
> > > actual message in the KIP since people may have thoughts
> on what it should
> > > be and want to comment on it as part of this discussion.
> > >
> > > 2. In the "Rejected Alternatives" section, an Nginx proxy
> is
> > > mentioned as
> > > one possible way to filter out stack traces from the REST
> API. It
> > > seems
> > > like a Connect REST extension (
> > >
> > >
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html
> )
> > > would be a better alternative than an Nginx proxy; had you
> > > considered
> > > utilizing one? I still think this KIP is worthwhile and a
> REST
> > > extension
> > > shouldn't be necessary in order to lock down the REST API
> this way,
> > > but it
> > > might be worth calling out as an alternative and perhaps
> even a
> > > workaround
> > > in cases where users are stuck on a given version of
> Connect and
> > > can't
> > > upgrade to 2.6 (or whichever version this KIP lands on)
> any time
> > > soon.
> > >
> > > 3. The "error.rest.response.message.detail.enabled"
> property is a bit of a
> > > mouthful; it'd be great if we could come up with something
> more succinct.
> > > What do you think about something like
> "rest.response.stack.traces"?
> > >
> > > 4. The KIP is targeted at stack traces for uncaught
> exceptions, but it's
> > > also possible that stack traces get exposed in the REST
> API when querying
> > > the status of a connector or one of its tasks. Was this
> intentional? If so,
> > > it'd be great to call out why that kind of filtering is
> not required in the
> > > "Rejected Alternatives" section, and if not, it's probably
> not too late to
> > > consider modifying the KIP to cover those cases as well.
> > >
> > > 5. The KIP mentions creating a new, separate exception
> mapper class. This
> > > seems like more of an implementation detail and something
> that can be
> > > decided on during code review; unless it's critical to the
> functionality
> > > that the KIP aims to accomplish, I'd suggest leaving that
> part out since it
> > > shouldn't affect the impact on users of the Connect
> framework.
> > >
> > > Thanks for the KIP, looking forward to seeing this happen!
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <
> CPenhale@perforce.com>
> > > wrote:
> > >
> > > > Hello All!
> > > >
> > > > I’ve created the following KIP:
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > > >
> > > > The PR that originated this discussion, is here:
> > > > https://github.com/apache/kafka/pull/8355 It is based
> on 2.0,
> > > but I
> > > > would be working on Kafka Connect in 2.6 to get this
> behavior
> > > changed to
> > > > the community’s preference.
> > > >
> > > > Looking forward to working with everyone!
> > > >
> > > > Thanks!
> > > > Connor
> > > > ---
> > > > Connor Penhale | Enterprise Architect, OpenLogic
> > > (https://openlogic.com/)
> > > > Perforce (https://www.perforce.com/)
> > > > Support: +1 866.399.6736
> > > >
> > > >
> > > >
> > > >
> > > > This e-mail may contain information that is privileged or
> > > confidential. If
> > > > you are not the intended recipient, please delete the
> e-mail and
> > > any
> > > > attachments and notify us immediately.
> > > >
> > > >
> > >
> > >
> > > CAUTION: This email originated from outside of the
> organization. Do
> > > not click on links or open attachments unless you recognize
> the sender
> > > and know the content is safe.
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> confidential.
> > > If you are not the intended recipient, please delete the
> e-mail and any
> > > attachments and notify us immediately.
> > >
> > >
>
>
> CAUTION: This email originated from outside of the organization.
> Do not click on links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or
> confidential. If you are not the intended recipient, please delete the
> e-mail and any attachments and notify us immediately.
>
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi All!
Is there any additional feedback that the community can provide me on the KIP? Has anyone else run into requirements like this, or maybe my customer is the only one :)? If the scope looks good, is it time to call a vote? Or should I start porting my 2.0 code to 2.6 to show examples?
Thanks!
Connor
On 4/6/20, 9:03 AM, "Connor Penhale" <CP...@perforce.com> wrote:
Hi Colin,
We did not find a specific security vulnerability. Our customer had auditors in their environment, and they identified Kafka Connect as out of compliance with their particular standards, something that happens all the time for REST-based applications. What these security auditors expected Kafka Connect to be able to do was tune the response. As Kafka Connect could not provide this functionality, I'm proposing this KIP. Does that make sense? Should I still go through the process of a security disclosure?
Our particular need was around suppressing exceptions in the "public" response, as Kafka Connect was passing these exceptions without authentication, they became a public endpoint upon which the auditors could fuzz, and show it being out of compliance. Keeping these exceptions in the logs, as proposed in the KIP, makes sense to me as an operator.
I only mention PCI-DSS as this was the kind of environment my customer had that was making the request for being able to tune the response.
Thanks!
Connor
---
Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/)
Perforce (https://www.perforce.com/)
Support: +1 866.399.6736
On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
Also, if you do find a security issue, the process to follow is here: https://kafka.apache.org/project-security.html .
best,
Colin
On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> Hi Connor,
>
> If we are putting security-sensitive information into REST responses,
> that is a bug that needs to be fixed, not worked around with a
> configuration option. Do you have an example of security-sensitive
> information appearing in the exception text? Why do you feel that
> PCI-DSS requires this change?
>
> By the way, the same concern applies to log messages. We do not log
> sensitive information such as passwords to the log4j output. If you
> know of that happening somewhere, please file a bug so it can be fixed.
>
> best,
> Colin
>
>
> On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > Hi Chris!
> >
> > Thanks for your feedback! I'll number my responses to your questions / thoughts.
> >
> > 1. Apologies on that lack of clarity! I settled on "Detailed exception
> > information has been suppressed. Please see logs."
> > (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing.
> >
> > 2. I was unaware of the REST extensions! I'll see if I can implement
> > the same behavior as a REST extension. I agree that the KIP still has
> > merit, regardless of the feasibility of the extension, but in regards
> > to the 5th thought, this might make that decision easier.
> >
> > 3. I agree with your suggestion here. Absolutely ready to take the
> > community feedback on what makes sense here.
> >
> > 4. I should note that while I emphasized uncaught exceptions, I mean
> > all exceptions handled by the ExceptionMapper, including
> > ConnectRestExceptions. An example of this is here:
> > https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> >
> > 5. I didn't know how specific I should get if I had already taken a
> > stab at implementing! I'm happy to edit this in whatever way we want to
> > go about it.
> >
> > Let me know if anyone has any other questions or feedback!
> >
> >
> > Thanks!
> > Connor
> >
> > On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
> >
> > Hi Connor,
> >
> > Great stuff! I generally like being able to see the stack trace of an
> > exception directly via the REST API but can definitely understand the
> > security concerns here. I've got a few questions/remarks about the KIP and
> > would be interested in your thoughts:
> >
> > 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
> > outline what this message would actually be. It'd be great to see the
> > actual message in the KIP since people may have thoughts on what it should
> > be and want to comment on it as part of this discussion.
> >
> > 2. In the "Rejected Alternatives" section, an Nginx proxy is
> > mentioned as
> > one possible way to filter out stack traces from the REST API. It
> > seems
> > like a Connect REST extension (
> >
> > https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
> > would be a better alternative than an Nginx proxy; had you
> > considered
> > utilizing one? I still think this KIP is worthwhile and a REST
> > extension
> > shouldn't be necessary in order to lock down the REST API this way,
> > but it
> > might be worth calling out as an alternative and perhaps even a
> > workaround
> > in cases where users are stuck on a given version of Connect and
> > can't
> > upgrade to 2.6 (or whichever version this KIP lands on) any time
> > soon.
> >
> > 3. The "error.rest.response.message.detail.enabled" property is a bit of a
> > mouthful; it'd be great if we could come up with something more succinct.
> > What do you think about something like "rest.response.stack.traces"?
> >
> > 4. The KIP is targeted at stack traces for uncaught exceptions, but it's
> > also possible that stack traces get exposed in the REST API when querying
> > the status of a connector or one of its tasks. Was this intentional? If so,
> > it'd be great to call out why that kind of filtering is not required in the
> > "Rejected Alternatives" section, and if not, it's probably not too late to
> > consider modifying the KIP to cover those cases as well.
> >
> > 5. The KIP mentions creating a new, separate exception mapper class. This
> > seems like more of an implementation detail and something that can be
> > decided on during code review; unless it's critical to the functionality
> > that the KIP aims to accomplish, I'd suggest leaving that part out since it
> > shouldn't affect the impact on users of the Connect framework.
> >
> > Thanks for the KIP, looking forward to seeing this happen!
> >
> > Cheers,
> >
> > Chris
> >
> > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
> > wrote:
> >
> > > Hello All!
> > >
> > > I’ve created the following KIP:
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > >
> > > The PR that originated this discussion, is here:
> > > https://github.com/apache/kafka/pull/8355 It is based on 2.0,
> > but I
> > > would be working on Kafka Connect in 2.6 to get this behavior
> > changed to
> > > the community’s preference.
> > >
> > > Looking forward to working with everyone!
> > >
> > > Thanks!
> > > Connor
> > > ---
> > > Connor Penhale | Enterprise Architect, OpenLogic
> > (https://openlogic.com/)
> > > Perforce (https://www.perforce.com/)
> > > Support: +1 866.399.6736
> > >
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential. If
> > > you are not the intended recipient, please delete the e-mail and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization. Do
> > not click on links or open attachments unless you recognize the sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or confidential.
> > If you are not the intended recipient, please delete the e-mail and any
> > attachments and notify us immediately.
> >
> >
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Colin,
We did not find a specific security vulnerability. Our customer had auditors in their environment, and they identified Kafka Connect as out of compliance with their particular standards, something that happens all the time for REST-based applications. What these security auditors expected Kafka Connect to be able to do was tune the response. As Kafka Connect could not provide this functionality, I'm proposing this KIP. Does that make sense? Should I still go through the process of a security disclosure?
Our particular need was around suppressing exceptions in the "public" response, as Kafka Connect was passing these exceptions without authentication, they became a public endpoint upon which the auditors could fuzz, and show it being out of compliance. Keeping these exceptions in the logs, as proposed in the KIP, makes sense to me as an operator.
I only mention PCI-DSS as this was the kind of environment my customer had that was making the request for being able to tune the response.
Thanks!
Connor
---
Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/)
Perforce (https://www.perforce.com/)
Support: +1 866.399.6736
On 4/3/20, 3:24 PM, "Colin McCabe" <cm...@apache.org> wrote:
Also, if you do find a security issue, the process to follow is here: https://kafka.apache.org/project-security.html .
best,
Colin
On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> Hi Connor,
>
> If we are putting security-sensitive information into REST responses,
> that is a bug that needs to be fixed, not worked around with a
> configuration option. Do you have an example of security-sensitive
> information appearing in the exception text? Why do you feel that
> PCI-DSS requires this change?
>
> By the way, the same concern applies to log messages. We do not log
> sensitive information such as passwords to the log4j output. If you
> know of that happening somewhere, please file a bug so it can be fixed.
>
> best,
> Colin
>
>
> On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > Hi Chris!
> >
> > Thanks for your feedback! I'll number my responses to your questions / thoughts.
> >
> > 1. Apologies on that lack of clarity! I settled on "Detailed exception
> > information has been suppressed. Please see logs."
> > (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing.
> >
> > 2. I was unaware of the REST extensions! I'll see if I can implement
> > the same behavior as a REST extension. I agree that the KIP still has
> > merit, regardless of the feasibility of the extension, but in regards
> > to the 5th thought, this might make that decision easier.
> >
> > 3. I agree with your suggestion here. Absolutely ready to take the
> > community feedback on what makes sense here.
> >
> > 4. I should note that while I emphasized uncaught exceptions, I mean
> > all exceptions handled by the ExceptionMapper, including
> > ConnectRestExceptions. An example of this is here:
> > https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> >
> > 5. I didn't know how specific I should get if I had already taken a
> > stab at implementing! I'm happy to edit this in whatever way we want to
> > go about it.
> >
> > Let me know if anyone has any other questions or feedback!
> >
> >
> > Thanks!
> > Connor
> >
> > On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
> >
> > Hi Connor,
> >
> > Great stuff! I generally like being able to see the stack trace of an
> > exception directly via the REST API but can definitely understand the
> > security concerns here. I've got a few questions/remarks about the KIP and
> > would be interested in your thoughts:
> >
> > 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
> > outline what this message would actually be. It'd be great to see the
> > actual message in the KIP since people may have thoughts on what it should
> > be and want to comment on it as part of this discussion.
> >
> > 2. In the "Rejected Alternatives" section, an Nginx proxy is
> > mentioned as
> > one possible way to filter out stack traces from the REST API. It
> > seems
> > like a Connect REST extension (
> >
> > https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
> > would be a better alternative than an Nginx proxy; had you
> > considered
> > utilizing one? I still think this KIP is worthwhile and a REST
> > extension
> > shouldn't be necessary in order to lock down the REST API this way,
> > but it
> > might be worth calling out as an alternative and perhaps even a
> > workaround
> > in cases where users are stuck on a given version of Connect and
> > can't
> > upgrade to 2.6 (or whichever version this KIP lands on) any time
> > soon.
> >
> > 3. The "error.rest.response.message.detail.enabled" property is a bit of a
> > mouthful; it'd be great if we could come up with something more succinct.
> > What do you think about something like "rest.response.stack.traces"?
> >
> > 4. The KIP is targeted at stack traces for uncaught exceptions, but it's
> > also possible that stack traces get exposed in the REST API when querying
> > the status of a connector or one of its tasks. Was this intentional? If so,
> > it'd be great to call out why that kind of filtering is not required in the
> > "Rejected Alternatives" section, and if not, it's probably not too late to
> > consider modifying the KIP to cover those cases as well.
> >
> > 5. The KIP mentions creating a new, separate exception mapper class. This
> > seems like more of an implementation detail and something that can be
> > decided on during code review; unless it's critical to the functionality
> > that the KIP aims to accomplish, I'd suggest leaving that part out since it
> > shouldn't affect the impact on users of the Connect framework.
> >
> > Thanks for the KIP, looking forward to seeing this happen!
> >
> > Cheers,
> >
> > Chris
> >
> > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
> > wrote:
> >
> > > Hello All!
> > >
> > > I’ve created the following KIP:
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > >
> > > The PR that originated this discussion, is here:
> > > https://github.com/apache/kafka/pull/8355 It is based on 2.0,
> > but I
> > > would be working on Kafka Connect in 2.6 to get this behavior
> > changed to
> > > the community’s preference.
> > >
> > > Looking forward to working with everyone!
> > >
> > > Thanks!
> > > Connor
> > > ---
> > > Connor Penhale | Enterprise Architect, OpenLogic
> > (https://openlogic.com/)
> > > Perforce (https://www.perforce.com/)
> > > Support: +1 866.399.6736
> > >
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential. If
> > > you are not the intended recipient, please delete the e-mail and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization. Do
> > not click on links or open attachments unless you recognize the sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or confidential.
> > If you are not the intended recipient, please delete the e-mail and any
> > attachments and notify us immediately.
> >
> >
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled exceptions in security-sensitive environments
Posted by Colin McCabe <cm...@apache.org>.
Also, if you do find a security issue, the process to follow is here: https://kafka.apache.org/project-security.html .
best,
Colin
On Fri, Apr 3, 2020, at 14:20, Colin McCabe wrote:
> Hi Connor,
>
> If we are putting security-sensitive information into REST responses,
> that is a bug that needs to be fixed, not worked around with a
> configuration option. Do you have an example of security-sensitive
> information appearing in the exception text? Why do you feel that
> PCI-DSS requires this change?
>
> By the way, the same concern applies to log messages. We do not log
> sensitive information such as passwords to the log4j output. If you
> know of that happening somewhere, please file a bug so it can be fixed.
>
> best,
> Colin
>
>
> On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> > Hi Chris!
> >
> > Thanks for your feedback! I'll number my responses to your questions / thoughts.
> >
> > 1. Apologies on that lack of clarity! I settled on "Detailed exception
> > information has been suppressed. Please see logs."
> > (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing.
> >
> > 2. I was unaware of the REST extensions! I'll see if I can implement
> > the same behavior as a REST extension. I agree that the KIP still has
> > merit, regardless of the feasibility of the extension, but in regards
> > to the 5th thought, this might make that decision easier.
> >
> > 3. I agree with your suggestion here. Absolutely ready to take the
> > community feedback on what makes sense here.
> >
> > 4. I should note that while I emphasized uncaught exceptions, I mean
> > all exceptions handled by the ExceptionMapper, including
> > ConnectRestExceptions. An example of this is here:
> > https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
> >
> > 5. I didn't know how specific I should get if I had already taken a
> > stab at implementing! I'm happy to edit this in whatever way we want to
> > go about it.
> >
> > Let me know if anyone has any other questions or feedback!
> >
> >
> > Thanks!
> > Connor
> >
> > On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
> >
> > Hi Connor,
> >
> > Great stuff! I generally like being able to see the stack trace of an
> > exception directly via the REST API but can definitely understand the
> > security concerns here. I've got a few questions/remarks about the KIP and
> > would be interested in your thoughts:
> >
> > 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
> > outline what this message would actually be. It'd be great to see the
> > actual message in the KIP since people may have thoughts on what it should
> > be and want to comment on it as part of this discussion.
> >
> > 2. In the "Rejected Alternatives" section, an Nginx proxy is
> > mentioned as
> > one possible way to filter out stack traces from the REST API. It
> > seems
> > like a Connect REST extension (
> >
> > https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
> > would be a better alternative than an Nginx proxy; had you
> > considered
> > utilizing one? I still think this KIP is worthwhile and a REST
> > extension
> > shouldn't be necessary in order to lock down the REST API this way,
> > but it
> > might be worth calling out as an alternative and perhaps even a
> > workaround
> > in cases where users are stuck on a given version of Connect and
> > can't
> > upgrade to 2.6 (or whichever version this KIP lands on) any time
> > soon.
> >
> > 3. The "error.rest.response.message.detail.enabled" property is a bit of a
> > mouthful; it'd be great if we could come up with something more succinct.
> > What do you think about something like "rest.response.stack.traces"?
> >
> > 4. The KIP is targeted at stack traces for uncaught exceptions, but it's
> > also possible that stack traces get exposed in the REST API when querying
> > the status of a connector or one of its tasks. Was this intentional? If so,
> > it'd be great to call out why that kind of filtering is not required in the
> > "Rejected Alternatives" section, and if not, it's probably not too late to
> > consider modifying the KIP to cover those cases as well.
> >
> > 5. The KIP mentions creating a new, separate exception mapper class. This
> > seems like more of an implementation detail and something that can be
> > decided on during code review; unless it's critical to the functionality
> > that the KIP aims to accomplish, I'd suggest leaving that part out since it
> > shouldn't affect the impact on users of the Connect framework.
> >
> > Thanks for the KIP, looking forward to seeing this happen!
> >
> > Cheers,
> >
> > Chris
> >
> > On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
> > wrote:
> >
> > > Hello All!
> > >
> > > I’ve created the following KIP:
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> > >
> > > The PR that originated this discussion, is here:
> > > https://github.com/apache/kafka/pull/8355 It is based on 2.0,
> > but I
> > > would be working on Kafka Connect in 2.6 to get this behavior
> > changed to
> > > the community’s preference.
> > >
> > > Looking forward to working with everyone!
> > >
> > > Thanks!
> > > Connor
> > > ---
> > > Connor Penhale | Enterprise Architect, OpenLogic
> > (https://openlogic.com/)
> > > Perforce (https://www.perforce.com/)
> > > Support: +1 866.399.6736
> > >
> > >
> > >
> > >
> > > This e-mail may contain information that is privileged or
> > confidential. If
> > > you are not the intended recipient, please delete the e-mail and
> > any
> > > attachments and notify us immediately.
> > >
> > >
> >
> >
> > CAUTION: This email originated from outside of the organization. Do
> > not click on links or open attachments unless you recognize the sender
> > and know the content is safe.
> >
> >
> >
> > This e-mail may contain information that is privileged or confidential.
> > If you are not the intended recipient, please delete the e-mail and any
> > attachments and notify us immediately.
> >
> >
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled exceptions in security-sensitive environments
Posted by Colin McCabe <cm...@apache.org>.
Hi Connor,
If we are putting security-sensitive information into REST responses, that is a bug that needs to be fixed, not worked around with a configuration option. Do you have an example of security-sensitive information appearing in the exception text? Why do you feel that PCI-DSS requires this change?
By the way, the same concern applies to log messages. We do not log sensitive information such as passwords to the log4j output. If you know of that happening somewhere, please file a bug so it can be fixed.
best,
Colin
On Fri, Apr 3, 2020, at 12:56, Connor Penhale wrote:
> Hi Chris!
>
> Thanks for your feedback! I'll number my responses to your questions / thoughts.
>
> 1. Apologies on that lack of clarity! I settled on "Detailed exception
> information has been suppressed. Please see logs."
> (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing.
>
> 2. I was unaware of the REST extensions! I'll see if I can implement
> the same behavior as a REST extension. I agree that the KIP still has
> merit, regardless of the feasibility of the extension, but in regards
> to the 5th thought, this might make that decision easier.
>
> 3. I agree with your suggestion here. Absolutely ready to take the
> community feedback on what makes sense here.
>
> 4. I should note that while I emphasized uncaught exceptions, I mean
> all exceptions handled by the ExceptionMapper, including
> ConnectRestExceptions. An example of this is here:
> https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
>
> 5. I didn't know how specific I should get if I had already taken a
> stab at implementing! I'm happy to edit this in whatever way we want to
> go about it.
>
> Let me know if anyone has any other questions or feedback!
>
>
> Thanks!
> Connor
>
> On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
>
> Hi Connor,
>
> Great stuff! I generally like being able to see the stack trace of an
> exception directly via the REST API but can definitely understand the
> security concerns here. I've got a few questions/remarks about the KIP and
> would be interested in your thoughts:
>
> 1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
> outline what this message would actually be. It'd be great to see the
> actual message in the KIP since people may have thoughts on what it should
> be and want to comment on it as part of this discussion.
>
> 2. In the "Rejected Alternatives" section, an Nginx proxy is
> mentioned as
> one possible way to filter out stack traces from the REST API. It
> seems
> like a Connect REST extension (
>
> https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
> would be a better alternative than an Nginx proxy; had you
> considered
> utilizing one? I still think this KIP is worthwhile and a REST
> extension
> shouldn't be necessary in order to lock down the REST API this way,
> but it
> might be worth calling out as an alternative and perhaps even a
> workaround
> in cases where users are stuck on a given version of Connect and
> can't
> upgrade to 2.6 (or whichever version this KIP lands on) any time
> soon.
>
> 3. The "error.rest.response.message.detail.enabled" property is a bit of a
> mouthful; it'd be great if we could come up with something more succinct.
> What do you think about something like "rest.response.stack.traces"?
>
> 4. The KIP is targeted at stack traces for uncaught exceptions, but it's
> also possible that stack traces get exposed in the REST API when querying
> the status of a connector or one of its tasks. Was this intentional? If so,
> it'd be great to call out why that kind of filtering is not required in the
> "Rejected Alternatives" section, and if not, it's probably not too late to
> consider modifying the KIP to cover those cases as well.
>
> 5. The KIP mentions creating a new, separate exception mapper class. This
> seems like more of an implementation detail and something that can be
> decided on during code review; unless it's critical to the functionality
> that the KIP aims to accomplish, I'd suggest leaving that part out since it
> shouldn't affect the impact on users of the Connect framework.
>
> Thanks for the KIP, looking forward to seeing this happen!
>
> Cheers,
>
> Chris
>
> On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
> wrote:
>
> > Hello All!
> >
> > I’ve created the following KIP:
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
> >
> > The PR that originated this discussion, is here:
> > https://github.com/apache/kafka/pull/8355 It is based on 2.0,
> but I
> > would be working on Kafka Connect in 2.6 to get this behavior
> changed to
> > the community’s preference.
> >
> > Looking forward to working with everyone!
> >
> > Thanks!
> > Connor
> > ---
> > Connor Penhale | Enterprise Architect, OpenLogic
> (https://openlogic.com/)
> > Perforce (https://www.perforce.com/)
> > Support: +1 866.399.6736
> >
> >
> >
> >
> > This e-mail may contain information that is privileged or
> confidential. If
> > you are not the intended recipient, please delete the e-mail and
> any
> > attachments and notify us immediately.
> >
> >
>
>
> CAUTION: This email originated from outside of the organization. Do
> not click on links or open attachments unless you recognize the sender
> and know the content is safe.
>
>
>
> This e-mail may contain information that is privileged or confidential.
> If you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Connor Penhale <CP...@perforce.com>.
Hi Chris!
Thanks for your feedback! I'll number my responses to your questions / thoughts.
1. Apologies on that lack of clarity! I settled on "Detailed exception information has been suppressed. Please see logs." (https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R34). Should I update the KIP to reflect what I've already thought about? It's my first one, not sure what the process should be for editing.
2. I was unaware of the REST extensions! I'll see if I can implement the same behavior as a REST extension. I agree that the KIP still has merit, regardless of the feasibility of the extension, but in regards to the 5th thought, this might make that decision easier.
3. I agree with your suggestion here. Absolutely ready to take the community feedback on what makes sense here.
4. I should note that while I emphasized uncaught exceptions, I mean all exceptions handled by the ExceptionMapper, including ConnectRestExceptions. An example of this is here: https://github.com/apache/kafka/pull/8355/files#diff-64c265986e7bbe40cdd79f831e961907R46
5. I didn't know how specific I should get if I had already taken a stab at implementing! I'm happy to edit this in whatever way we want to go about it.
Let me know if anyone has any other questions or feedback!
Thanks!
Connor
On 4/2/20, 3:58 PM, "Christopher Egerton" <ch...@confluent.io> wrote:
Hi Connor,
Great stuff! I generally like being able to see the stack trace of an
exception directly via the REST API but can definitely understand the
security concerns here. I've got a few questions/remarks about the KIP and
would be interested in your thoughts:
1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
outline what this message would actually be. It'd be great to see the
actual message in the KIP since people may have thoughts on what it should
be and want to comment on it as part of this discussion.
2. In the "Rejected Alternatives" section, an Nginx proxy is mentioned as
one possible way to filter out stack traces from the REST API. It seems
like a Connect REST extension (
https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
would be a better alternative than an Nginx proxy; had you considered
utilizing one? I still think this KIP is worthwhile and a REST extension
shouldn't be necessary in order to lock down the REST API this way, but it
might be worth calling out as an alternative and perhaps even a workaround
in cases where users are stuck on a given version of Connect and can't
upgrade to 2.6 (or whichever version this KIP lands on) any time soon.
3. The "error.rest.response.message.detail.enabled" property is a bit of a
mouthful; it'd be great if we could come up with something more succinct.
What do you think about something like "rest.response.stack.traces"?
4. The KIP is targeted at stack traces for uncaught exceptions, but it's
also possible that stack traces get exposed in the REST API when querying
the status of a connector or one of its tasks. Was this intentional? If so,
it'd be great to call out why that kind of filtering is not required in the
"Rejected Alternatives" section, and if not, it's probably not too late to
consider modifying the KIP to cover those cases as well.
5. The KIP mentions creating a new, separate exception mapper class. This
seems like more of an implementation detail and something that can be
decided on during code review; unless it's critical to the functionality
that the KIP aims to accomplish, I'd suggest leaving that part out since it
shouldn't affect the impact on users of the Connect framework.
Thanks for the KIP, looking forward to seeing this happen!
Cheers,
Chris
On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
wrote:
> Hello All!
>
> I’ve created the following KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
>
> The PR that originated this discussion, is here:
> https://github.com/apache/kafka/pull/8355 It is based on 2.0, but I
> would be working on Kafka Connect in 2.6 to get this behavior changed to
> the community’s preference.
>
> Looking forward to working with everyone!
>
> Thanks!
> Connor
> ---
> Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/)
> Perforce (https://www.perforce.com/)
> Support: +1 866.399.6736
>
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>
CAUTION: This email originated from outside of the organization. Do not click on links or open attachments unless you recognize the sender and know the content is safe.
This e-mail may contain information that is privileged or confidential. If you are not the intended recipient, please delete the e-mail and any attachments and notify us immediately.
Re: [DISCUSS] KIP-587 Suppress detailed responses for handled
exceptions in security-sensitive environments
Posted by Christopher Egerton <ch...@confluent.io>.
Hi Connor,
Great stuff! I generally like being able to see the stack trace of an
exception directly via the REST API but can definitely understand the
security concerns here. I've got a few questions/remarks about the KIP and
would be interested in your thoughts:
1. The KIP mentions a SUPRESSED_EXCEPTION_MESSAGE, but doesn't actually
outline what this message would actually be. It'd be great to see the
actual message in the KIP since people may have thoughts on what it should
be and want to comment on it as part of this discussion.
2. In the "Rejected Alternatives" section, an Nginx proxy is mentioned as
one possible way to filter out stack traces from the REST API. It seems
like a Connect REST extension (
https://kafka.apache.org/24/javadoc/index.html?org/apache/kafka/connect/rest/ConnectRestExtension.html)
would be a better alternative than an Nginx proxy; had you considered
utilizing one? I still think this KIP is worthwhile and a REST extension
shouldn't be necessary in order to lock down the REST API this way, but it
might be worth calling out as an alternative and perhaps even a workaround
in cases where users are stuck on a given version of Connect and can't
upgrade to 2.6 (or whichever version this KIP lands on) any time soon.
3. The "error.rest.response.message.detail.enabled" property is a bit of a
mouthful; it'd be great if we could come up with something more succinct.
What do you think about something like "rest.response.stack.traces"?
4. The KIP is targeted at stack traces for uncaught exceptions, but it's
also possible that stack traces get exposed in the REST API when querying
the status of a connector or one of its tasks. Was this intentional? If so,
it'd be great to call out why that kind of filtering is not required in the
"Rejected Alternatives" section, and if not, it's probably not too late to
consider modifying the KIP to cover those cases as well.
5. The KIP mentions creating a new, separate exception mapper class. This
seems like more of an implementation detail and something that can be
decided on during code review; unless it's critical to the functionality
that the KIP aims to accomplish, I'd suggest leaving that part out since it
shouldn't affect the impact on users of the Connect framework.
Thanks for the KIP, looking forward to seeing this happen!
Cheers,
Chris
On Thu, Apr 2, 2020 at 11:01 AM Connor Penhale <CP...@perforce.com>
wrote:
> Hello All!
>
> I’ve created the following KIP:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-587:+Suppress+detailed+responses+for+handled+exceptions+in+security-sensitive+environments
>
> The PR that originated this discussion, is here:
> https://github.com/apache/kafka/pull/8355 It is based on 2.0, but I
> would be working on Kafka Connect in 2.6 to get this behavior changed to
> the community’s preference.
>
> Looking forward to working with everyone!
>
> Thanks!
> Connor
> ---
> Connor Penhale | Enterprise Architect, OpenLogic (https://openlogic.com/)
> Perforce (https://www.perforce.com/)
> Support: +1 866.399.6736
>
>
>
>
> This e-mail may contain information that is privileged or confidential. If
> you are not the intended recipient, please delete the e-mail and any
> attachments and notify us immediately.
>
>