Skip to content

rgw & msgr fixes from 15.2.1 #34927

Merged
tchaikov merged 5 commits intomasterfrom
upstream-rgw-msgr-fixes
Jun 20, 2020
Merged

rgw & msgr fixes from 15.2.1 #34927
tchaikov merged 5 commits intomasterfrom
upstream-rgw-msgr-fixes

Conversation

@theanalyst
Copy link
Member

These were on nautilus/octopus but not on master

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

idryomov and others added 5 commits May 6, 2020 09:54
The secure mode uses AES-128-GCM with 96-bit nonces consisting of a
32-bit counter followed by a 64-bit salt.  The counter is incremented
after processing each frame, the salt is fixed for the duration of
the session.  Both are initialized from the session key generated
during session negotiation, so the counter starts with essentially
a random value.  It is allowed to wrap, and, after 2**32 frames, it
repeats, resulting in nonce reuse (the actual sequence numbers that
the messenger works with are 64-bit, so the session continues on).

Because of how GCM works, this completely breaks both confidentiality
and integrity aspects of the secure mode.  A single nonce reuse reveals
the XOR of two plaintexts and almost completely reveals the subkey
used for producing authentication tags.  After a few nonces get used
twice, all confidentiality and integrity goes out the window and the
attacker can potentially encrypt-authenticate plaintext of their
choice.

We can't easily change the nonce format to extend the counter to
64 bits (and possibly XOR it with a longer salt).  Instead, just
remember the initial nonce and cut the session before it repeats,
forcing renegotiation.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Reviewed-by: Sage Weil <sage@redhat.com>
As a AES-GCM IV, nonce_t is implicitly shared between server and
client.  Currently, if their endianness doesn't match, they are unable
to communicate in secure mode because each gets its own idea of what
the next nonce should be after the counter is incremented.

Several RFCs state that the nonce counter should be BE, but since we
use LE for everything on-disk and on-wire, make it LE.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
Reviewed-by: Sage Weil <sage@redhat.com>
Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>
Reviewed-by: Casey Bodley <cbodley@redhat.com>
(cherry picked from commit d8dd5e513c0c62bbd7d3044d7e2eddcd897bd400)
As per Robin's comments and S3 spec

Signed-off-by: Abhishek Lekshmanan <abhishek@suse.com>
S3 GetObject permits overriding response header values, but those inputs
need to be validated to insure only characters that are valid in an HTTP
header value are present.

Credit: Initial vulnerability discovery by William Bowling (@wcbowling)
Credit: Further vulnerability discovery by Robin H. Johnson <rjohnson@digitalocean.com>
Signed-off-by: Robin H. Johnson <rjohnson@digitalocean.com>
@theanalyst theanalyst requested review from cbodley, idryomov and jdurgin May 6, 2020 14:09
@idryomov
Copy link
Contributor

idryomov commented May 6, 2020

Huh, I could have sworn I saw them on master -- and indicated so in more than one conversation. Must have confused with the octopus branch that I had checked out for a while because of unrelated backport work, sorry!

@batrick batrick added messenger Issues involving one of the Ceph messenger implementations needs-qa rgw labels May 6, 2020
@theanalyst
Copy link
Member Author

Huh, I could have sworn I saw them on master -- and indicated so in more than one conversation. Must have confused with the octopus branch that I had checked out for a while because of unrelated backport work, sorry!

Yes, I also assumed they were still in the octopus -> master merge cycle, and forgot an explicit cherry-pick to master

@idryomov
Copy link
Contributor

@neha-ojha, @tchaikov This is tagged with messenger, so probably went under your radar. Could you please pull it into your integration branches?

@neha-ojha neha-ojha added the core label May 20, 2020
@neha-ojha
Copy link
Member

@neha-ojha, @tchaikov This is tagged with messenger, so probably went under your radar. Could you please pull it into your integration branches?

sure, added the core label as well. The rados suite has a lot of noise right now, we'll test this once those clear up.

@tchaikov
Copy link
Contributor

@cbodley Casey, could you include this changset in your next batch?

@idryomov
Copy link
Contributor

@neha-ojha Ping, do we need another RADOS run here?

@cbodley What is the status on your end? Do you want to get this through rgw suite or are you OK with us merging?

@neha-ojha
Copy link
Member

@neha-ojha Ping, do we need another RADOS run here?

don't think so, I am not seeing any changes since @tchaikov tested it

@cbodley What is the status on your end? Do you want to get this through rgw suite or are you OK with us merging?

@idryomov
Copy link
Contributor

@neha-ojha Sorry, I'm not clear -- do you expect changes to this PR or are you OK with the results of Kefu's run?

@neha-ojha
Copy link
Member

neha-ojha commented Jun 11, 2020

@neha-ojha Sorry, I'm not clear -- do you expect changes to this PR or are you OK with the results of Kefu's run?

@idryomov I meant that Kefu's results looked good and nothing in the PR has changed since then that would require us to retest it.

@tchaikov tchaikov merged commit 2f30093 into master Jun 20, 2020
@tchaikov tchaikov deleted the upstream-rgw-msgr-fixes branch June 20, 2020 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core messenger Issues involving one of the Ceph messenger implementations needs-qa rgw

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants