Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve descriptions of apikey and psk security schemes #1031

Merged
merged 6 commits into from
Jan 27, 2021

Conversation

mmccool
Copy link
Contributor

@mmccool mmccool commented Jan 17, 2021

To resolve issue #998

Notes:

  1. The wotsec.ttl file has been updated, and rendering of index.html updated.
  2. Will treat the problem of non-query URI templates separately and orthogonally (discussed in issue [question] How to describe Philips Hue security scheme #923)
  3. An example would be useful, as requested in issue [question] How to describe apikey in query #922. Not handled in this PR, however.
  4. td-validation.ttl also updated, although it seems redundant. It would be helpful to clarify which is the master.

Preview | Diff

@mmccool
Copy link
Contributor Author

mmccool commented Jan 18, 2021

  • Added copies of descriptions of apikey and psk to td-validation.ttl. Which is the master? The render script does not make clear. It would be nice to not have to enter things twice.
  • Render script seems to be broken, getting a syntax error in the included file. Waiting for a fix. Until this is fixed the changes will not get reflected in the index.html file. In the meantime, reviewers should look at the text in ontology/wotsec.ttl for the apikey and psk schemes.

@mmccool mmccool marked this pull request as ready for review January 18, 2021 03:08
@mmccool
Copy link
Contributor Author

mmccool commented Jan 18, 2021

Marking as "ready for review" but "not ready for merging". Intend to discuss in Security call on Jan 18

@mmccool mmccool changed the title Improve descriptions of apikey and psk security schemes WIP: Improve descriptions of apikey and psk security schemes Jan 18, 2021
@mmccool
Copy link
Contributor Author

mmccool commented Jan 18, 2021

@OliverPfaff notes that there is an IETF RFC, https://tools.ietf.org/html/rfc4949 for security terminology we should cite, but unfortunately it does not define either "API Key" (which is a "marketing" term...) but more surprisingly (since it is used in other RFCs like TLS-PSK) does not define "PSK" or "pre-shared key".

@mmccool
Copy link
Contributor Author

mmccool commented Jan 18, 2021

Rendering error seems to have been fixed now but new text still not showing up, so I'm probably not making the edit in the right place... will try to fix in/for the TD call, in the meantime please do try to provide feedback on the proposed text. I plan to reword the last sentence in the PSK spec to make is sound less like an assertion (it really should be, but testing would be difficult, so...)

@egekorkan
Copy link
Contributor

This looks good and has the content that I would look for when I am seeing these terms for the first time

@sebastiankb
Copy link
Contributor

From today's TD call:

  • the content looks good
  • however, PR not ready yet, needs some render script check (@vcharpenay can you help here?)
  • PR maybe ready to merge next week

@danielpeintner
Copy link
Contributor

* however, PR not ready yet, needs some render script check (@vcharpenay can you help here?)

I did look into the the "issue" and my local rendering does work just fine. I think the updated content is in index.html.
Maybe you want trying to re-run the render script.

index.zip

@mmccool
Copy link
Contributor Author

mmccool commented Jan 25, 2021

  • Thanks, I'll try the rendering again. I assume a bug got fixed...
  • In the discussion in the TD call and in the security call, we agreed that an assertion prohibiting putting secret keys in the TD would be useful. It can't easily be tested, but can be manually asserted, and gives us a way to say that people who put secret keys in a TD are not compliant with the specification. I plan to structure this as a single blanket assertion rather than assertions in particular schemes (so it is prohibited, using the MUST NOT RFC2119 keyword, for all schemes, current and future). Wording of the assertion, to be added to the header section for all security schemes, to be as follows: "For all security schemes, private keys, passwords, or other sensitive information directly providing access should be shared and stored out-of-band and MUST NOT be stored in the TD." This would be followed by the informative statement: "The purpose of the TD is to describe how to access the system if a consumer has authorization, not to grant that authorization."

@mmccool mmccool changed the title WIP: Improve descriptions of apikey and psk security schemes Improve descriptions of apikey and psk security schemes Jan 25, 2021
@mmccool
Copy link
Contributor Author

mmccool commented Jan 25, 2021

Rendering fixed, changes discussed above implemented. Ready to merge.

@egekorkan
Copy link
Contributor

One thing I have realized is that the API Key says that it can be in a proprietary format. Thus, wouldn't we need a keyword called format (or similar) that can be used to convey this information. Otherwise, one would need documentation on that format via out-of-band-information. The string value can be left open or constrained to a set of values if such formats are explored.

@sebastiankb
Copy link
Contributor

from today's TD call we will merge this PR.

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.

4 participants