March 2020 - tor-commits - lists.torproject.org

[tor/maint-0.4.2] Trivial bugfixes found during TROVE investigation.
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit c940b7cf13d8ceb5705d52c215256e1de5a4e757 Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Feb 10 16:35:40 2020 +0200 Trivial bugfixes found during TROVE investigation. --- src/feature/hs/hs_client.c | 6 +++--- src/lib/crypt_ops/crypto_ed25519.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index fd2d26645..0efe9fc28 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1275,7 +1275,7 @@ hs_client_decode_descriptor(const char *desc_str, uint8_t subcredential[DIGEST256_LEN]; ed25519_public_key_t blinded_pubkey; hs_client_service_authorization_t *client_auth = NULL; - curve25519_secret_key_t *client_auht_sk = NULL; + curve25519_secret_key_t *client_auth_sk = NULL; tor_assert(desc_str); tor_assert(service_identity_pk); @@ -1284,7 +1284,7 @@ hs_client_decode_descriptor(const char *desc_str, /* Check if we have a client authorization for this service in the map. */ client_auth = find_client_auth(service_identity_pk); if (client_auth) { - client_auht_sk = &client_auth->enc_seckey; + client_auth_sk = &client_auth->enc_seckey; } /* Create subcredential for this HS so that we can decrypt */ @@ -1297,7 +1297,7 @@ hs_client_decode_descriptor(const char *desc_str, /* Parse descriptor */ ret = hs_desc_decode_descriptor(desc_str, subcredential, - client_auht_sk, desc); + client_auth_sk, desc); memwipe(subcredential, 0, sizeof(subcredential)); if (ret < 0) { goto err; diff --git a/src/lib/crypt_ops/crypto_ed25519.c b/src/lib/crypt_ops/crypto_ed25519.c index 400f96389..0a442bb73 100644 --- a/src/lib/crypt_ops/crypto_ed25519.c +++ b/src/lib/crypt_ops/crypto_ed25519.c @@ -795,7 +795,7 @@ ed25519_point_is_identity_element(const uint8_t *point) int ed25519_validate_pubkey(const ed25519_public_key_t *pubkey) { - uint8_t result[32] = {9}; + uint8_t result[32] = {0}; /* First check that we were not given the identity element */ if (ed25519_point_is_identity_element(pubkey->pubkey)) {

1 0

[tor/maint-0.4.2] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit e15a621ac81c472f69a082180fa9b4ed39274e43 Merge: 9ef8f5d1b 5f4e14b8c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 11:45:16 2020 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2 changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_client.c | 6 +++--- src/feature/hs/hs_service.c | 6 ++++++ src/lib/crypt_ops/crypto_ed25519.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-)

1 0

[tor/maint-0.4.3] Trivial bugfixes found during TROVE investigation.
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit c940b7cf13d8ceb5705d52c215256e1de5a4e757 Author: George Kadianakis <desnacked(a)riseup.net> Date: Mon Feb 10 16:35:40 2020 +0200 Trivial bugfixes found during TROVE investigation. --- src/feature/hs/hs_client.c | 6 +++--- src/lib/crypt_ops/crypto_ed25519.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index fd2d26645..0efe9fc28 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1275,7 +1275,7 @@ hs_client_decode_descriptor(const char *desc_str, uint8_t subcredential[DIGEST256_LEN]; ed25519_public_key_t blinded_pubkey; hs_client_service_authorization_t *client_auth = NULL; - curve25519_secret_key_t *client_auht_sk = NULL; + curve25519_secret_key_t *client_auth_sk = NULL; tor_assert(desc_str); tor_assert(service_identity_pk); @@ -1284,7 +1284,7 @@ hs_client_decode_descriptor(const char *desc_str, /* Check if we have a client authorization for this service in the map. */ client_auth = find_client_auth(service_identity_pk); if (client_auth) { - client_auht_sk = &client_auth->enc_seckey; + client_auth_sk = &client_auth->enc_seckey; } /* Create subcredential for this HS so that we can decrypt */ @@ -1297,7 +1297,7 @@ hs_client_decode_descriptor(const char *desc_str, /* Parse descriptor */ ret = hs_desc_decode_descriptor(desc_str, subcredential, - client_auht_sk, desc); + client_auth_sk, desc); memwipe(subcredential, 0, sizeof(subcredential)); if (ret < 0) { goto err; diff --git a/src/lib/crypt_ops/crypto_ed25519.c b/src/lib/crypt_ops/crypto_ed25519.c index 400f96389..0a442bb73 100644 --- a/src/lib/crypt_ops/crypto_ed25519.c +++ b/src/lib/crypt_ops/crypto_ed25519.c @@ -795,7 +795,7 @@ ed25519_point_is_identity_element(const uint8_t *point) int ed25519_validate_pubkey(const ed25519_public_key_t *pubkey) { - uint8_t result[32] = {9}; + uint8_t result[32] = {0}; /* First check that we were not given the identity element */ if (ed25519_point_is_identity_element(pubkey->pubkey)) {

1 0

[tor/maint-0.4.2] Merge branch 'maint-0.3.5' into maint-0.4.1
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit 5f4e14b8c8cd4c8907f669144a5a4bb8b8b7a585 Merge: c22696e36 089e57d22 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 11:45:16 2020 -0400 Merge branch 'maint-0.3.5' into maint-0.4.1 changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_client.c | 6 +++--- src/feature/hs/hs_service.c | 6 ++++++ src/lib/crypt_ops/crypto_ed25519.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-)

1 0

[tor/maint-0.4.3] Fix TROVE-2020-003.
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit 089e57d22f7c5e755a2d88d0b102207f7207ee27 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Feb 11 18:37:55 2020 +0200 Fix TROVE-2020-003. Given that ed25519 public key validity checks are usually not needed and (so far) they are only necessary for onion addesses in the Tor protocol, we decided to fix this specific bug instance without modifying the rest of the codebase (see below for other fix approaches). In our minimal fix we check that the pubkey in hs_service_add_ephemeral() is valid and error out otherwise. --- changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_service.c | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/changes/trove_2020_003 b/changes/trove_2020_003 new file mode 100644 index 000000000..aa1a8f1c7 --- /dev/null +++ b/changes/trove_2020_003 @@ -0,0 +1,4 @@ + o Minor bugfixes (onion services v3): + - Fix assertion failure that could result from a corrupted ADD_ONION control + port command. Found by Saibato. Fixes bug 33137; bugfix on + 0.3.3.1-alpha. This issue is also being tracked as TROVE-2020-003. diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 7e150599f..6d32cae86 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -3578,6 +3578,12 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports, goto err; } + if (ed25519_validate_pubkey(&service->keys.identity_pk) < 0) { + log_warn(LD_CONFIG, "Bad ed25519 private key was provided"); + ret = RSAE_BADPRIVKEY; + goto err; + } + /* Make sure we have at least one port. */ if (smartlist_len(service->config.ports) == 0) { log_warn(LD_CONFIG, "At least one VIRTPORT/TARGET must be specified "

1 0

[tor/maint-0.4.3] Merge branch 'maint-0.3.5' into maint-0.4.1
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit 5f4e14b8c8cd4c8907f669144a5a4bb8b8b7a585 Merge: c22696e36 089e57d22 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 11:45:16 2020 -0400 Merge branch 'maint-0.3.5' into maint-0.4.1 changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_client.c | 6 +++--- src/feature/hs/hs_service.c | 6 ++++++ src/lib/crypt_ops/crypto_ed25519.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-)

1 0

[tor/maint-0.4.3] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit e15a621ac81c472f69a082180fa9b4ed39274e43 Merge: 9ef8f5d1b 5f4e14b8c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 11:45:16 2020 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2 changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_client.c | 6 +++--- src/feature/hs/hs_service.c | 6 ++++++ src/lib/crypt_ops/crypto_ed25519.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-)

1 0

[tor/maint-0.4.3] Add unittest for TROVE-2020-003.
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit 5ff8757aa89cd9caa17207beb080607941336a5e Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Feb 11 14:51:58 2020 +0200 Add unittest for TROVE-2020-003. This unittest currently fails on purpose (to demonstrate the bug) but it will stop failing after the next commit (the bugfix). --- src/test/test_hs_control.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 881479016..566cc21fe 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -638,6 +638,46 @@ test_hs_control_store_permanent_creds(void *arg) hs_client_free_all(); } +/** Test that ADD_ONION properly handles an attacker passing it a bad private + * key. */ +static void +test_hs_control_add_onion_with_bad_pubkey(void *arg) +{ + (void) arg; + + MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock); + + int retval; + control_connection_t conn; + char *args = NULL; + char *cp1 = NULL; + size_t sz; + + hs_init(); + + { /* Setup the control conn */ + memset(&conn, 0, sizeof(control_connection_t)); + TO_CONN(&conn)->outbuf = buf_new(); + conn.current_cmd = tor_strdup("ADD_ONION"); + } + + args = tor_strdup("ED25519-V3:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA Port=9735,127.0.0.1 Flags=DiscardPK"); + + retval = handle_control_command(&conn, (uint32_t) strlen(args), args); + tt_int_op(retval, OP_EQ, 0); + + /* Check control port response */ + cp1 = buf_get_contents(TO_CONN(&conn)->outbuf, &sz); + tt_str_op(cp1, OP_EQ, "551 Failed to generate onion address\r\n"); + + done: + tor_free(args); + tor_free(cp1); + buf_free(TO_CONN(&conn)->outbuf); + tor_free(conn.current_cmd); +} + struct testcase_t hs_control_tests[] = { { "hs_desc_event", test_hs_desc_event, TT_FORK, NULL, NULL }, @@ -649,6 +689,8 @@ struct testcase_t hs_control_tests[] = { NULL, NULL }, { "hs_control_store_permanent_creds", test_hs_control_store_permanent_creds, TT_FORK, NULL, NULL }, + { "hs_control_add_onion_with_bad_pubkey", + test_hs_control_add_onion_with_bad_pubkey, TT_FORK, NULL, NULL }, END_OF_TESTCASES };

1 0

[tor/maint-0.4.3] Merge branch 'maint-0.4.2' into maint-0.4.3
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit bbc80ea042a124a5935bd4264677180efdcb2da2 Merge: 855cd533e e15a621ac Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 17 11:45:16 2020 -0400 Merge branch 'maint-0.4.2' into maint-0.4.3 changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_client.c | 6 +++--- src/feature/hs/hs_service.c | 6 ++++++ src/lib/crypt_ops/crypto_ed25519.c | 2 +- 4 files changed, 14 insertions(+), 4 deletions(-) diff --cc src/feature/hs/hs_client.c index 222261e60,9d67f7127..af8cb0b41 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@@ -1870,9 -1296,9 +1870,9 @@@ hs_client_decode_descriptor(const char /* Parse descriptor */ ret = hs_desc_decode_descriptor(desc_str, subcredential, - client_auht_sk, desc); + client_auth_sk, desc); memwipe(subcredential, 0, sizeof(subcredential)); - if (ret < 0) { + if (ret != HS_DESC_DECODE_OK) { goto err; }

1 0

[tor/master] Fix TROVE-2020-003.
by nickm＠torproject.org 17 Mar '20

17 Mar '20

commit 089e57d22f7c5e755a2d88d0b102207f7207ee27 Author: George Kadianakis <desnacked(a)riseup.net> Date: Tue Feb 11 18:37:55 2020 +0200 Fix TROVE-2020-003. Given that ed25519 public key validity checks are usually not needed and (so far) they are only necessary for onion addesses in the Tor protocol, we decided to fix this specific bug instance without modifying the rest of the codebase (see below for other fix approaches). In our minimal fix we check that the pubkey in hs_service_add_ephemeral() is valid and error out otherwise. --- changes/trove_2020_003 | 4 ++++ src/feature/hs/hs_service.c | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/changes/trove_2020_003 b/changes/trove_2020_003 new file mode 100644 index 000000000..aa1a8f1c7 --- /dev/null +++ b/changes/trove_2020_003 @@ -0,0 +1,4 @@ + o Minor bugfixes (onion services v3): + - Fix assertion failure that could result from a corrupted ADD_ONION control + port command. Found by Saibato. Fixes bug 33137; bugfix on + 0.3.3.1-alpha. This issue is also being tracked as TROVE-2020-003. diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 7e150599f..6d32cae86 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -3578,6 +3578,12 @@ hs_service_add_ephemeral(ed25519_secret_key_t *sk, smartlist_t *ports, goto err; } + if (ed25519_validate_pubkey(&service->keys.identity_pk) < 0) { + log_warn(LD_CONFIG, "Bad ed25519 private key was provided"); + ret = RSAE_BADPRIVKEY; + goto err; + } + /* Make sure we have at least one port. */ if (smartlist_len(service->config.ports) == 0) { log_warn(LD_CONFIG, "At least one VIRTPORT/TARGET must be specified "

1 0