6,287 karma · joined April 14, 2015
andres [at] hn dot anarazel dot de
True in the contents of sshd logins it isn't that big, but ~500ms to get from _start() to main() isn't small either, compared to the normal cost of that phase of library startup. Their problem was that the sshd daemon fork+exec's itself to handle a connection, so they had to redo a lot of the work for each connection.
I suspect they started off with much smaller overhead and then it increased gradually, with every feature they added, just like it happens with many software projects. Here's the number of symbols being looked that a reversing effort has documented: https://github.com/smx-smx/xzre/blame/ff3ba18a39bad272ff628b... https://github.com/smx-smx/xzre/blob/ff3ba18a39bad272ff628bb...
Afaict all of this happens before there's any indication of the attacker's keys being presented - that's not visible to the fork+exec'd sshd until a lot later.
They needed to some of the work before main(), to redirect RSA_public_decrypt(). That'd have been some measurable overhead, but not close to 500ms. The rest of the startup could have been deferred until after RSA_public_decrypt() was presented with something looking like their key as part of the ssh certificate.
> but the backdoor made each login attempt use a significant amount of CPU time to check whether it was an encrypted request from the attacker
Absurdly enough, the high cpu usage is well before it even tries to figure that out. Due to avoiding "suspicious" strings in both the binary and memory, it has a more expensive string matching routine. Finding all the symbols it from the in-memory symbol tables ends up slow due to that.
That's why even sshd -h (i.e. printing help) is slow when started in the right environment. There's not enough visibility into other things that early during process startup (this happens long before main() is called), so they couldn't check what key is being presented or such. They really "should" have deferred much more of their initialization until after the ed448 check happened.
> (and this particular machine had its sshd open to the Internet because it was a cloud machine being accessed via ssh through the open Internet)
Unfortunately not. It's a machine I have at home, with some port of my public IP redirected to it, as I often need it when not at home. Oddly enough, my threat model did not include getting attacked with something as sophisticated as this, so I thought that was fine (only pubkey, no root).
Yes. Mostly they should have reduced the cost of starting up sshd with the backdoor. A lot of that seems to be due to all the symbol lookups they needed to do, while staying obfuscated. It feels like they started with a reasonable set of features and then just piled on more and more, leading to the noticeable cpu usage.
I think the valgrind warnings were only triggered when using -fno-omit-frame-pointers. Which, at the time they wrote this stuff, wasn't the default anywhere. They got unlucky in that Fedora changed to default to that and that I happened to have that set in my valgrind tests.
> What was the moment like, when you realized you have stumbled upon a backdoor? I mean, it is riveting just to read the various reports of this backdoor!
It was many hours of slowly figuring that out, room for different emotions. Lots of nervous cackling. Thinking I must just be hallucinating. Worry about how to deal with this. And more...
Edit: Grammar
I view it as being OSS or postgresql dev that happens to work at microsoft. I've been doing the former for much longer (starting somewhere between 2005 and 2008, depending on how you count) than the latter (2019-12).
Possible but unlikely.
> I read through the report, but what wasn't directly clear to me was: what does the exploit actually do?
It injects code that runs early during sshd connection establishment. Likely allowing remote code execution if you know the right magic to send to the server.
It looks like the backdoor "deactivates" itself when it detects being started interactively, as a security researcher might. I was eventually able to circumvent that, but unless you do so, it'll not be active when started interactively.
However, the backdoor would also be active if you started it with an shell script (as the traditional sys-v rc scripts did) outside the context of an interactive shell, as TERM wouldn't be set either in that context.
> Maybe some distributions that don't use systemd strip the libxz code from the upstream OpenSSH release, but I wouldn't bet on it if a fix is available.
There's no xz code in openssh.
Practically speaking it can't - For one the script injected into the build process tests that you're running on x86-64 linux, for another, the injected code is elf code, which wouldn't link on a mac. It also needs to manipulate dynamic linker datastructures, which would also not work the same on a mac.
> This could perhaps affect more than just sshd on x86-64 linux?
This however is true - /usr/sbin/sshd was the only argv[0] value that I found to "work", but it's possible there are others. "/usr/sbin/sshd" isn't a string directly visible in the injected code, so it's hard to tell.
> > If we were to provide zig consumable ways of creating the struct from C headers, would that help?
> Yes, that would be awesome.
I don't know much about zig and won't have a whole lot of time to learn - but if you can outline what is required to make C macros [un]usable, it might be possible to improve something. Either on its own, or as part of future work.
> I'm curious how they will look like in the future.
There's quite a few things.
For one, I'd like to introduce a faster function calling infrastructure for the simple cases (mainly small number of arguments, without SRF support). That'll need to be declared in the function info struct.
For another, eventually I want to support a different encoding for variable length types. Including making it reasonably efficient to have variable-length integers.
> We have had the most difficulties with the module magic, function info, and varatt/Datum macros. Fortunately you have to solve the module magic/function info "only once". The Datum conversion and VARATT macros are more troublesome. We have some conversion support for a number of common zig types. But ideally we would like users to be able to use the C APIs as is, while we provide some type directed default conversions for convenience.
Ugh, the varatt stuff doesn't look easily maintainable long-term. It looks like you just need it for getDatumTextSliceZ()?
At the same time, I don't really know why you need it? Most of this should be doable via C functions, and the parts that are not, you could easily wrap yourself - you already seem to have some C code as part of pgzx.
Edit: Minor grammar fix
E.g. you have code to maintain the module magic and function info, for both of which there are some plans to change the contents in the next few major versions. If we were to provide zig consumable ways of creating the struct from C headers, would that help?
- data in WAL segments has to be checkpointed
- no replication slot, physical or logical, may require the WAL file (see the pg_replication_slots view)
- archiving, if configured, has to have archived the file (see pg_stat_archiver)
It used to be more complicated, for historical reasons we used to keep two checkpoints worth of WAL around, but I don't think any supported versions of postgres still have that behavior.
Edit:
What's more mysterious is whether WAL files are removed when not necessary, or whether they're recycled (renamed to be reused). That's indeed a bit hard to get insight to.
That's not really correct - postgres' MVCC implementation doesn't read from the WAL. Sure, row changes are WAL logged, but that's not really related to MVCC.
It does, but I restarted postgres and cleared the OS cache.
Btw, the primary bottleneck for the array-ified query is the unnest() handling in the functions. The minimal thing would be to make the functions faster, e.g. via:
CREATE OR REPLACE FUNCTION array_avg(_data anyarray) RETURNS numeric IMMUTABLE PARALLEL SAFE LANGUAGE sql AS $$ select avg(a) from (SELECT unnest(_data) as a) $$;
But that way the unnest is still done 3x. Something like
SELECT a_min, a_max, a_avg FROM temps_by_city, LATERAL (SELECT min(u) a_min, max(u) a_max, avg(u) a_avg FROM (SELECT unnest(array_agg) u)) limit 5;
should be faster.We (postgres) should fix that at some point... The difference basically is that there's a dedicated path to insert many tuples at once that's often used by COPY that isn't used by INSERT INTO ... SELECT. The logic for determining when that optimization is correct (consider e.g. after-insert per-row triggers, the trigger invocation for row N may not yet see row N+1) is specific to COPY right now. We need to generalize it to be usable in more places.
To be fair, part of the reason the COPY approach is faster is that the generate_series() query actually uses a fair bit of CPU on its own, and the piped psql's lead to the data generation and data loading being run separately. Of course, partially paying for that by needing to serialize/deserialize the data and handling all the data in four processes.
When doing the COPYs separately to/from a file, it actually takes longer to generate the data than loading the data into an unlogged table.
# COPY (SELECT (1000*random())::int2 as city, (random()*random()*1100)::int2 temp FROM (SELECT generate_series(1,1e8::int8))) TO '/tmp/data.pgcopy' WITH BINARY;
COPY 100000000
Time: 21560.956 ms (00:21.561)
# BEGIN;DROP TABLE IF EXISTS temps_int2; CREATE UNLOGGED TABLE temps_int2 (city int2 NOT NULL, temp int2 NOT NULL); COPY temps_int2 FROM '/tmp/data.pgcopy' WITH BINARY;COMMIT;
BEGIN
Time: 0.128 ms
DROP TABLE
Time: 0.752 ms
CREATE TABLE
Time: 0.609 ms
COPY 100000000
Time: 18874.010 ms (00:18.874)
COMMIT
Time: 229.650 ms
Loading into a logged table is a bit slower, at 20250.835 ms.> Of course, if we really want to "go fast" then we want parallel loading, which means firing up N postgresql backends and each generate and write the data concurrently to different tables, then each compute a partial summary in N summary tables, and finally merge them together.
With PG >= 16, you need a fair bit of concurrency to hit bottlenecks due to multiple backends loading data into the same table with COPY. On my ~4 year old workstation I reach over 3GB/s, with a bit more work we can get higher. Before that the limit was a lot lower.
If I use large enough shared buffers so that IO does not become a bottleneck, I can load the 1e9 rows fairly quickly in parallel, using pgbench:
c=20; psql -Xq -c "COPY (SELECT (1000*random())::int2 as city, (random()*random()*1100)::int2 temp FROM (SELECT generate_series(1,1e6::int8))) TO '/tmp/data-1e6.pgcopy' WITH BINARY;" -c 'DROP TABLE IF EXISTS temps_int2; CREATE UNLOGGED TABLE temps_int2 (city int2 NOT NULL, temp int2 NOT NULL); ' && time pgbench -c$c -j$c -n -f <( echo "COPY temps_int2 FROM '/tmp/data-1e6.pgcopy' WITH BINARY;" ) -t $((1000/${c})) -P1
real 0m26.486s
That's just 1.2GB/s, because the bottleneck is the per-row and per-field processing, due to their narrowness.> The ultimate would be to hack into postgres and skip everything and just write the actual filesystem files in-place using knowledge of the file formats, then "wire in" these files to the database system tables. This normally gets hairy (e.g. TOAST) but with a simple table like this, it might be possible. This is a project I've always wanted to try.
I doubt that will ever be a good idea. For one, the row metadata contain transactional information, that'd be hard to create correctly outside of postgres. It'd also be too easy to cause issues with corrupted data.
However, there's a lot we could do to speed up data loading performance further. The parsing that COPY does, uhm, show signs of iterative development over decades. Absurdly enough, that's where the bottleneck most commonly is right now. I'm reasonably confident that there's at least 3-4x possible without going to particularly extreme lengths. I think there's also at least a not-too-hard 2x for the portion of loading loading data into the table.
I think it actually shows that you're IO bound (the 'D' in the 'S' column). On my workstation the query takes ~10.9s after restarting postgres and dropping the os caches. And this is a four year old CPU that wasn't top of the line at the time either.
> postgres=# explain select city, min(array_min(array_agg)), avg(array_avg(array_agg)), max(array_max(array_agg)) from temps_by_city group by 1 order by 1 ;
Note that you dropped the limit 5 here. This causes the query to be a good bit slower, the expensive part here is all the array unnesting, which only needs to happen for the actually selected cities.
On my workstation the above takes 1.8s after adding the limit 5.
1) use the integer version of generate_series, the 1e9 leads to the floating point version being chosen
2) move the generate_series() to the select list of a subselect - for boring reasons, that we should fix, the FROM version materializes the result first
3) using COPY is much faster, however a bit awkward to write
psql -Xq -c 'COPY (SELECT (1000random())::int2 as city, (random()random()*1100)::int2 temp FROM (SELECT generate_series(1,1e9::int8))) TO STDOUT WITH BINARY' | psql -Xq -c 'COPY temps_int2 FROM STDIN WITH BINARY'