Portage seems to accept any sort of junk you feed into KEYWORDS, including something that has recently been in the tree: > KEYWORDS="*" Neither repoman, nur portage catched that error here. It seem it assumes stable arch or somesuch for '*'. This is against PMS afais and must be rejected, see https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-260003.1.6 https://dev.gentoo.org/~ulm/pms/head/pms.html#x1-690007.3.2 example ebuild app-misc/foo-1.0 ============== DESCRIPTION="" SLOT="0" KEYWORDS="*" ============== but not only that, you can also feed junk like > KEYWORDS="^^*##" and it will yield something like: > The following keyword changes are necessary to proceed: > (see "package.accept_keywords" in the portage(5) man page for more details) > # required by app-misc/foo (argument) > =app-misc/foo-1.0 ** This seems completely random and dangerous. It could potentially parse mis-typed keywords successfully and break users systems.
I cannot see anything related to this in PMS that would be wrong. Therefore removing pms-bugs from CC.
KEYWORDS="*" and KEYWORDS="~*" are used by Funtoo, so I'm copying Daniel here.
(In reply to Ulrich Müller from comment #1) > I cannot see anything related to this in PMS that would be wrong. Therefore > removing pms-bugs from CC. I was just CCing pms-bugs to confirm that the syntax is wrong and not allowed.
this has come up already in the past although i don't remember where. portage should continue to accept KEYWORDS="*" (and "~*") and PMS is irrelevant here. CrOS also uses it heavily because it's rare for specific arches to matter.
Echoing Vapier's comments: I originally added KEYWORDS to Portage. KEYWORDS="*" and "~*" is something that has been historically used by Gentoo for years, from the inception of KEYWORDS. Gentoo moved away from it as a matter of policy for arch maintenance, not because it was not part of the ebuild design. It is not "wrong" -- it is just against current Gentoo policy. I would imagine that * and ~* was left out of PMS because the author(s) were unfamiliar with its prior use. Funtoo Linux has used ~* and * in KEYWORDS for years, and continues to use it. It is indeed supported in Portage. Since Portage existed before PMS, and PMS was an attempt to document the functionality of Portage in a formal way, and this capability has been part of KEYWORDS since its creation, this appears to be an (intentional or not, I cannot say) omission on the part of PMS and *not* a situation where "this is against PMS and must be rejected." It appears that rather than documenting technical functionality (by looking at the actual implementation of KEYWORDS,) Gentoo's current KEYWORDS *policy* was documented in PMS instead. It is generally understood to be bad practice to make policy decisions part of technical specifications for software, as PMS has unfortunately done in this case.
(In reply to SpanKY from comment #4) I'm not sure how downstream forks are relevant for correctness of the upstream source code. Also, this is not just about KEYWORDS="*", it even parses worse things.
The "The following keyword changes are necessary to proceed" in the OP's report is expected behavior. If an ebuild has "^^*##" in KEYWORDS, Portage is going to want to see "^^*##" in ACCEPT_KEYWORDS. Otherwise, a package.keywords fix will be needed. To Portage, except for the wildcards, these are just strings and Portage looks for matches. I am not sure how this behavior can be considered dangerous.
(In reply to Daniel Robbins from comment #7) I do consider accepting junk input dangerous. There's no clear specification on how portage is going to behave when non-PMS compliant strings are passed, except the current state of the codebase which can arbitrarily change and no one understands anyway. I also tried feeding non-ascii characters and got funny tracebacks. Then portage dies at a completely unrelated stage and yields an unrelated error message, so it obviously tries to get as far as possible while carrying the junk data through numerous internal functions. I wonder if you could even forge KEYWORDS strings that let you run arbitrary commands through portage. So I'm not sure how you can not consider this dangerous.
(In reply to Julian Ospald (hasufell) from comment #6) there is no requirement that PM's must only support the subset that PMS describes. even if it were, it's clearly a stupid requirement that in practice no one has followed. hence KEYWORDS=* & ~* is a valid portage extension that people using portage use, not just in downstream distros. your dislike for it is irrelevant. your initial post focused on "*" which we've refuted. but nowhere did we say that validating KEYWORDS in someway is a bad idea. you're projecting.
Yep, the OP is the one claiming/speculating, and thus the one obligated to demonstrate, how this behavior is dangerous or a security vulnerability. This is really three bugs in one. We have dealt with the PMS policy angle, and the security conjecture angle. The one legitimate angle, which I don't think is a very big deal but might be good to fix, is that there is no master file or variable that lists what atoms are valid in KEYWORDS, I don't think. So, if a user types something that is invalid into ACCEPT_KEYWORDS or package.keywords, Portage will not recognize it as invalid, and will request a package.keywords change. It could be a bit nicer to have a variable that lists all valid KEYWORDS entries and then Portage can use this to validate these entries. This wouldn't be very burdensome as new KEYWORDS are not added very frequently. Adding this functionality would require adding a new variable, such as VALID_KEYWORDS, which would appear somewhere in a make.defaults base profile file, and which the ACCEPT_KEYWORDS setting and package.keywords lines would be validated against (but Portage would assume that KEYWORDS in ebuilds are correct, for performance reasons, and because presumably the ebuilds have been checked by repoman. We are primarily protecting the user from shooting themselves in the foot.) *This*, I could agree with.
(In reply to Daniel Robbins from comment #10) profiles/arch.list contains the list of valid keywords for a repo. repoman and some other tools (like ekeyword) use that to check for typos. the repoman angle is handled in bug 563642 (as currently it also allows "*" and its -*/~* forms in the main tree which we want to disallow by current Gentoo policy). let's throw away the "*" is invalid part and focus on two things: (1) regardless of the content of KEYWORDS, portage should not crash (2) portage should accept [[:alnum:][:space:]+_.*-] and reject KEYWORDS not matching that (i don't think this breaks any existing/historical use cases)
(In reply to SpanKY from comment #11) I agree with (1). For (2), if we have a list of valid atoms for KEYWORDS, I think it would only help to use these to validate user settings. I realize that this is only currently used by repoman but would make sense for Portage to use these as well. One of the things we would want to detect are typos that may pass the regex test.
(Note that I probably would suggest migrating the valid KEYWORD atoms list to a make.defaults variable in order to ease implementation.)
(In reply to Daniel Robbins from comment #10) > Yep, the OP is the one claiming/speculating, and thus the one obligated to > demonstrate, how this behavior is dangerous or a security vulnerability. > Not validating input is _always_ dangerous, no matter if there already is a known security vulnerability or not. I'm not sure if you are serious about this statement. > This is really three bugs in one. We have dealt with the PMS policy angle, > and the security conjecture angle. > I don't see where. The portage and PMS teams haven't commented.
(In reply to SpanKY from comment #9) > > there is no requirement that PM's must only support the subset that PMS > describes Even if it supports more, no ebuild in the gentoo tree must ever rely on that behavior and that includes KEYWORDS="*". And the PM must ensure that this gets ultimately rejected for the gentoo tree. I don't think adding "spec-files" to the repository is a clean solution, but probably better than none. It would would make more sense to have this behind an unsupported FEATURES flag.
(In reply to Julian Ospald (hasufell) from comment #15) > Not validating input is _always_ dangerous, no matter if there already is a > known security vulnerability or not. I'm not sure if you are serious about > this statement. If it's always dangerous as you claim, then demonstrate it. If you are right, then it should be easy to do. Here is my perspective. It's not dangerous if 1) the file is owned by root (user would need to be root already to modify it) and 2) the input in question is treated as a simple string and not evaluated as code. Both of which are true here. Feel free to argue these points above logically, rather than question my 'seriousness' (not a good debate tactic, btw. Not that we should treat Gentoo bugs as a forum for debating... but rather as a logical and impersonal exploration of a particular problem.)
(In reply to Daniel Robbins from comment #17) > (In reply to Julian Ospald (hasufell) from comment #15) > > > Not validating input is _always_ dangerous, no matter if there already is a > > known security vulnerability or not. I'm not sure if you are serious about > > this statement. > > If it's always dangerous as you claim, then demonstrate it. If you are > right, then it should be easy to do. > You are relying on the current state of the code and the implicit state machine to not screw up instead of relying on explicit input validation. That's not only bad practice, but always dangerous indeed, since it can introduce critical bugs on any minor code change, which is probably one of the reasons why there are so few portage developers. But you are right, this is not the place to debate this. And I was really not sure if you are serious, but I think understand your opinion now.
(In reply to Julian Ospald (hasufell) from comment #16) no one said the main gentoo tree should permit "*" or "~*". we already have a policy that says that we don't want to do that today. but that is irrelevant to the PM supporting it at all (portage should). that's why imo the policy belongs in the repo. putting it behind a FEATURES does nothing for checking tree policy. we already have arch.list which tools can check, so we could easily extend that to also include the glob: * repoman would use that to determine whether the active tree permits */~*. we can make -* as always permitted.
(In reply to SpanKY from comment #19) I think I can agree with that.
