Opened 11 months ago

Last modified 6 months ago

#27896 needs_information defect

base32 padding inconsistency between client and server in HS v3 client auth preview

Reported by: jchevali Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.3.5.1-alpha
Severity: Normal Keywords: tor-hs, hs-auth, 040-deferred-20190220
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

There seems to be some base32 padding tolerance inconsistency between client and server for the HS v3 client auth preview in tor-0.3.5.1-alpha

The server seems to accept base32-encoded client public keys padded with = signs to 56 characters in length and won't work otherwise (i.e., if = signs are removed), while the client would work without the padding (i.e., = signs removed) but will ignore the client's private key if the padding is present.

I don't think this affects how the feature works (which I haven't been able to test anyway because it doesn't seem to enforce authorization at this stage - it still seems to let everyone in), but at least it seems to affect which values are valid and allowed to be loaded when reading the config.

Child Tickets

Change History (17)

comment:1 Changed 11 months ago by nickm

Milestone: Tor: 0.3.5.x-final

comment:2 Changed 11 months ago by dgoulet

Keywords: tor-hs added

comment:3 in reply to:  description Changed 10 months ago by dgoulet

Milestone: Tor: 0.3.5.x-finalTor: unspecified
Status: newneeds_information

Replying to jchevali:

There seems to be some base32 padding tolerance inconsistency between client and server for the HS v3 client auth preview in tor-0.3.5.1-alpha

The server seems to accept base32-encoded client public keys padded with = signs to 56 characters in length and won't work otherwise (i.e., if = signs are removed).

I don't think that is accurate. Service side we explicitly do not allow padded base32. If the client key line is not equal to the *no padding* length, we don't accept.

  if (strlen(pubkey_b32) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
    log_warn(LD_REND, "Client authorization encoded base32 public key "
                      "length is invalid: %s", pubkey_b32);
    goto err;
  }

while the client would work without the padding (i.e., = signs removed) but will ignore the client's private key if the padding is present.

And the client code does exactly the same:

  if (strlen(seckey_b32) != BASE32_NOPAD_LEN(CURVE25519_PUBKEY_LEN)) {
    log_warn(LD_REND, "Client authorization encoded base32 private key "
                      "length is invalid: %s", seckey_b32);
    goto err;
  }

I don't think this affects how the feature works (which I haven't been able to test anyway because it doesn't seem to enforce authorization at this stage - it still seems to let everyone in), but at least it seems to affect which values are valid and allowed to be loaded when reading the config.

Can you explain how you came down to this conclusion? And also, why are you saying it is "not enforced"? If the service is configured to use client authorization and a client is configured for it, it should NOT let everyone go through.

comment:4 Changed 10 months ago by jchevali

I've tried to pick up on this test but I can't even get that far.

Now I got client-side errors when trying to connect to a v3 HS with user authentication defined for it, even before the client has defined his own client-side bit of the authentication.

I got a file at the server inside the HS folder with .auth extension containing this:

descriptor:x25519:LSHKYRU3WPY3QW6HZWET6UW4IKU2WZXRWAVVZZVGR2NROXJ3WQZQ

What I get at the client side when I try to connect to this HS is this:

Oct 19 20:08:06.000 [warn] Failed to store hidden service descriptor
Oct 19 20:08:06.000 [warn] Encrypted service descriptor MAC check failed
Oct 19 20:08:06.000 [warn] Decrypting encrypted desc failed.
Oct 19 20:08:06.000 [warn] Service descriptor decryption failed.
Oct 19 20:08:06.000 [warn] Could not parse received descriptor as client.
Oct 19 20:08:06.000 [warn] Failed to parse received descriptor "..."(lots of stuff)[...truncated]

Is this the way the client is meant to convey that it does not possess the right credentials for this HS? With a long series of warnings thrown to the log?

comment:5 Changed 10 months ago by jchevali

It's funny; I'm testing this on two servers that are using the same tor-0.3.5.3-alpha build (though not the same version of Debian).

One server is causing the problem I described an hour ago, but only at the client side. There seem to be no problem at the server side, and it seems to accept unpadded pk descriptors.

The other server however is not accepting unpadded descriptors, only padded descriptors, as per my original bug submission, i.e., it would accept this:

descriptor:x25519:LSHKYRU3WPY3QW6HZWET6UW4IKU2WZXRWAVVZZVGR2NROXJ3WQZQ====

(which as you see is the same as the contents in my previous bug comment, only with padding)

If I give it an unpadded pk descriptor in the .auth file, with the same exact content I gave the other server, it would crash like this (part what I originally called 'padding intolerance'):

tor_assertion_failed_(): Bug: src/feature/hs/hs_descriptor.c:2897:
	hs_desc_build_authorized_client:
	Assertion !tor_mem_is_zero((char *) descriptor_cookie, HS_DESC_DESCRIPTOR_COOKIE_LEN) failed; aborting.
