Skip to content

Commit

Permalink
auth/KeyRing: try to decode as plaintext first
Browse files Browse the repository at this point in the history
otherwise i have following ASan error when compiling
the tree with ASan enabled.

==1086666==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffe896c364 at pc 0x7ffff76253ae bp 0x7fffe896c330 sp 0x7fffe896bae0
    #0 0x7ffff76253ad in __interceptor_sigaltstack ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9996
    #1 0x7ffff7687163 in __asan::PlatformUnpoisonStacks() ../../../../src/libsanitizer/asan/asan_posix.cpp:44
    #2 0x7ffff768be6c in __asan_handle_no_return ../../../../src/libsanitizer/asan/asan_rtl.cpp:612
    #3 0x555570b14515 in EntityName::decode(ceph::buffer::v15_2_0::list::iterator_impl<true>&) ../src/common/entity_name.h:39
    #4 0x555570b14626 in decode(EntityName&, ceph::buffer::v15_2_0::list::iterator_impl<true>&) ../src/common/entity_name.h:88
    #5 0x555571e5f579 in std::enable_if<(!denc_traits<EntityName, void>::supported)||(!denc_traits<EntityAuth, void>::supported), void>::type ceph::decode<EntityName, EntityAuth, std::less<EntityName>, std::allocator<std::pair<EntityName const, EntityAuth> >, denc_traits<
EntityName, void>, denc_traits<EntityAuth, void> >(std::map<EntityName, EntityAuth, std::less<EntityName>, std::allocator<std::pair<EntityName const, EntityAuth> > >&, ceph::buffer::v15_2_0::list::iterator_impl<true>&) ../src/include/encoding.h:1046
    #6 0x555571e5a637 in KeyRing::decode(ceph::buffer::v15_2_0::list::iterator_impl<true>&) ../src/auth/KeyRing.cc:210
    #7 0x555571e5b0e4 in KeyRing::load(crimson::common::CephContext*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ../src/auth/KeyRing.cc:232
    #8 0x555571e5438a in KeyRing::from_ceph_context(crimson::common::CephContext*) ../src/auth/KeyRing.cc:48
    #9 0x5555721163b8 in AuthRegistry::_refresh_config() ../src/auth/AuthRegistry.cc:163
    #10 0x555571efa019 in AuthRegistry::refresh_config() ../src/auth/AuthRegistry.h:46
    ceph#11 0x555571eae4fc in crimson::mon::Client::start() ../src/crimson/mon/MonClient.cc:423
    ceph#12 0x55556e87d73b in operator() ../src/crimson/osd/main.cc:160
    ceph#13 0x55556e896b10 in __invoke_impl<void, fetch_config()::<lambda()> > /usr/include/c++/11/bits/invoke.h:61
    ceph#14 0x55556e8934eb in __invoke<fetch_config()::<lambda()> > /usr/include/c++/11/bits/invoke.h:96
    ceph#15 0x55556e88f2a3 in __apply_impl<fetch_config()::<lambda()>, std::tuple<> > /usr/include/c++/11/tuple:1806
    ceph#16 0x55556e88f313 in apply<fetch_config()::<lambda()>, std::tuple<> > /usr/include/c++/11/tuple:1817
    ceph#17 0x55556e88f3b4 in apply<fetch_config()::<lambda()> > ../src/seastar/include/seastar/core/future.hh:2099
    ceph#18 0x55556e88980c in operator() ../src/seastar/include/seastar/core/thread.hh:258
    ceph#19 0x55556e8995d7 in call ../src/seastar/include/seastar/util/noncopyable_function.hh:124
    ceph#20 0x555574f5c8fe in seastar::noncopyable_function<void ()>::operator()() const ../src/seastar/include/seastar/util/noncopyable_function.hh:209
    ceph#21 0x5555754089ea in seastar::thread_context::main() ../src/seastar/src/core/thread.cc:299
0x7fffe896c364 is located 246628 bytes inside of 262144-byte region [0x7fffe8930000,0x7fffe8970000)
allocated by thread T0 here:
    #0 0x7ffff76825df in __interceptor_aligned_alloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:192

SUMMARY: AddressSanitizer: stack-buffer-overflow ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9996 in __interceptor_sigaltstack

the root cause is that when we decode a KeyRing as a binary blob, we first
decode the struct_v and then decode as remainder into a std::map<EntityName,
EntityAuth>. if the buffer being decoded is a actually a plaintext, there is
good chance the number of items of the key would be a huge number, and the
decoder of map<> just following the instruction and try to decode all of them
until reaching the end of buffer. but we don't actually check the boundary of
bufferlist when decoding it, and we move across the boundary of the bufferlist,
we are accessing the forbidden bits..

to workaround this issue, in this change, we try to decode the KeyRing as
plaintext first, and if it fails to decode, we try to decode as a binary blob.

this change does not address the ASan issue, it just alleviates it. unless
we have a magic number in front of the bufferlist denoting if the keyring
blob is in plaintext or binary, it's difficult to fully address this issue.

but we have lots of keyring persisted in existing Ceph deployment, it might be
difficult to enfoce the new keyring format outlined above.

Signed-off-by: Kefu Chai <kchai@redhat.com>
  • Loading branch information
tchaikov committed Jun 1, 2021
1 parent 023dcc1 commit f7df850
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/auth/KeyRing.cc
Expand Up @@ -205,12 +205,12 @@ void KeyRing::decode(bufferlist::const_iterator& bl) {
__u8 struct_v;
auto start_pos = bl;
try {
decode_plaintext(start_pos);
} catch (ceph::buffer::error& err) {
keys.clear();
using ceph::decode;
decode(struct_v, bl);
decode(keys, bl);
} catch (ceph::buffer::error& err) {
keys.clear();
decode_plaintext(start_pos);
}
}

Expand Down

0 comments on commit f7df850

Please sign in to comment.