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

OpenSSL dependencies list #39

Closed
eoger opened this issue May 8, 2018 · 12 comments
Closed

OpenSSL dependencies list #39

eoger opened this issue May 8, 2018 · 12 comments

Comments

@eoger
Copy link
Contributor

eoger commented May 8, 2018

I believe that to simplify and shrink our Rust libraries builds, we should move away from OpenSSL.
Instead, we should use the ring rust library which doesn't need to be linked against a static library.
However, ring is still incomplete and we are missing some important crypto primitives, so we should consider this issue more as a laundry list.

User Primitives used Solution
reqwest TLS (on linux/android, implemented with openssl) Use the rustls-tls feature of reqwest.
sync15-adapter AEAD(HMAC::sha256/AES-256-CBC) (encrypt/decrypt) Depends on: briansmith/ring#588.

┆Issue is synchronized with this Jira Story

@briansmith
Copy link

sync15-adapter AEAD(HMAC::sha256/AES-256-CBC) (encrypt/decrypt) Depends on: briansmith/ring#588.

Is this some legacy thing that can't be changed to use a different AEAD?

Preliminary ECDSA signing support was just added in ring 0.13.0-alpha4, and the ring::signature::sign() API is "stable enough" for things to start using for ECDSA.

@rfk
Copy link
Contributor

rfk commented May 31, 2018

Is this some legacy thing that can't be changed to use a different AEAD?

Yes, it's to access data from existing Firefox Sync clients.

@eoger
Copy link
Contributor Author

eoger commented Jun 2, 2018

I'm currently trying to integrate biscuit (for JWKs) in order to drop the cjose/jansson dependencies.

However, I couldn't find an accessor for the x and y params of an ECDSA public key. I'm guessing something like public_key::parse_uncompressed_point in combination with public_from_private would probably do the trick, but not sure where to go from there. Toughts @briansmith? I'd be happy to write a PR if necessary.

@briansmith
Copy link

However, I couldn't find an accessor for the x and y params of an ECDSA public key. I'm guessing something like public_key::parse_uncompressed_point in combination with public_from_private would probably do the trick, but not sure where to go from there. Toughts @briansmith? I'd be happy to write a PR if necessary.

Later on, we might expand ring's API to be more accomodating to non-standard forms and other forms of keys, but looking at the backlog, that probably won't happen soon. The current design of ring is that we don't provide the X and Y coordinates intentionally. Instead we provide the entire public key in a long-ago-standardized form. In the case of JOSE they made a mistake and decided to make their own new format for public keys. So far, I've told JWT libraries that they should internally convert the standard form that ring provides into the form they need by parsing the standard form themselves. In the case of ECC public keys, the first byte is "0x04", the X coordinate is the first half of what's left, and the Y coordinate is the second half of what's left. So, I would submit a PR to biscuit to do that.

@vladikoff
Copy link
Contributor

ECDSA

Should be ECDH

@briansmith
Copy link

ECDSA

Should be ECDH

The standard form of the public key is exactly the same for both.

@eoger
Copy link
Contributor Author

eoger commented Jun 4, 2018

Thank you Vlad, I've updated the list: we need ECDH-ES w/ A256GCM.
I need to figure out if there's stuff missing from ring to support it in biscuit.

@eoger
Copy link
Contributor Author

eoger commented Jul 11, 2018

Thanks to the efforts made in #95 and #98, we have eliminated the openssl dependency on select platforms for the fxa-client crate (only reqwest pulls it in now). 🎉
I have updated the first post laundry list in consequence.

@briansmith
Copy link

If you use the rustls-tls feature of reqwest (and disable default features?) then it will use Rustls instead of OpenSSL for TLS.

@thomcc
Copy link
Contributor

thomcc commented Feb 6, 2019

We still need OpenSSL as part of SQLcipher (for logins, places) and for sync15-adapter, though. All our consumers of the FxA crate realistically still pull in OpenSSL.

@briansmith
Copy link

We still need OpenSSL as part of SQLcipher (for logins, places) and for sync15-adapter, though. All our consumers of the FxA crate realistically still pull in OpenSSL.

Those can be dealt with too.

@eoger
Copy link
Contributor Author

eoger commented Jul 9, 2019

Our last OpenSSL dependant are:

  • thereqwest crate (TLS stack) on Desktop Linux (Android has viaduct eliminating that dep).
  • rust-ece (but we implement our custom crypto backend, therefore not really using the openssl primitives).
    Closing in favor of individual issues.

@eoger eoger closed this as completed Jul 9, 2019
dmose pushed a commit to dmose/application-services that referenced this issue Mar 15, 2021
* experiments and enrollment status are all stored in the DB. Only
  fetches experiments in the constructor if there are none in the DB,
  which doesn't smell like the right thing long term - otherwise though,
  you must explicitly call `update_experiments()`. We should work out
  what behaviour makes sense for apps here.

* Fully implements the `opt_out` etc methods. Any experiment can be opted in
  to and opted out of, with this stored in the DB. Calling
  `update_experiments()` will not change the opted-on/out status - there's
  a new 'reset_enrollment` function that does though. Note that opting
  out is fully supported, but the expectation is that this must only be
  used in a dev or test context.

* When `update_experiments()` is called, enrollments for experiments
  which no longer exist are removed, and new experiments are evaluated
  for enrollment. We don't emit any events yet, but we could. We do log
  so you can see things happening. We don't support experiments being paused,
  nor start and end dates for experiments, etc.

* It allows you to opt in/out/etc to non-existing experiments - but they will
  be removed when `update_experiments()` is called (ie, it's treated as
  though the experiment was removed. We could fix this, but meh.

* No longer hits the server as the SDK interface is constructed.

* Our command-line utility `experiment.rs` supports all of this - use --help.
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

No branches or pull requests

6 participants