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" <chr...@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 <cpenh...@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" <cpenh...@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" <cmcc...@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"
<chr...@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 <
> cpenh...@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.
