Seth For Privacy: can you dm me ? could really use your help on something

meeting in about 2 hours

Hey folks,it's meeting time. The agenda is here: monero-project/meta #703

This should be a quick meeting for updates, so let's get to it. But first, a round of greetings to see who is here

hi all

hi

Hello

hello

hi

hi

Posting the updated plan from last meeting's logs, so we all have it:

Branch/feature complete: May 16th, 2022... (full message at libera.ems.host/_matrix/media/r0/do…0ce9ba32f53f5a8f455ac0a7133e7869dbe)

hi

Thank you all for coming

You release before testnet ?

Oh nvm, the list isn't ordered. Ignore me.

Branch/feature complete: May 16th, 2022... (full message at libera.ems.host/_matrix/media/r0/do…dfae93fe69e46c9dc876f7f2ad8ab61bb6f)

hi

^ ordered to avoid confusion

So, master is expected to be feature complete by Monday. What's the current status?

.merges

seems like we're close to quiescent\

8149 does not really have enough review

What type of review are we talking about UkoeHB? I thought KayabaNerve's crypto review was the only big review missing.

Like, it's pretty much just me and a some from vtnerd (the original author flaked, and there have been some changes, rebases, cleanup since his original work).

ErCiccione: comprehensive review of the core issues

right, so it's very unlikely that will be reviewed by monday

we should move up branching then, not ideal, but that fix cannot really be postponed

and the testnet hard fork as well

the multisig fix?

yes: monero-project/monero #8149

Who are the possible candidates for that final review then?

unless we find somebody before then. Can we?

The multisig patch does not require a fork, technically. So it can be released in a subsequent version whenever someone with a clue has reviewed it.

yeah but the hard fork would enforce it to everybody, which especially in this case would be important

technically the correct play would be disabling multisig until it can be reviewed properly

seems a 3rd party audit is the ideal route to me at this point

in wallets?

3rd party audit is unlikely to be doable within a single month

that kind of excludes multisig from the rest of the timetable

afaiu, can't really disable at the network / protocol level

disabling multisig sounds quite extreme. We don't know who uses it, beside being a huge problem for haveno and rino

it's just disabling something that can't be used anyway (until/if something that works can be merged)

Well, it does work, but it has exploitable weaknesses, which are still 2 different things IMHO

if disabled, then the exploits can't be used

One could add a "Multisig is currently dangerous if condition1 condition2 etc". Use "please !*" to confirm. And add a please command that redirects its params to the worker func, bypassing the message.

AFAIK the condition is "you don't trust all members", right ?

yes

If so, exchanges should be alright using it.

^ much better option. Disabling a feature that we don't know who uses it is a dangerous path

moneromooo: is that something you could work on?

sorry to be stupid about this. what do you mean by disable

gingeropolous: make the calls to sign multisig txs throw

I suppose I could. Easy enough so I don't get brain on fire.

UkoeHB, in a wallet implementation?

Thankfully the CLI is the only wallet where everybody can freely multisig

No way I can usefully review the patch btw, before you ask (sorry).

yeah that's ok

my point is that its not consensus level. Further, with the notion that the core implementation of the software should provide "the best", disable it with the release.

if someone really wants to do it, they can use an old wallet or compile whatever

its not like it can really be shut off

alright, so that is sorted. Now the thing is, do we want to go on without multisig in the hard fork then? Sounds like we don't really have a choice, as we lack reviewers

I thought the idea is to go with multisig, but with a big fat warning?

Actually, I'll make a "enable_multisig" bool setting in the wallet, off by default. Simpler, more user friendly, and works for RPC in a transparent way.

moneromooo: should be good, with a confirmation message that says "multisig is experimental and possibly exploitable, use at your own risk"?

Yes.

Feel free to paste me a message that explains the risks.

ok

alright, so

we have no choice than to not include 8149 in the network upgrade

but we really really have to activate the community in search for a reviewer

if we take that route of 'experiment and risky', then merging 8149 early may be ok since it is significantly less risky than the current stuff

