feat(server): Add initial minidump scrubbing #682

flub · 2020-07-28T10:51:00Z

Implements initial data scrubbing of UTF-8 strings in heap and stack memory streams in minidumps. All sections are marked as pii = "maybe", which makes it effectively disabled for current PII rules.

flub · 2020-07-30T16:25:29Z

examples/edit_minidump.rs

+        changed |= pii_processor.scrub_attachment_bytes(
+            &filename,
+            data.desc_mut_slice(&raw_desc),
+            AttachmentBytesType::PlainAttachment,
+        );


@untitaker this is extremely untested, it just compiles. The format of these is just a bunch of bytes, which are "unix string" (aka any corrupted encoding possible) separated by NULL bytes.

I wonder whether we can get the locale settings out of the minidump to figure out how to decode paths, env, and whatever this is.

Possibly we can find locale settings in the env, so we could try to lossy-decode the env as ascii first, parse out locale, then parse everything (except the memory) as that encoding.

Yes, locale can normally be parsed out of the environ and used to parse out the encoding (my past self seems to think https://hg.sr.ht/~flub/syskit/browse/syskit/_utils.py?rev=tip#L109 is the way to get this, there's probably rust libs though).

Doing this gets back to how to reverse an encoding to modify the minidump though.

flub · 2020-08-17T15:32:04Z

@untitaker @jan-auer would you mind another review mainly looking at:

how this is integrated in relay. I'm not sure this is the right place or done in the right way (also, the interface between the two can be done better)
where the code should live. I've created a new crate as that was suggested sometime long ago, but it's a bit of a pain to keep the shared deps in sync. Though in some way having all the processing-feature functionality in a crate seems appealing to me and I don't think that exists yet. Anyway, if a crate is the way to go i should obviously do this correctly with making it optional and correctly pulled in byt the processing feature.

comments on relay-minidump/src/lib.rs are also welcome, though less necessary currently. I've left the example untouched for now, but the idea is that it would also use this and becomes a thing cli wrapper only.

This also zeroes out the stack, which is entirely undesirable.

This also removes some of the manual work to read the minidump as the existing API does actually expose enough to get to all the information.

This also puts some indirection in place and documents things a bit more.

This is pretty basic, but works.

Move regex mapping into separate module. We need to share this logic with attachment scrubbing once it's there. Also change usage of `Option<BTreeSet<u8>>` into dedicated enum.

Also refactor things a bit, use MINIDUMP_LOCATION_DESCRIPTOR instead of our own custom types. Add a trait to make it's slicing easy.

This needs access also to the raw data to do pointer arithmetic. So this refactors this a bit to have one struct which contains both the Minidump struct and the raw data.

flub · 2020-08-18T16:18:50Z

Ok, this is not as finished as I'd like. Current state:

Theres a dependency mistake somewhere. Invoking just cargo build fails but cargo build --workspaces --all-features does work (which is what I usually do.
There are some failing integration tests about the minidumps, I think this is an actual bug that should be found
(minor) The example doesn't use the new module yet

* master: fix(danger): Comment only on missing changelogs (#723) test: Require any python3 and support python3.8 (#722)

* master: feat(server): Support chunked formdata for Crashpad (#721)

jan-auer · 2020-08-20T13:54:24Z

@untitaker Can you confirm that this will only scrub minidumps if explicitly configured?

untitaker

This looks fine but we need to figure out how to deploy this without enabling. Right now this will match on selectors * and **, and the former cannot be solved with selector specificness at the moment. Also if somebody were to have configured a selector named 'UE4Minidump.dmp', this would (and should) apply as far as I understand.

I think the sanest way forward would be to have a featureflag inside of the projectconfig that is attached to a featureflag in Sentry.

* master: build(workspace): Move relay binary to workspace crate (#733) fix: Fix data schemas github action (#730) fix(docs): Incorrect init command in README (#732) fix(getting-started): Fix documentation links to other pages (#729) feat(server): Add initial minidump scrubbing (#682) fix: Remove jsonschema docs (#727) fix(docs): Fix a broken link test(server): Add unit tests to tracked future (#725) feat(server): Support chunked formdata for Crashpad (#721) fix(danger): Comment only on missing changelogs (#723) test: Require any python3 and support python3.8 (#722) fix: Re-add pii=maybe to span databags (#720) fix(schema): Type out span data and tags (#713)

flub force-pushed the feat/minidump-edit-example branch from b38fbe4 to 3366e0f Compare July 28, 2020 14:56

untitaker mentioned this pull request Jul 30, 2020

WIP: Basic raw attachment scrubbing #687

Closed

flub changed the title ~~Extremely rough and inefficient version of zeroing out memory~~ Minidump scrubbing experiment Jul 30, 2020

flub commented Jul 30, 2020

View reviewed changes

Floris Bruynooghe and others added 24 commits August 18, 2020 17:58

Extremely rough and inefficient version of zeroing out memory

de7fcd7

This also zeroes out the stack, which is entirely undesirable.

Only zero-out non-stack memory regions

ffe7564

This also removes some of the manual work to read the minidump as the existing API does actually expose enough to get to all the information.

Allow access to both stack and non-stack memory

efda7e7

This also puts some indirection in place and documents things a bit more.

First step in editing code names

5174e67

This is pretty basic, but works.

ref(pii): Split out regex mapping into separate module

c3ff360

Move regex mapping into separate module. We need to share this logic with attachment scrubbing once it's there. Also change usage of `Option<BTreeSet<u8>>` into dedicated enum.

remove leftover binary type

9f6a337

ref(pii): Split out regex mapping into separate module

cedcb8f

Move regex mapping into separate module. We need to share this logic with attachment scrubbing once it's there. Also change usage of `Option<BTreeSet<u8>>` into dedicated enum.

initial work on attachment scrubbing

4f261cc

initial work

8860913

wip

9dee99c

wip

cdc5b85

fix compilation

fe0ba5c

finish implementation and add more value types

444665e

wip testing

0d8b11e

remove valuetype inheritance and fix up tests

b67ae77

wire up minidump example with pii processor and fix some bugs

868d310

add testcase for bytes regexes

a120626

fix formatting

1505c21

Expose the linux environment

edb06dd

Also refactor things a bit, use MINIDUMP_LOCATION_DESCRIPTOR instead of our own custom types. Add a trait to make it's slicing easy.

Use this new trait in mem_desc as well

55b9ed1

Start integrating minidump scrubbing in relay

70d416a

Add LinuxCmd and LinuxEnviron scrubbing

52d3ce1

This needs access also to the raw data to do pointer arithmetic. So this refactors this a bit to have one struct which contains both the Minidump struct and the raw data.

Move things and rebase

79a19b6

Fix up Cargo.toml and actually add minidump module

0f1e034

flub force-pushed the feat/minidump-edit-example branch from 5baa975 to 0f1e034 Compare August 18, 2020 16:16

jan-auer changed the title ~~Minidump scrubbing experiment~~ feat(server): Add initial minidump scrubbing Aug 19, 2020

jan-auer added 3 commits August 19, 2020 13:01

Merge branch 'master' into feat/minidump-edit-example

05d370f

* master: fix(danger): Comment only on missing changelogs (#723) test: Require any python3 and support python3.8 (#722)

ref: Clean up implementation

d0f9ee3

ref: Move utility into tools folder

5bb1138

jan-auer approved these changes Aug 20, 2020

View reviewed changes

jan-auer added 3 commits August 20, 2020 15:25

Merge branch 'master' into feat/minidump-edit-example

b0e534c

* master: feat(server): Support chunked formdata for Crashpad (#721)

meta: Changelog

f630886

build: Fix docker build

2157fe1

jan-auer marked this pull request as ready for review August 20, 2020 13:35

jan-auer requested a review from a team August 20, 2020 13:35

jan-auer approved these changes Aug 20, 2020

View reviewed changes

untitaker requested changes Aug 20, 2020

View reviewed changes

jan-auer added 4 commits August 21, 2020 13:37

ref: Mark minidumps and attachments as pii=maybe

bc0b4c6

ref: Better error messages

c507a5b

fix: Clippy suggests static over const

9649ced

ref: Format

a22a568

jan-auer requested a review from untitaker August 24, 2020 07:32

jan-auer merged commit bbb063c into master Aug 24, 2020

jan-auer deleted the feat/minidump-edit-example branch August 24, 2020 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(server): Add initial minidump scrubbing #682

feat(server): Add initial minidump scrubbing #682

flub commented Jul 28, 2020 •

edited by jan-auer

Loading

flub Jul 30, 2020

untitaker Jul 30, 2020 •

edited

Loading

flub Jul 31, 2020

flub commented Aug 17, 2020

flub commented Aug 18, 2020

jan-auer commented Aug 20, 2020

untitaker left a comment

feat(server): Add initial minidump scrubbing #682

feat(server): Add initial minidump scrubbing #682

Conversation

flub commented Jul 28, 2020 • edited by jan-auer Loading

flub Jul 30, 2020

Choose a reason for hiding this comment

untitaker Jul 30, 2020 • edited Loading

Choose a reason for hiding this comment

flub Jul 31, 2020

Choose a reason for hiding this comment

flub commented Aug 17, 2020

flub commented Aug 18, 2020

jan-auer commented Aug 20, 2020

untitaker left a comment

Choose a reason for hiding this comment

flub commented Jul 28, 2020 •

edited by jan-auer

Loading

untitaker Jul 30, 2020 •

edited

Loading