Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-10 Thread Sai Prakash Ranjan

Hi Andi,

On 2021-03-09 20:14, Andi Kleen wrote:
The disk encryption is just one example and there might be others 
which
we might not be aware of yet and we are not suspecting there is 
something

wrong with the crypto code that needs to be fixed.


Then you don't have any leaks relating to branch tracing.

restrict an external(in the sense that its not related to crypto or 
any
other security related component) entity such as hardware assisted 
tracing
like ARM coresight and so on. I don't see why or how the crypto code 
needs
to be fixed for something that is not related to it although it is 
affected.


It's just a general property that if some code that is handling secrets
is data dependent it already leaks.


The analogy would be like of the victims and a perpetrator. Lets take 
coresight
as an example for perpetrator and crypto as the victim here. Now we 
can try


There's no victim with branch tracing, unless it is already leaky.

If we just know one victim (lets say crypto code here), what happens 
to the
others which we haven't identified yet? Do we just wait for someone to 
write

an exploit based on this and then scramble to fix it?


For a useful security mitigation you need a threat model first I would 
say.


So you need to have at least some idea how an attack with branch
tracing would work.


Initial change was to restrict this only to HW assisted instruction 
tracing [1]


I don't think it's needed for instruction tracing.



From what I know, newer ARM A-profile cores doesn't allow data tracing. 
And you

are saying that just the instruction tracing cannot be used to infer any
important data.

There are few security folks in CC who probably can give us more details 
on how

branch tracing can be used for an exploit. @mnissler?

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member

of Code Aurora Forum, hosted by The Linux Foundation


Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-09 Thread Andi Kleen
> The disk encryption is just one example and there might be others which
> we might not be aware of yet and we are not suspecting there is something
> wrong with the crypto code that needs to be fixed.

Then you don't have any leaks relating to branch tracing.

> restrict an external(in the sense that its not related to crypto or any
> other security related component) entity such as hardware assisted tracing
> like ARM coresight and so on. I don't see why or how the crypto code needs
> to be fixed for something that is not related to it although it is affected.

It's just a general property that if some code that is handling secrets
is data dependent it already leaks.


> The analogy would be like of the victims and a perpetrator. Lets take 
> coresight
> as an example for perpetrator and crypto as the victim here. Now we can try

There's no victim with branch tracing, unless it is already leaky.

> If we just know one victim (lets say crypto code here), what happens to the
> others which we haven't identified yet? Do we just wait for someone to write
> an exploit based on this and then scramble to fix it?

For a useful security mitigation you need a threat model first I would say.

So you need to have at least some idea how an attack with branch
tracing would work.


> Initial change was to restrict this only to HW assisted instruction tracing 
> [1]

I don't think it's needed for instruction tracing.

-Andi


Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-08 Thread Sai Prakash Ranjan
Hi Andi,

On 2021-03-05 01:47, Andi Kleen wrote:
> Andi Kleen  writes:
>>
>> Normally disk encryption is in specialized work queues. It's total
>> overkill to restrict all of the kernel if you just want to restrict
>> those work queues.
>>
>> I would suggest some more analysis where secrets are actually stored
>> and handled first.
> 
> Also thinking about this more:
> 
> You really only want to limit data tracing here.
> 
> If tracing branches could reveal secrets the crypto code would be
> already insecure due to timing side channels. If that's the
> case it would already require fixing the crypto code.
> 

The disk encryption is just one example and there might be others which
we might not be aware of yet and we are not suspecting there is something
wrong with the crypto code that needs to be fixed. Here the idea was to
restrict an external(in the sense that its not related to crypto or any
other security related component) entity such as hardware assisted tracing
like ARM coresight and so on. I don't see why or how the crypto code needs
to be fixed for something that is not related to it although it is affected.

The analogy would be like of the victims and a perpetrator. Lets take coresight
as an example for perpetrator and crypto as the victim here. Now we can try
to harden the protection and safeguard the victims which may or may not be
successful but it will be possible only if we know the victims beforehand.
If we just know one victim (lets say crypto code here), what happens to the
others which we haven't identified yet? Do we just wait for someone to write
an exploit based on this and then scramble to fix it?

Now the other foolproof way of saving the victims would be locking down the
perpetrator which would definitely save all the victims but that needs the
perpetrator to be identified. In our case, the perpetrator (coresight or any
other hw tracing) is already known, so we want to lock it down or restrict it
so that if there is actually a vulnerability in crypto or other areas, then
this HW assisted tracing wouldn't be able to help exploit it.

Initial change was to restrict this only to HW assisted instruction tracing [1]
but Peter wasn't convinced that this applies to only instruction tracing. Hence
this change for all kernel level pmu tracing. And this is a configurable option
via kernel config so that we don't force it on everyone. This config is also
required for coresight etm drivers because they have a sysfs mode of tracing as
well in addition to perf mode.