multisig is crucial for importantant projects currently being built, so IMO (yes, i'm biased), that should be a priority

UkoeHB: That sounds reasonable. Opinions?

With "early" you mean tomorrow, right? For branching on Monday

it's a risky approach, but if we already flag it as insecure might have sense

sound reasonable to me for the release. as far as getting reviewers, why not tap past auditors who audited BP/BP+?

I would merge that any day, frankly

maybe? with follow-up PRs to clean up anything reviewers find...

that sounds ok

if they get feedback to us in time it can be fixed in the branch before release

if not, not.

do we have consensus on this? (merge 8149 by tomorrow, add the warning message)

jberman[m]: the problem is third-party auditors don't have a broad insight into the monero protocol or code stack; they are great for targeted audits (e.g. proof implementations), but it isn't clear they are great for broader reviews (I could be wrong)

id say if 8149 is in without default disable, the user prompt / warning should be like multiple OKs

er, i meant default disable

because I know some software users that just click through things very quickly....

UkoeHB: Would be nice to start scaling something like this up so Monero does have better audit options in the future

well, I don't know how to do that lol

UkoeHB: Would it make sense to ask to audit the PR, after it's merged? It's a much narrower scope, but still useful

narrowing scope sounds reasonable\

I can squash 8149 on monday. Someone should at least check I am not inserting some shit into the commits lol.

We can hardfork testnet on the master branch and postpone branching for a small number of days, technically, right?

ErCiccione: we can probably request an audit for just the signing component (making CLSAGs); everything about the tx protocol is really our domain

Would be a good start IMO

we still need the core parts reviewed as far as i understood, but the two things could go in parallel no?

the original PR author said he was going to write some documentation/proofs for the signing component (which would be very useful for auditing), but...

yes

yeah disappeared, leaving some critical code right there. Not an ideal situation

anyway, let's go on then. As far as i can tell, we are all fine with merging the pr as it is,

there are different opinions about a warning or disabling entirely. I strongly favour the former, for the reasons i mentioned above

enabling via wallet setting, disabled by default, sounds best

works for me

+1

Yup

that shouldn't disrupt current users right?

they might have to update their usage to enable signing

which would be good, so users will gain more awareness (if such users even exist)

we should definitely let them know as soon as they try to use multisig though

if that's what people prefer, but looks like it

that = disabling it by default

anything else in the agenda?

Unfortunately my #8076 is still not ready. There is special case that can lead to a sensitivity info leak, and another special case where a tx can go unreported

I can work on it tomorrow, but not sure I will be able to complete

jberman 's eagle eyes spotted those cases :)

Which is good of course

alright. Anything else to discuss related to the netwrok upgrade?

good news: I've identified some daemon reliability fixes that are super small/easy to fix that I'm hoping we can get included as well (8324, 8326, and 1 more possibly 2 more incoming today). Running the fixes, I don't see the warnings of 6938 anymore and don't have tx submission issues

sounds promising

Nice! Hopefully they can be reviewed before Monday

anything else? Otherwise we can conclude here

btw, news people of Monero (Monero observer, monero moon etc), please point out in your reports that we need community actions to find reviewers for multisig, it wiwould help 🙂

Can't to harm, yes

*do

s/wiwould/would/

alright, thanks everyone for coming. Enjoy the rest of your saturday

the meeting is over, but obviously feel free to continue the conversations above 🙂

so should wallet give the scary warning when toggling the multisig enable setting?

"WARNING: if this was supposed to be on, it would not be off by default, you nincompoop."

I think that's a good moment, yes

i mean, at what point does a lack of interest in reviewing translate to a lack of interest in the feature?

there is no lack of interest in the future. Two projects depend on it

But it's fairly pointless since the way you'll know about this is when a ms command moans at you anyway.

we both already set bounties about it

Learned something, "nincompoop"

maybe when toggling the multisig enable setting, and also a warning when doing the multisig stuff. in case ppl forget

"oh i haven't seen a warning about this in a while. must be fine"

perhaps could also include in the error message "if you can help, review is needed" with a link to the github or whatever

presumably, if multisig is as used as it is, users that use it will encounter these warnings, and some n% might say "hey this warning is stupid ima fix it"

sounds reasonable. optimistic...

u know, that ven diagram of users of multisig and those with c++ and maths skills

Oooh. I could check whether gcc is installed, and then regularly nag about reviewing!

:)

