Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add -uaappend option to append a literal string to user agent #19242

Closed
wants to merge 2 commits into from

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Jun 11, 2020

This can be used by other software that controls the node instance (eg, ABCore)

@luke-jr luke-jr force-pushed the uaappend branch 4 times, most recently from 0a233b3 to ef2ae81 Compare Jun 11, 2020
@DrahtBot DrahtBot added the Tests label Jun 11, 2020
@laanwj
Copy link
Member

laanwj commented Jun 11, 2020

Can you show some usage examples and resulting UA string?

Also: For what it's worth in web browsers there's currently a trend toward simplifying user agents, creating less variants instead of more. Mostly for privacy reasons. Is signalling this extra information in the user agent useful for the user of the node, or is it simply for statistics for the developers?

@MarcoFalke
Copy link
Member

MarcoFalke commented Jun 11, 2020

Slightly tend to NACK. Might as well put in the OS, whether the full node is controlled by the GUI, ...

Sometimes remote attacks can only be exploited in a specific environment, putting that specific environment into the ua and sending it to the attacker seems counter productive.

@MarcoFalke MarcoFalke added P2P and removed Tests labels Jun 11, 2020
@luke-jr
Copy link
Member Author

luke-jr commented Jun 11, 2020

Can you show some usage examples and resulting UA string?

-uaappend ABCore:1.1.1 could produce /Satoshi:0.20.0/ABCore:1.1.1/

Sometimes remote attacks can only be exploited in a specific environment, putting that specific environment into the ua and sending it to the attacker seems counter productive.

You could argue for removing the UA entirely. But so long as it's there, there's no reason to stop other people from using it as intended. (Nobody is forcing anyone to use this if they don't want to.)

@kristapsk
Copy link
Contributor

kristapsk commented Jun 11, 2020

I agree with @luke-jr here, concept ACK.

Might as well put in the OS, whether the full node is controlled by the GUI,

This is different, it's something user puts there if he wants to (example - "UASF"), not something that gives out some information about your system by default.

@luke-jr
Copy link
Member Author

luke-jr commented Jun 11, 2020

Note we already have uacomment for user stuff. This would be just for applications built on top of Core.

@MarcoFalke
Copy link
Member

MarcoFalke commented Jun 11, 2020

Giving a footgun to a user is less bad than giving a footgun to applications built on top of Core. Explicitly offering the option makes it seem "officially supported" or even encouraged for applications to use it.

@luke-jr
Copy link
Member Author

luke-jr commented Jun 11, 2020

How about if we hide it behind -help-debug?

@MarcoFalke
Copy link
Member

MarcoFalke commented Jun 11, 2020

I simply don't see the use case to tell malicious remote peers that you are running on an Android system.

If there was a use case, then other operating systems (and other system information) should also be put in there, not only Android.

@luke-jr
Copy link
Member Author

luke-jr commented Jun 12, 2020

ABCore is not an operating system.

A better example might be Wasabi, but I'm not sure they support Core at the moment.

Or Armory, perhaps.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 21, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

luke-jr added 2 commits Aug 20, 2020
This can be used by other software that controls the node instance (eg, ABCore)
@luke-jr
Copy link
Member Author

luke-jr commented Aug 20, 2020

Rebased

@fanquake
Copy link
Member

fanquake commented Apr 12, 2021

I can't see much support for adding this, and for what it's worth I'm also a ~0/NACK. I don't think there's heaps of value to adding a "not really supported" hidden option to allow other pieces of software to add more information to our user agent. Also the examples given don't seem very convincing:

ABCore

Is this still actively developed / maintained? Seems like the website no longer exists, and the repository has a bunch of spam issues open / hasn't seen any updates for ~18 months. Has it moved somewhere else?

A better example might be Wasabi, but I'm not sure they support Core at the moment.

As mentioned, this software doesn't actually support Bitcoin Core, for reasons unrelated this this, so this seems like a poor argument for adding a feature for it to use?

Or Armory, perhaps.

Do we know if this is something that Armory actually wants/would use?

In any case, I'm going to close this for now.

@fanquake fanquake closed this Apr 12, 2021
@laanwj
Copy link
Member

laanwj commented Apr 12, 2021

don't think there's heaps of value to adding a "not really supported" hidden option to allow other pieces of software to add more information to our user agent.

I agree 100%. Also the privacy zeitgeist seems to be to strip as much as possible from user agents and other publicly visible software-identifying information, not to add more specifics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants