Il sab 5 ott 2019, 03:20 Karan Mehta <k.me...@salesforce.com.invalid> ha scritto:
> The Dos security issue is real and your comment makes sense, Enrico. > My earlier solution was to help ensure that bookie can recover from such > scenarios, mostly with a restart. I am not even sure under what conditions > does DigestManager doesn't even instantiate. > > I will break it up in following PR's. Hope this seems right. > 1. Add a global config on server side verification and always instantiate > CRC32 (hardcoded) manager. > Do you mean CRC32C? Maybe we can make it configurable. It is okay to me to use the same digest type for every ledger, but at least we should let the user choose between crc32 and crc32c. We can skip MAC as it needs a password Verify the checksum on every ADD_ENTRY and WRITE_LAC request with generic > error code StatusCode.EIO > 2. Add relevant metrics > 3. Enhance writeflags API with new bits > This should be the last one. It is okay to change the server first, but once we expose a flag in the public client API we must support it fully > 4. Handle addEntry code support for new writeFlags on client/server side. > 5. Enhance error handling on client side (if required) > As theit change is very little you can simply send one single patch for the client side (API and errors) Enrico > > Will add relevant test cases as well. > > On Fri, Oct 4, 2019 at 6:53 AM Venkateswara Rao Jujjuri <jujj...@gmail.com > > > wrote: > > > On Fri, Oct 4, 2019 at 12:16 AM Enrico Olivelli <eolive...@gmail.com> > > wrote: > > > > > Karan, > > > > > > Il giorno gio 3 ott 2019 alle ore 23:41 Karan Mehta > > > <k.me...@salesforce.com.invalid> ha scritto: > > > > > > > Thanks for the feedback. I will create a BP soon. > > > > > > > > > I won't shutdown the bookie, simply fail the write. It may happen > in > > > case > > > > of a partial upgrade of the cluster and a write with a new digest > type > > > > comes to the bookie > > > > > > > > Interesting point. As per my assumptions, `All the options assume > that > > > the > > > > server version will be greater than client version.`, this should not > > > > happen. > > > > I guessed most organisations operate and release in that fashion. I > can > > > > confirm for Salesforce. If you believe that is not the case, we > should > > > > discuss. > > > > > > > > > > > > > Usually the answer is "yes", the server should be upgraded before > > upgrading > > > the client. > > > But currently latest clients are compatible with older servers as far > as > > > they do not use new features. > > > This is a cool "feature", in BookKeeper ecosystem we have very > different > > > applications. > > > In my case it is possible that an application is using an older > cluster. > > > > > > Apart fro that consideration, the real problem is that having a client > > that > > > just asks for an unsupported digest type makes the bookie auto-shutdown > > is > > > kind of a DoS security flawn > > > > > > It is better to fail the write > > > > > > > I agree with Enrico. I am not sure why we should shut down bookie. Just > > fail the write and let > > Ops decide the corrective action. > > > > JV > > > > > > > > > > > > > > > > > > > Thinking about the future and about ideas shared with JV some month > > > ago, > > > > I lean towards having ledger metadata in the bookie. Having metadata > > > opens > > > > the way to new features, like per ledger storage type > > > > > > > > Yes it does bring those benefits, however I have two counter-args to > > it. > > > > 1. It adds a RPC call and all the potential complexities of dealing > > with > > > zk > > > > in the critical path for at least some writes (later on we can cache > > > > obviously). > > > > 2. Most of ledger storage or QoS related stuff (some of our internal > > use > > > > cases require that), can also be driven via writeFlags. Hence we > > decided > > > to > > > > opt on it. > > > > > > > > Internally we are going by the writeFlags option for now. We will > keep > > > the > > > > community posted if we make any progress and also would require your > > help > > > > to counter any challenges that we face along the way. Thank you! > > > > > > > > > > > > > I am fine with the WriteFlags option, it is consistent with current API > > and > > > WriteFlags appeared just during those discussions about "ledger > > > types"/"Qos"..... > > > > > > When sending out code please ensure to split the patch into smaller > > tasks, > > > at least two: > > > - server side changes > > > - client side changes > > > > > > You could also add an integration test about what happens when a new > > client > > > uses the new WriteFlag against an old bookie, it should receive an > error > > > > > > Enrico > > > > > > > > > > > > > > > > > > On Thu, Oct 3, 2019 at 11:12 AM Enrico Olivelli <eolive...@gmail.com > > > > > > wrote: > > > > > > > > > Thank you for sharing this work. > > > > > Two initial comments: > > > > > > > > > > Error handling: > > > > > Unable to instantiate digest manager for that type > > > > > Decline the write, shutdown itself and wait for external > orchestrator > > > to > > > > > restart > > > > > > > > > > > > > > > I won't shutdown the bookie, simply fail the write. It may happen > in > > > case > > > > > of a partial upgrade of the cluster and a write with a new digest > > type > > > > > comes to the bookie > > > > > > > > > > > > > > > Which option is better? > > > > > Thinking about the future and about ideas shared with JV some month > > > ago, > > > > I > > > > > lean towards having ledger metadata in the bookie. > > > > > Having metadata opens the way to new features, like per ledger > > storage > > > > type > > > > > > > > > > > > > > > Enrico > > > > > > > > > > Il gio 3 ott 2019, 18:44 Sijie Guo <guosi...@gmail.com> ha > scritto: > > > > > > > > > > > Hi Karan, > > > > > > > > > > > > Thank you for your proposal. Can you also add your proposal as a > BP > > > to > > > > > the > > > > > > BP list? You can check the BP process here: > > > > > > http://bookkeeper.apache.org/community/bookkeeper_proposals/ > > > > > > > > > > > > Thanks, > > > > > > Sijie > > > > > > > > > > > > On Fri, Sep 27, 2019 at 5:53 AM Karan Mehta < > > k.me...@salesforce.com > > > > > > .invalid> > > > > > > wrote: > > > > > > > > > > > > > Hello everyone, > > > > > > > > > > > > > > I wrote up a document here < > > > https://salesforce.quip.com/FmlEAnMbtjnU > > > > > > > > > > > for > > > > > > > Apache Bookkeeper Checksum Validation for the issue > > > > > > > <https://github.com/apache/bookkeeper/issues/1046>. I have > added > > > > > certain > > > > > > > options and highlighted the pros/cons of each design. I would > > like > > > to > > > > > > hear > > > > > > > everyone's thoughts on it. Feel free to comment on the doc to > > > suggest > > > > > > > ideas. Thanks for your inputs! > > > > > > > > > > > > > > -- > > > > > > > Karan Mehta > > > > > > > > > > > > > > < > > http://smart.salesforce.com/sig/k.mehta//us_mb/default/link.html> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Karan Mehta > > > > > > > > <http://smart.salesforce.com/sig/k.mehta//us_mb/default/link.html> > > > > > > > > > > > > > -- > > Jvrao > > --- > > First they ignore you, then they laugh at you, then they fight you, then > > you win. - Mahatma Gandhi > > > > > -- > Karan Mehta > > <http://smart.salesforce.com/sig/k.mehta//us_mb/default/link.html> >
