Skip to content

Clarify meaning of sensitive header values#427

Merged
seanmonstar merged 1 commit intohyperium:masterfrom
pluehne:clarify-meaning-of-sensitive-header-values
Jun 1, 2020
Merged

Clarify meaning of sensitive header values#427
seanmonstar merged 1 commit intohyperium:masterfrom
pluehne:clarify-meaning-of-sensitive-header-values

Conversation

@pluehne
Copy link
Contributor

@pluehne pluehne commented May 30, 2020

While the documentation mentions that header values can be marked as sensitive in order to inform outside components that these header values may require special treatment, it was unclear to me whether doing that affects the behavior of this crate.

In fact, the only place in this crate that makes a distinction between regular and sensitive header values is the Debug trait implementation of HeaderValue, which masks sensitive values in the output.

This adds a short note to the documentation to clarify this and to avoid that users think that there might be, for example, a security benefit on the protocol level.

@seanmonstar
Copy link
Member

There is a protocol level benefit, it's just not used in this crate because it doesn't implement a client or server.

But the h2 crate uses the sensitive flag to signal the header value shouldn't be saved in the hpack dynamic table, as one example.

@pluehne
Copy link
Contributor Author

pluehne commented May 30, 2020

Thanks for clarifying, @seanmonstar! I guess I should have researched more.

I’m still thinking how the documentation could be improved to make it a bit clearer what exactly the benefit of marking header values as sensitive is. I was thinking of something like this:

Sensitive data could represent passwords or other data that should not be stored on disk or in memory. By marking header values as sensitive, components using this crate can be instructed to treat them with special care for security reasons. For example, caches can avoid storing sensitive values, and HPACK encoders used by HTTP/2.0 implementations can choose not to compress them.

Note that sensitivity is not factored into equality or ordering.

I’m not sure it’s a big improvement, but I think it would have made the security benefits of the sensitivity flag more obvious to me. But if you think this isn’t worth changing, I totally wouldn’t mind closing this pull request 🙂.

@seanmonstar
Copy link
Member

What you just proposed sounds great to me!

While the documentation mentions that header values can be marked as
sensitive in order to inform outside components that these header values
may require special treatment, it was unclear to me whether doing that
affects the behavior of this crate.

This adds a short note to the documentation to clarify that the main
purpose of the sensitivity flag is to make components building on this
crate to be aware of sensitive data, such that it can be treated with
special care for security purposes.
@pluehne pluehne force-pushed the clarify-meaning-of-sensitive-header-values branch from c086f17 to a45f2a7 Compare May 31, 2020 14:53
@pluehne
Copy link
Contributor Author

pluehne commented May 31, 2020

Thanks for the feedback 👍! I made the changes I suggested above and updated the commit message.

I thought it couldn’t hurt to point out that the Debug trait implementation masks sensitive values—as long as I don’t make it sound that this is that flag’s main purpose—so I added back a sentence about that.

@seanmonstar seanmonstar merged commit 4115fc1 into hyperium:master Jun 1, 2020
BenxiangGe pushed a commit to BenxiangGe/http that referenced this pull request Jul 26, 2021
While the documentation mentions that header values can be marked as
sensitive in order to inform outside components that these header values
may require special treatment, it was unclear to me whether doing that
affects the behavior of this crate.

This adds a short note to the documentation to clarify that the main
purpose of the sensitivity flag is to make components building on this
crate to be aware of sensitive data, such that it can be treated with
special care for security purposes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants