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

Make site secret available through public api #7824

Closed
jdalsem opened this issue Jan 27, 2015 · 8 comments
Closed

Make site secret available through public api #7824

jdalsem opened this issue Jan 27, 2015 · 8 comments
Assignees

Comments

@jdalsem
Copy link
Member

jdalsem commented Jan 27, 2015

A few plugins need to use the site secret. Currently the function get_site_secret is access private. Is that on purpose? Do we need a new publicly available elgg_get_site_secret or a class function $site->getSecret()

@ewinslow
Copy link
Contributor

Pointers to the plugins? Description of use cases?

The point of keeping things private is so that we don't over commit to a poorly designed API

@beck24
Copy link
Member

beck24 commented Jan 27, 2015

I have a number of plugins that rely on it - usually for creating access tokens.
Here's a public one: https://community.elgg.org/plugins/1874086

@jdalsem
Copy link
Member Author

jdalsem commented Jan 27, 2015

same here; all kinds of tokens that needed to be unique per site, like:

  • webservices tokens
  • logged out access to private content (with a secure url)
  • protecting upgrade/cron jobs
  • invitation urls

There are ways around this, but that would mean to think of a way in every plugin. With the site secret there is an easy way to get a site specific random secret.

@mrclay
Copy link
Member

mrclay commented Jan 27, 2015

We could do all those by exposing ElggCrypto::getHmac and ::areEqual in some way. It eliminates the risk of plugins doing something dumb with the key, and it nudges devs toward HMAC, which is the right tool for token (MAC) generation.

areEqual() does timing-attack-proof string comparison; unlikely to be a vector because the Elgg boot is so noisy, but why not do the right thing.

@mrclay
Copy link
Member

mrclay commented Jan 27, 2015

Basically I'd rather provide a "token service" that provides and validates them.

@mrclay
Copy link
Member

mrclay commented Mar 11, 2015

PR #8057

@mrclay
Copy link
Member

mrclay commented Mar 11, 2015

What use case(s) require direct access to the site secret key? A key could be derived from the secret key via elgg_generate_mac('some unique string', 'sha256'), which would cause this to change when the secret key changes.

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 12, 2015
This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings.

Fixes Elgg#7824
mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 12, 2015
This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings.

Fixes Elgg#7824
mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 12, 2015
This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings.

Fixes Elgg#7824
mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 18, 2015
This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings.

Fixes Elgg#7824
mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 18, 2015
This also has getHmac() use a binary encoding of the site key instead
of the Base64 or hex encodings, and improves the docs of the site secret
component.

Fixes Elgg#7824
mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Mar 24, 2015
HMAC uses a binary encoding of the site key instead of the Base64 or hex encodings, and improves the docs of the site secret component.

Fixes Elgg#7824
@jdalsem
Copy link
Member Author

jdalsem commented Apr 2, 2015

Closing this as #8057 removed the need to access the site secret publically

@jdalsem jdalsem closed this as completed Apr 2, 2015
@Srokap Srokap removed the in progress label Apr 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants