search

hyperium / http

Rust HTTP types
Apache License 2.0
1.16k stars 291 forks source link

Clarify meaning of sensitive header values #427

Closed pluehne closed 4 years ago

pluehne commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 :slightly_smiling_face:.

seanmonstar commented 4 years ago

What you just proposed sounds great to me!

pluehne commented 4 years ago

Thanks for the feedback :+1:! 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.