[1] 
https://lore.kernel.org/lkml/cover.1611909025.git.saiprakash.ran...@codeaurora.org/

Thanks,
Sai
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-04 Thread Andi Kleen
Andi Kleen  writes:
>
> Normally disk encryption is in specialized work queues. It's total
> overkill to restrict all of the kernel if you just want to restrict
> those work queues.
>
> I would suggest some more analysis where secrets are actually stored
> and handled first.

Also thinking about this more:

You really only want to limit data tracing here.

If tracing branches could reveal secrets the crypto code would be
already insecure due to timing side channels. If that's the
case it would already require fixing the crypto code.

-Andi


Re: [PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-04 Thread Andi Kleen
Sai Prakash Ranjan  writes:
>
> "Consider a system where disk contents are encrypted and the encryption
> key is set up by the user when mounting the file system. From that point
> on the encryption key resides in the kernel. It seems reasonable to
> expect that the disk encryption key be protected from exfiltration even
> if the system later suffers a root compromise (or even against insiders
> that have root access), at least as long as the attacker doesn't
> manage to compromise the kernel."

Normally disk encryption is in specialized work queues. It's total
overkill to restrict all of the kernel if you just want to restrict
those work queues.

I would suggest some more analysis where secrets are actually stored
and handled first.

-Andi


[PATCHv2 0/4] perf/core: Add support to exclude kernel mode PMU tracing

2021-03-01 Thread Sai Prakash Ranjan
Hardware assisted tracing families such as ARM Coresight, Intel PT
provides rich tracing capabilities including instruction level
tracing and accurate timestamps which are very useful for profiling
and also pose a significant security risk. One such example of
security risk is when kernel mode tracing is not excluded and these
hardware assisted tracing can be used to analyze cryptographic code
execution. In this case, even the root user must not be able to infer
anything.

To explain it more clearly in the words of a security team member
(credits: Mattias Nissler),

"Consider a system where disk contents are encrypted and the encryption
key is set up by the user when mounting the file system. From that point
on the encryption key resides in the kernel. It seems reasonable to
expect that the disk encryption key be protected from exfiltration even
if the system later suffers a root compromise (or even against insiders
that have root access), at least as long as the attacker doesn't
manage to compromise the kernel."

Here the idea is to protect such important information from all users
including root users since root privileges does not have to mean full
control over the kernel [1] and root compromise does not have to be
the end of the world.

But "Peter said even the regular counters can be used for full branch trace,
the information isn't as accurate as PT and friends and not easier but
is good enough to infer plenty". This would mean that a global tunable
config for all kernel mode pmu tracing is more appropriate than the one
targeting the hardware assisted instruction tracing.

Currently we can exclude kernel mode tracing via perf_event_paranoid
sysctl but it has following limitations,

 * No option to restrict kernel mode instruction tracing by the
   root user.
 * Not possible to restrict kernel mode instruction tracing when the
   hardware assisted tracing IPs like ARM Coresight ETMs use an
   additional interface via sysfs for tracing in addition to perf
   interface.

So introduce a new config CONFIG_EXCLUDE_KERNEL_PMU_TRACE to exclude
kernel mode pmu tracing for all users including root which will be
generic and applicable to all hardware tracing families and which
can also be used with other interfaces like sysfs in case of ETMs.

Patch 1 adds this new config and the support in perf core to exclude
all kernel mode PMU tracing.

Patch 2 adds the perf evsel warning message when the perf tool users
attempt to perform a kernel mode trace with the config enabled to
exclude the kernel mode tracing.

Patch 3 and Patch 4 adds the support for excluding kernel mode for
ARM Coresight ETM{4,3}XX sysfs mode using the newly introduced generic
config.

[1] https://lwn.net/Articles/796866/

Changes in v2:
 * Move from kernel mode instruction tracing to all kernel level PMU tracing 
(Peter)
 * Move the check and warning to the caller mode_store() (Doug)

Sai Prakash Ranjan (4):
  perf/core: Add support to exclude kernel mode PMU tracing
  perf evsel: Print warning for excluding kernel mode instruction
tracing
  coresight: etm4x: Add support to exclude kernel mode tracing
  coresight: etm3x: Add support to exclude kernel mode tracing

 drivers/hwtracing/coresight/coresight-etm3x-core.c  |  3 +++
 drivers/hwtracing/coresight/coresight-etm3x-sysfs.c |  6 ++
 drivers/hwtracing/coresight/coresight-etm4x-core.c  |  6 +-
 drivers/hwtracing/coresight/coresight-etm4x-sysfs.c |  6 ++
 init/Kconfig| 11 +++
 kernel/events/core.c|  3 +++
 tools/perf/util/evsel.c |  3 ++-
 7 files changed, 36 insertions(+), 2 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation