w3c / wot-profile

Web of Things (WoT) Profile
http://w3c.github.io/wot-profile/
Other
16 stars 8 forks source link

Recommended Security #6

Open mmccool opened 5 years ago

mmccool commented 5 years ago

In section 4.1.9.1 5.4, Recommended Practice for Security, it currently states

"When using the "no security" or "Basic Auth" security schemes it is strongly recommended to use transport layer encryption."

This should instead state, normatively:

"When using the "Basic Auth" or "Digest Auth" security schemes it is REQUIRED to use transport layer encryption. For all other schemes use of transport layer encryption is RECOMMENDED."

This is because Basic and Digest Auth are basically useless without transport encryption, they are designed to be used with it: they basically transmit passwords in the clear otherwise. Worse, using these without encryption gives people a false sense of confidence.

I will have to look at the other schemes, but in general we should say encryption is at least RECOMMENDED for all of them.

For "nosec" while you are not controlling access you might still want to control confidentiality with transport security, but on balance I think for this we can leave it up to the user; this mode should however be primarily used only for development. I think it's probably fine to lump it under RECOMMENDED.

Note however that using transport encryption with HTTP in systems not connected to the internet can be a bit troublesome, but oh, well.

benfrancis commented 3 years ago

Adding a note here based on feedback on https://github.com/w3c/wot-profile/pull/78 that the final list of mandatory security schemes for Core Profile compliant Consumers should be reviewed based on implementation experience.

My view is that this list should be kept relatively short. I advocate for NoSecurityScheme and OAuth2SecurityScheme being on the list.

benfrancis commented 2 years ago

See also: #220 and #221.

mmccool commented 1 year ago

Note: I am fine with removing Digest. It's not any better than basic when TLS is used, basic is easier to implement, and without TLS digest is not secure, either.

mmccool commented 1 year ago

Discussion in Security TF 2023.02.13:

mmccool commented 1 year ago

Actions (generation PR assigned to @lu-zero):

  1. Move section under HTTP Basic.
  2. Modify assertions to ONLY allow the specified schemes.
  3. Add assertions disallowing non-default values for "in" for Basic (e.g. mandating "header").
  4. Add an informative statement about secure transport (referencing the assertions in Architecture which should be sufficient).
benfrancis commented 1 year ago

Actions (generation PR assigned to @lu-zero):

  1. Move section under HTTP Basic.

Note that the HTTP SSE Profile and HTTP Webhook Profile do not depend on the HTTP Basic Profile, they can be used independently. That means that if you move the security assertions from Common Constraints to the HTTP Basic Profile, they will no longer apply to the other two profiles. You would instead have to duplicate the text in all three profiles or refer to the section of the HTTP Basic Profile, which then makes it another Common Constraints section.

Since this version of the specification only defines HTTP-based profiles, I see no need to make this change.

  1. Modify assertions to ONLY allow the specified schemes.

I suspect what you really mean is that a Web Thing MUST support at least one of these security schemes (including NoSecurityScheme). Restricting Web Things to only support the specified schemes prevents them from supporting other schemes in addition (which should be allowed), and may prevent a Thing from conforming to multiple profiles in the future.

  1. Add assertions disallowing non-default values for "in" for Basic (e.g. mandating "header").

Please be careful here. There are some browser APIs (notably WebSocket and EventSource) which don't support setting an Authorization header and which force Consumers to include credentials in the URL. As a minimum the HTTP SSE Profile needs to allow Bearer tokens to be provided in a query string for the OAuth2 scheme, but possibly something similar for the Basic scheme as well if you want that security scheme to be supported by that profile.

  1. Add an informative statement about secure transport (referencing the assertions in Architecture which should be sufficient).

The recommendation to use TLS where possible is already repeated in many places, but I don't have a problem with repeating/referencing it again here if people think that's necessary. Note there are already references in the Security Considerations section which reference the existing assertions (though doesn't mention that one specifically).

lu-zero commented 1 year ago

Note that the HTTP SSE Profile and HTTP Webhook Profile do not depend on the HTTP Basic Profile, they can be used independently. That means that if you move the security assertions from Common Constraints to the HTTP Basic Profile, they will no longer apply to the other two profiles. You would instead have to duplicate the text in all three profiles or refer to the section of the HTTP Basic Profile, which then makes it another Common Constraints section.

I added a HTTP Common constraints by splitting what should apply for any protocol and what is HTTP-specific, then I'd iterate to add a security section per profile and address your other points.

benfrancis commented 1 year ago

@lu-zero wrote:

I added a HTTP Common constraints by splitting what should apply for any protocol and what is HTTP-specific, then I'd iterate to add a security section per profile and address your other points.

I feel like we're going round in circles here.

I argued for a long time that there should not be a Common Constraints section at all, because there are no assertions which we can be certain will apply to all future Profiles. I tried to include everything in a single HTTP profile instead. In the end we had to split the HTTP Profile into three separate profiles because we couldn't agree on a single eventing mechanism, so I agreed to have a Common Constraints section in which to include assertions which apply to all three HTTP profiles. This was originally nested inside an "HTTP Profiles" section but ended up being moved up to the top level to reduce unnecessary levels of nesting in the document.

Now you are suggesting creating a second Common Constraints section specifically for HTTP Profiles, even though the current specification only contains HTTP profiles. The current Common Constraints section is an HTTP Profiles Common Constraints section. We could rename it if you really think it's necessary, but it seems a bit pointless until such a time as there are non-HTTP profiles.

lu-zero commented 1 year ago

I assumed that the changes were made in order to be ready for non-http profiles (e.g. CoAP + OSCORE + ACE matching HTTPS + OAuth2), I wasn't aware of the past iterations.

I prepared the patchset so should be easy to drop the split if it is deemed unnecessary.

mlagally commented 1 year ago

I think (https://github.com/w3c/wot-profile/pull/364) has been updated to take the discussion into account and should be merged.

egekorkan commented 1 year ago

@mmccool just moving my comment here:

It is not here anymore but I think that each profile should explain the security scheme on its own. Is OAuth2 usable in Webhook?

mmccool commented 1 year ago

Notes from Security TF meeting 6 March:

mmccool commented 1 year ago

Hmmm... one technical issue I noted is that "name" does not actually have a default. So technically you MUST include it with an appropriate value whenever you use "basic". Lots of examples using "basic" leave it out, since it's optional, but technically that means it is undefined. It really should have been given a default value of "Authorization" in the TD spec... but it's too late now. But I'll tweak the above assertion to say that "name" MUST be included with the value "Authorization".

mmccool commented 1 year ago

Made a PR: https://github.com/w3c/wot-profile/pull/374

I addition to the issue I noted about about the lack of default value for "name", I also realized that in HTTP the header name has two possible values, based on whether or not a "proxy" value is provided. So in the end my PR adds three assertions: one for "in", and two for "name" (for two different cases: without proxy, and with proxy).

mmccool commented 1 year ago

Suspect a lot of the above will be resolved once we reorganize security schemes to only be applicable/defined in particular bindings. So should wait for that to be done for HTTP at least, then restrict from there. For TD 2.0 I also suspect we will get rid of the unnecessary "in" definition for Basic, etc.