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

router: Keep RSA onion public key in ASN.1 format #292

Closed
wants to merge 3 commits into from

Conversation

Labels
None yet
Projects
None yet
3 participants
@dgoulet-tor
Copy link
Contributor

@dgoulet-tor dgoulet-tor commented Aug 23, 2018

The OpenSSL "RSA" object is currently 408 bytes compares to the ASN.1 encoding
which is 140 for a 1024 RSA key.

We save 268 bytes per descriptor (routerinfo_t) and microdescriptor
(microdesc_t). Scaling this to 6000 relays, and considering client usually
only have microdescriptors, we save 1.608 MB of RAM which is considerable for
mobile client.

This commit makes it that we keep the RSA onion public key (used for TAP
handshake) in ASN.1 format instead of an OpenSSL RSA object.

Changes is done in both routerinfo_t and microdesc_t.

Closes #27246

Signed-off-by: David Goulet dgoulet@torproject.org

The OpenSSL "RSA" object is currently 408 bytes compares to the ASN.1 encoding
which is 140 for a 1024 RSA key.

We save 268 bytes per descriptor (routerinfo_t) *and* microdescriptor
(microdesc_t). Scaling this to 6000 relays, and considering client usually
only have microdescriptors, we save 1.608 MB of RAM which is considerable for
mobile client.

This commit makes it that we keep the RSA onion public key (used for TAP
handshake) in ASN.1 format instead of an OpenSSL RSA object.

Changes is done in both routerinfo_t and microdesc_t.

Closes #27246

Signed-off-by: David Goulet <dgoulet@torproject.org>
@coveralls
Copy link

@coveralls coveralls commented Aug 23, 2018

Coverage Status

Coverage increased (+0.02%) to 59.561% when pulling eb692dc on dgoulet-tor:ticket27246_035_01 into ac44e70 on torproject:master.

*
* Return NULL if md or the md's onion pkey is NULL or malformed. */
crypto_pk_t *
microdesc_get_rsa_onion_pkey(const microdesc_t *md)
Copy link
Contributor

@nmathewson nmathewson Aug 28, 2018

These two functions seem redundant with their routerinfo_t equivalent. Do you think it's reasonable to combine them -- especially the "set" one, since it's longer?

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Aug 29, 2018

Agrree. I merge them together and put the helper functions in router.c. Hope this is fine. See fixup commit: 61bdbe4

@@ -5498,7 +5498,7 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
r1->ipv6_orport != r2->ipv6_orport ||
r1->dir_port != r2->dir_port ||
r1->purpose != r2->purpose ||
!crypto_pk_eq_keys(r1->onion_pkey, r2->onion_pkey) ||
!tor_memeq(r1->onion_pkey, r2->onion_pkey, r1->onion_pkey_len) ||
Copy link
Contributor

@nmathewson nmathewson Aug 28, 2018

Bug here: you need to check that the lengths are equal first!

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Aug 29, 2018

Fixup eb692dc

goto done;
}

md->onion_pkey = tor_malloc_zero(len);
Copy link
Contributor

@nmathewson nmathewson Aug 28, 2018

You could use tor_memdup() here.

Copy link
Contributor Author

@dgoulet-tor dgoulet-tor Aug 29, 2018

Fixed in the previous commit.

@nmathewson
Copy link
Contributor

@nmathewson nmathewson commented Sep 30, 2018

This was squashed and merged.

@nmathewson nmathewson closed this Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment