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

Change statics to const, where possible #1427

Open
kevinburke opened this issue Nov 18, 2021 · 0 comments · May be fixed by #1428
Open

Change statics to const, where possible #1427

kevinburke opened this issue Nov 18, 2021 · 0 comments · May be fixed by #1428

Comments

@kevinburke
Copy link

kevinburke commented Nov 18, 2021

In rustls-ffi, we are hoping to export some ciphersuites (rustls::SupportedCipherSuite structs) to C code, by putting them in a const array (see rustls/rustls-ffi#165). One obstacle we are running into is that each rustls::SupportedCipherSuite has several statics inside of it, and you can't declare a const that points to statics:

pub static TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256: SupportedCipherSuite =
    SupportedCipherSuite::Tls12(&Tls12CipherSuite {
        common: CipherSuiteCommon {
            suite: CipherSuite::TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
            bulk: BulkAlgorithm::Chacha20Poly1305,
            aead_algorithm: &ring::aead::CHACHA20_POLY1305,
        },
        kx: KeyExchangeAlgorithm::ECDHE,
        sign: TLS12_ECDSA_SCHEMES,
        fixed_iv_len: 12,
        explicit_nonce_len: 0,
        aead_alg: &ChaCha20Poly1305,
        hmac_algorithm: ring::hmac::HMAC_SHA256,
    });

I am not a Rust expert, but from my understanding, the Rust compiler has gotten smarter about its ability to evaluate consts, which may make the conversion from static to const possible now, especially if your compatibility guarantee is only for the latest stable version of Rust.

If there is otherwise no difference, would it be possible to change some of the static values in ring to const values instead?

I will take a shot at a patch to see if it's technically feasible, then we can go from there.

kevinburke added a commit to kevinburke/ring that referenced this issue Nov 18, 2021
It may not have been possible to do this when this code was written,
but it should be possible now.

Fixes briansmith#1427.
kevinburke added a commit to kevinburke/ring that referenced this issue Nov 18, 2021
It may not have been possible to do this when this code was written,
but it should be possible now.

Fixes briansmith#1427.
kevinburke added a commit to kevinburke/ring that referenced this issue Nov 18, 2021
It may not have been possible to do this when this code was written,
but it should be possible now.

Fixes briansmith#1427.
@kevinburke kevinburke linked a pull request Nov 18, 2021 that will close this issue
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 a pull request may close this issue.

1 participant