Opened 7 months ago

Closed 6 months ago

#28184 closed defect (wontfix)

Reload is additive with regards to new v3 HS client authorizations but it won't subtract deleted ones

Reported by: jchevali Owned by: haxxpop
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version: Tor: 0.3.5.2-alpha
Severity: Normal Keywords: tor-hs
Cc: Actual Points:
Parent ID: Points:
Reviewer: asn Sponsor:

Description

Sending a reload signal seems to add to authorizations not present before the reload but when the reload is issued after some authorizations have been deleted or the files in which they resided given invalid extensions it won't clear those from memory in this running instance.

Child Tickets

Change History (12)

comment:1 Changed 7 months ago by dgoulet

Keywords: tor-hs added
Milestone: Tor: 0.3.5.x-final

comment:2 Changed 7 months ago by jchevali

This is client-side and with regards to private keys. Server side / public keys not tested, the problem there might or might not arise.

comment:3 Changed 7 months ago by dgoulet

Client side, it appears that if you remove a file from ClientOnionAuthDir and HUP tor, we do not close the corresponding circuits.

Service side it would be impossible since the service doesn't know which rendezvous circuit is for which client.

comment:4 Changed 7 months ago by dgoulet

Milestone: Tor: 0.3.5.x-finalTor: unspecified

Oh also, cleaning up the descriptor client side seems wise choice as well.

comment:5 Changed 7 months ago by haxxpop

Owner: set to haxxpop
Status: newassigned

comment:6 Changed 7 months ago by haxxpop

Status: assignedneeds_review

comment:7 Changed 7 months ago by dgoulet

Reviewer: asn

comment:8 Changed 7 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

(This will need a changes file. I assume it's for 0.3.5, since that's where client auth comes from?)

comment:9 Changed 7 months ago by dgoulet

Status: needs_reviewneeds_information

Ooook we had a discussion on IRC but without much of a conclusion I would say. I'll express my thoughts:

This patch is indeed quite large for what it does, not that there is a quicker way to do it but rather lets step back and think what we want.

  1. Closing the intro/RP circuits (client side):

That requires quite a bit of complexity including adding a way to lookup circuits by service identity key from the hs_circuitmap. I wouldn't be too sad if we don't do that. Those circuits would simply close by themselves at some point or heck even be used for the same .onion.

  1. Clearing our descriptor cache (client side):

This is a bit more interesting because if the client authorization for A.onion changed then the old descriptor is not usable anymore meaning we won't be able to decrypt it.

There lies another issue. I don't think we have that feature which is if a client looks up a descriptor in its cache and can not decrypt it, we should purge it and refetch it. A client does NOT store a descriptor that it can't decode so at least that is that. But this situation can happen if we change the client auth for A.onion and SIGHUP.

All in all, we could reduce the complexity of this patch by simply adding a way to "purge a undecodable descriptor in our cache" which will lead to fetching the new descriptor and using the new client authorization.

We would ignore the closing the circuits because if there is an RP circuit for A.onion, great we use it.

comment:10 in reply to:  9 ; Changed 7 months ago by haxxpop

Replying to dgoulet:

  1. Clearing our descriptor cache (client side):

This is a bit more interesting because if the client authorization for A.onion changed then the old descriptor is not usable anymore meaning we won't be able to decrypt it.

There lies another issue. I don't think we have that feature which is if a client looks up a descriptor in its cache and can not decrypt it, we should purge it and refetch it. A client does NOT store a descriptor that it can't decode so at least that is that. But this situation can happen if we change the client auth for A.onion and SIGHUP.

All in all, we could reduce the complexity of this patch by simply adding a way to "purge a undecodable descriptor in our cache" which will lead to fetching the new descriptor and using the new client authorization.

We would ignore the closing the circuits because if there is an RP circuit for A.onion, great we use it.

I would like to add some opinion here. I think "refetching when the client can't decode or can't use the IPs" should be considered not client auth related.

I mean we should refetch only when we can't decode or can't use the IPs. It shouldn't be triggered by anything else like when the client change the auth config, or anything else. Otherwise, I think the code will be too complex.

ps. I use the word "refetch" instead of "clear cache" because I think the meanings are similar.

comment:11 in reply to:  10 Changed 7 months ago by asn

Replying to haxxpop:

Replying to dgoulet:

  1. Clearing our descriptor cache (client side):

This is a bit more interesting because if the client authorization for A.onion changed then the old descriptor is not usable anymore meaning we won't be able to decrypt it.

There lies another issue. I don't think we have that feature which is if a client looks up a descriptor in its cache and can not decrypt it, we should purge it and refetch it. A client does NOT store a descriptor that it can't decode so at least that is that. But this situation can happen if we change the client auth for A.onion and SIGHUP.

All in all, we could reduce the complexity of this patch by simply adding a way to "purge a undecodable descriptor in our cache" which will lead to fetching the new descriptor and using the new client authorization.

We would ignore the closing the circuits because if there is an RP circuit for A.onion, great we use it.

I would like to add some opinion here. I think "refetching when the client can't decode or can't use the IPs" should be considered not client auth related.

I mean we should refetch only when we can't decode or can't use the IPs. It shouldn't be triggered by anything else like when the client change the auth config, or anything else. Otherwise, I think the code will be too complex.

Agreed. But we should also place a cap on how many refetches we do in this case, otherwise a client who - for some reason - tries to access a client-authed HS without any credentials will keep on re-fetching the descriptor since they can't decrypt it.

comment:12 Changed 6 months ago by dgoulet

Resolution: wontfix
Status: needs_informationclosed

I mean we should refetch only when we can't decode or can't use the IPs. It shouldn't be triggered by anything else like when the client change the auth config, or anything else. Otherwise, I think the code will be too complex.

Data point to this. The tor client stores the descriptor *decoded* in the cache so my statement earlier was wrong that if the client authorization file is changed for a .onion for which we have a descriptor in our cache, it won't matter, tor will still use the descriptor because it is completely decoded (including decrypted ofc).

Few observations I took the time to go over:

  1. The client cache does *not* store an undecodable descriptor so we don't have the problem of having "unusable descriptor" in our cache, they can only be unusable if the IPs aren't working (basically what I said above).
  1. If we can't decode a descriptor, tor will try a new HSDir until it runs out of HSDir to ask and then the SOCKS connection is cut off.
  1. If the intro points are unusable, tor client will do a refetch. And this will happen like (2) as in every HSDir will be queried until none remains. See hs_client_any_intro_points_usable().

Which means that all in all, I will close this ticket for now. If we want to improve something that I might have missed, lets open a new simpler ticket about it.

Note: See TracTickets for help on using tickets.