Bug: Assertion !tor_mem_is_zero((char *) descriptor_cookie, HS_DESC_DESCRIPTOR_COOKIE_LEN) failed in
	hs_desc_build_authorized_client at src/feature/hs/hs_descriptor.c:2897. Stack trace:
Bug:     ./tor(log_backtrace_impl+0x47) [0x5644802aa7d7]
Bug:     ./tor(tor_assertion_failed_+0x93) [0x5644802a5e63]
Bug:     ./tor(hs_desc_build_authorized_client+0x2c7) [0x5644801b99a7]
Bug:     ./tor(+0x1026e0) [0x5644801bb6e0]
Bug:     ./tor(hs_service_load_all_keys+0x7c0) [0x5644801bf580]
Bug:     ./tor(set_options+0xe0a) [0x564480229c2a]
Bug:     ./tor(options_init_from_string+0x4c7) [0x56448022b997]
Bug:     ./tor(options_init_from_torrc+0x471) [0x56448022bef1]
Bug:     ./tor(+0x53d61) [0x56448010cd61]
Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(+0x22a6c) [0x7fe103a85a6c]
Bug:     /usr/lib/x86_64-linux-gnu/libevent-2.1.so.6(event_base_loop+0x5a7) [0x7fe103a86537]
Bug:     ./tor(do_main_loop+0x91) [0x564480121241]
Bug:     ./tor(tor_run_main+0x1373) [0x56448010f1b3]
Bug:     ./tor(tor_main+0x3a) [0x56448010c61a]
Bug:     ./tor(main+0x19) [0x56448010c179]
Bug:     /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7fe1029c9b45]
Bug:     ./tor(+0x531c9) [0x56448010c1c9]

If I use the padded version, then the server would not crash, however, the client will be able to connect to the HS -- which shouldn't be the case, because the client hasn't been given any private key to match that (it doesn't matter if I give the client under the form of .auth_private files private keys for unrelated services, or no private keys / no files at all).

comment:6 Changed 10 months ago by jchevali

So we have one server (described in comment 4) that does not crash on unpadded .auth file entries, but might be generating invalid descriptors (unless they're valid and it's the client's fault -- by the way, the client I have at the moment is on Win32 and slightly older version at tor-0.3.5.2-alpha). This server is not where Tor was build; it was copied over from another server, just the executables (some libraries external to Tor might be different versions, though Tor doesn't complain at run time).

And we have another server, where the build was done originally (described in comment 5) that does crash on unpadded .auth file entries, but perhaps that's not because of lack of padding, but something else. Perhaps in this server the assertion is hit and in the other isn't. By the way, I can only wonder what'd happen if this assertion wasn't hit (i.e., would it also lead to uploading invalid descriptors? I don't know.)

Conversely, perhaps this later server isn't exhibiting any symptoms in the case I give it padded .auth file entries. But in this case possibly it is not setting up any HS client-side authentication at all either. That'd explain why the client can connect: maybe not a failure of the HS v3 authentication mechanism, but simply that, silently, for the padded .auth file case authentication isn't actually set up. While I originally thought that in this case the server was working successfully and the client was failing (by being able to connect when it shouldn't have), perhaps it's the reverse: the server's silently failing to set up authentication and publishing descriptors that would let everyone in, and the client of course connects.

Does it make sense?

comment:7 Changed 10 months ago by traumschule

Keywords: hs-auth added

comment:8 Changed 10 months ago by jchevali

I think I'm starting to question the wisdom of rend-spec-v3.txt, para 6.1.2, "Tor SHOULD ignore lines it does not recognize."

Probably the procedure should be, if there aren't otherwise any valid lines, unless all invalid lines are comments, assume if there's an invalid entry in an .auth file a valid entry was meant and a mistake was made, and access should be denied by default instead of being granted by default.

Because presumably if there were other valid entries access would be denied except to those, and failure to parse a further entry would not result in unrestricted access. But where there's only one entry, or a bunch of unparseable entries, a failure to parse in this case in practice would result in unrestricted access, which perhaps wasn't what was meant. In this case probably failure to parse should mean no one gets in, until those are corrected.

On the principle that failure to access a service would be noticed and probably soon corrected, but failure to set up security might not get noticed, and as a result overall security compromised.

comment:9 Changed 10 months ago by jchevali

Perhaps you'd like to deal with the assertion failed issue in the ticket that pege's opened, #28129 (although that same stack trace was presented here earlier), and leave this ticket for the other issues, namely, how should we investigate invalid descriptors (by the server that doesn't crash -- i.e., if given its characteristics, e.g., no GCC, no IPv4, etc., you still deem it worthy of testing), and whether padded content should be allowed (after all under the base32 definition I believe it's legal), and whether padded entries or otherwise invalid entries (except comments) should, when they're not among otherwise valid entries, result in an unsecured service or a secured (closed) one.

comment:10 Changed 10 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.6.x-final

comment:11 Changed 10 months ago by jchevali

I should add with regards to comment nº 4 above, that this server is only apparently producing invalid descriptors (or so the client says) for the HS that has server-side authorization defined on it. For others without access defined the descriptors are fine and the service(s) can be used successfully.

This assuming they're invalid and it's not just a question of not being able to decrypt. In which case the client message should be changed. What I'd mostly do to change it is to remove the long verbatim descriptor from the error message. Even with truncation, this makes the message extremely long. I don't think there's a need to, or it should be truncated to only a few hundred characters. (If it's truncated in the first place surely there's no question of copying it from the message and into a manually operated decryption tool, so not even for developers could the extra length be of help.)

comment:12 Changed 10 months ago by jchevali

Today for the first time following PR-415 I'm able to bring a secured hidden service up without the server crashing (you can't blame if before I thought the server was crashing due to a padding intolerance, since that's what it appeared to do: it'd only crash if the public key that protected the service was unpadded).