gdb maybe. gcc is probably installed even on ubuntu type distros.

"hey, i see you have gdb installed. maybe you can help. There are bounties you know....."

Now you get creative :)

your review nag should also obvi include the ascii cow

the nag should be a cowsay, definitely

Now we're talking.

Sucks I missed this, but I explicitly believe 8149 should be merged as a marked improvement over what stands. While I won't say it doesn't need further review, and an audit would be preferred, it has no known issues. I think a fair compromise, to not repeat the previous multisig, would be removed with a CLI flag to enable, and then we submit a minor release post-sufficient review to not require a CLI flag. It does sound like that's

the plan, just wanted to chime in again.

I'd rather the flag itself communicate, rather than (or with) any message.

--enable-multisig-experimemtal

I also did review the cryptography, mainly with regards to Drijver's and confirming nonce handling, but I'm planning another review due to a concern that was raised. I'm just booked the next week or so :/

I'll try to write informal notes of the protocol as I do and I'd like to raise a note about audit isolation. I submitted a bug in the transaction protocol explicit to multisig that had nothing to do with CLSAG. While it's now fixed, an audit solely doing CLSAG cannot be argued as comprehensive.

With viewtags would the inputs with same tag as output tag would be identifiable? Or inputs are all with same tag ?

I'd also like to establish bounds on privacy de-anon on the blockchain/p2p level. I don't believe any currently exist but...

> <@nikg83:matrix.org> With viewtags would the inputs with same tag as output tag would be identifiable? Or inputs are all with same tag ?

>

>

View tags are effectively random on a per output basis. There's only a 1/255 chance an output lines up with a given ring member and that doesn't reveal anything other than chance

I think. I saw jberman[m]: typing as I hit spend and he'd definitely be the expert here 👀

1/256* :) but generally yep

D: I thought max value was 255 and forgot 0 existed :(

Thanks for the correction :p silly mistake on my end.

<ErCiccione> "we still need the core parts..." <- it looks like you're even better in cryptography than luigi1111

<kayabanerve[m]> "I also did review the cryptograp..." <- Did you audit cryptography in your own project ? How if yes ?

ooo123ooo1234[m]: Uhhhh not sure what my own project is here.

If you mean Meros, it didn't use any groundbreaking cryptography. It was Schnorr signatures and BLS. The latter I used an existing impl for, with my own as comparison, and the former is trivial. If you mean ASMR, the DLEq proof was by sarang and I didn't even write its initial implementation. From there it was just basic private key management. If you mean my current project, I actually am rolling my own crypto, and it's not audited.

It's just extensively reviewed by myself, informally, with basic tests for now to ensure it works.

So none of those really count as "auditing".

19:45 <ErCiccione> Nice! Hopefully they can be reviewed before Monday <-- the code freeze is not for bug fixes

we can and will still merge bug fixes after monday

<chesterfield[m]> "Would be nice to start scaling..." <- You probably meant scaling down number of competent people and scaling up number of loyal people

or scaling up number of frontend developers

and scaling down number of people that can at least read code

UkoeHB: monero-project/monero #8328

<jberman[m]> "1/256* :) but generally yep" <- So wallets will have outputs with different viewtags ? & when scanning the blockchain forward it will only check tx with those viewtags only?

<nikg83[m]> "So wallets will have outputs..." <- It will calculate what its view tag would be for the given output, and if it lines up, it'll continue doing checks to see if it actually received the output.

It skips processing of 255/256 outputs whose view tags aren't what they would be if it was a valid TX to them.