Unfortunately I'm still not able to check that a client can connect to that, since the client is still showing the error (from comment nº 4), even though it has a matching private key for the HS and the server can bring it up.

I guess I might have to wait for a further pull request (written for a bug, perhaps, still to be logged) to be able to use a secured hidden service end-to-end for the first time.

comment:13 Changed 10 months ago by jchevali

At least I'm able to dispel my doubts that an unreliable server of mine might be causing this issues, since now the HS to which the client can't connect is served right from the server where Tor 0.3.5.3-alpha was built (and with a history of building and using it for about a year).

comment:14 Changed 10 months ago by jchevali

Again an issue described here already present elsewhere so that the scope of this bug may shrink:

#27550 (hs-v3: Don't warn so loudly when tor is unable to decode a descriptor)

comment:15 Changed 10 months ago by nickm

Milestone: Tor: 0.3.6.x-finalTor: 0.4.0.x-final

Tor 0.3.6.x has been renamed to 0.4.0.x.

comment:16 Changed 9 months ago by jchevali

Several issues have been mentioned in this ticket, but I'd like to give a bit of graphical expression to the issue I described 'formally' in comment nº 8.

There are in my view two ways in which a given Tor service security lines can fail to be understood. When the whole line can't be understood, and when only the key part can't be understood, but the rest of the line can.

So suppose we have an .auth file server-side under authorized_clients some of whose lines are properly formed up to the second colon ("descriptor:x25519:"), but not any further.

For example, this case we came up recently, which is invalid because the key is hex, not base32. However, the intention was clearly here to set up security:

descriptor:x25519:5c8eac469bb3f1b85bc7cd893f52dc42a9ab66f1b02b5ce6a68e9b175d3bb433

Or the case at the centre of this ticket, where the client key is base32 but has padding, which currently makes it invalid:

descriptor:x25519:OM7TGIVRYMY6PFX6GAC6ATRTA5U6WW6U7A4ZNHQDI6OVL52XVV2Q====

A further could be a key that someone wrongly pastes, e.g., someone could paste just part of the string, maybe missing only one digit, or by mistake paste it with a space in it:

descriptor:x25519:OM7TGIVRYMY6PFX6GAC6ATRTA5U6WW6U7A4ZNHQDI6OVL52XVV2

or

descriptor:x25519: OM7TGIVRYMY6PFX6GAC6ATRTA5U6WW6U7A4ZNHQDI6OVL52XVV2Q

My argument is that in these cases, security for the hidden service in question should still be activated, and if the only keys that are present can't be parsed (all of them), then no one should be allowed in.

Because we currently deal with the case when there are keys and some are valid, which we handle fine (the valid ones will take effect and the invalid are ignored), and when there are altogether no valid lines (security for the service overall won't be activated). But we might be failing to handle the intermediate case: when security is defined but the key parsing (and only that) fails (for all keys found, be that just one or more than one). In that case we should throw away the unparseable keys, but not throw away the fact that security has been defined for the service (by way of an .auth file in the right place and apparently legal except for the keys). In this case, overall security for the service should be turned on, and access for anybody trying to access it turned off.

Because the alternative is that if someone tries to set up security for a service and doesn't get it exactly right, his service will be wide open. And he won't notice this fault, perhaps, because his clients will have access. He'll think his clients will have access by virtue of the keys given, but in fact they'll have access because everybody will have access. That can be very dangerous.

comment:17 Changed 6 months ago by nickm

Keywords: 040-deferred-20190220 added
Milestone: Tor: 0.4.0.x-finalTor: unspecified

Deferring 51 tickets from 0.4.0.x-final. Tagging them with 040-deferred-20190220 for visibility. These are the tickets that did not get 040-must, 040-can, or tor-ci.

Note: See TracTickets for help on using tickets.