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
interfaces/builtin: add dbus interface #1613
Merged
Merged
Changes from 101 commits
Commits
Show all changes
111 commits
Select commit
Hold shift + click to select a range
a289901
interfaces: add preliminary dbus-bind (TODO: attributes)
94736b4
implement SanitizeSlot with tests
57fb199
perform substitutions in PermanentSlotAppArmor policy
8b7e9f3
Merge remote-tracking branch 'upstream/master' into dbus-bind
98c39de
update comments and documentation
413e19c
Merge remote-tracking branch 'upstream/master' into dbus-bind
a624250
update receive and send rules for talking with dbus-daemon
34dd46f
add connected plug and connected slot policy. improve tests
eb2d79b
Merge remote-tracking branch 'upstream/master' into dbus-bind
2345399
add interfaces.SecurityMount to PermanentSlotSnippet
969be86
BROKEN TEST BRANCH
b743fe7
revert last commit
af16d22
Merge remote-tracking branch 'upstream/master' into dbus-bind
2f73a16
start conversion to proposed yaml
24fd251
update SanitizeSlot and tests for getBusNames()
12721f0
remove dbusBindBusNames, iterate through names for policy (TODO: clea…
0a62fc2
abstract out shared and individual dbus name rules, fix testsuite
a978d37
update TestPermanentSlotAppArmorSystem() to test abstraction
22aeba5
abstract out AppArmor abstraction calculation
6a865ce
abstract out individual policy generation to getAppArmorIndividualSni…
4861623
update comments and shuffle code around
a6af67f
update docs, implement plugs, refactor tests, add tests
ab91e06
Merge remote-tracking branch 'upstream/master' into dbus-bind
c2da833
add Introspectable policy
7954f99
go fmt changes
ce961f0
fix a couple typos and a the mocked test yaml
11d768c
ensure that connections specify the same bus and name
4915bd9
non-existent plug should skip. non-existent slot should error
28577f9
don't autoconnect
26b8e2e
Merge remote-tracking branch 'upstream/master' into dbus-bind
fa105bd
fix copy and waste in auto connect test
1d0019b
Merge remote-tracking branch 'upstream/master' into dbus-bind
0e80dfc
add dbus-app interface (LP: #1590679)
dcbcce7
remove some plugs yaml in tests
bee2592
update docs/interfaces.md for dbus-bind/dbus-app and new implementation
4d72745
make getBusNames() private
eba49a1
improve documentation with some example yaml
52104cc
remove commented out test for plug connection; we don't support it yet
a8b34ce
add comment for AutoConnect
63fe8cb
remove unused function that carried over from dbus-bind exploratory PR
7392e2a
small code cleanup suggested by mvo
6ded192
Merge remote-tracking branch 'upstream/master' into dbus-app
70b21d2
Merge remote-tracking branch 'upstream/master' into dbus-app
e0618e5
change policy 'var's to 'const' as per the new guidelines for interfa…
da0cb81
Merge remote-tracking branch 'upstream/master' into dbus-app
843c02a
update for ErrUnknownSecurity, TestUnused and TestUnknown tests
64c83c4
Merge remote-tracking branch 'upstream/master' into dbus-app
d6f9443
use '%q' instead of '%s' in fmt.Errof()
8023baf
move DBus bus name checks into ValidateDBusBusName() in core.go
dfc1eed
clarify comment in getAppArmorIndividualSnippet()
c6d7b62
Merge remote-tracking branch 'upstream/master' into dbus-app
0548657
update for new AutoConnect, LegacyAutoConnect and remove test
5126c3d
add dbus-app to base declaration
45772dc
adjust whitespace in docs/interfaces.md
caf636d
Merge remote-tracking branch 'upstream/master' into dbus-app
b2cc5e0
Merge remote-tracking branch 'upstream/master' into dbus-app
a2b8070
dbus-app should use 'deny-auto-connection: true'
d24897f
Merge remote-tracking branch 'upstream/master' into dbus-app
7ce23c9
Merge remote-tracking branch 'upstream/master' into dbus-app
4554657
rename as 'dbus'
7622c70
update basedeclaration and tests
9e2fa01
update attributes for new design
3533d68
adjust error string in ValidateDBusBusName() for clarity
cc1360b
move the helper get* functions above where they are first used
712cc6c
rename test names to reflect what they are reflecting now
df86460
implement ConnectedSlot policy, various small cleanups
755d0df
implement ConnectedPlug policy
3b77d40
move some code so that it flows in the same way organizationally
2b35752
add missing TestUsedSecuritySystems tests
0203c11
assign to bus and name after verifying val
d745963
various minor cleanups
6fe33cd
one more cleanup
3930158
go fmt fixes
52f9ebe
Merge remote-tracking branch 'upstream/master' into dbus-app
e518e1e
only connect matching attributes
528cdc1
add a few unconfined rules for introspection and fine-tune connection…
33a8b29
update testsuite for last commit
b2c3366
use ###DBUS_INTROSPECT_PATH### for introspection rule
0224642
update a few comments for clarity
122e1e0
Merge remote-tracking branch 'upstream/master' into dbus-app
3a65f1c
go fmt for last commit
17c0b2a
clarify comment for ###DBUS_PATH### rules
be0ab2b
one more clarifying comment
cef1975
support KDE's use of 'well-known' names
74fdc0d
adjust tests to not end with '-[0-9]+'
7344b6d
don't allow bus names to end with -[0-9]+
66b7ce6
adjust introspection rules for KDE applications
68b4ca0
fine-tune the pid alternation with something equivalent but prettier
72a1529
leave ValidateDBusBusName() as DBus validator and move snapd check to…
06cc998
address review feedback from niemeyer for getAttribs()
269da61
remove old comment that doesn't apply any more
486e07c
don't hard-code bus=system with registration rules
f5ba68f
also add abstraction to ConnectedPlug policy
7569e32
fix testsuite for last commit
3bb7e6e
add DBus security backend for system bus policy
f7f6840
change default context for dbus bus policy to allow for non-root
cc668af
Merge remote-tracking branch 'upstream/master' into dbus-app
7c36dc7
allow plugging side to connect to anything on the slot side via well-…
d9550f2
Merge remote-tracking branch 'upstream/master' into dbus-app
b9bc03e
Merge remote-tracking branch 'upstream/master' into dbus-app
316b439
Merge remote-tracking branch 'upstream/master' into dbus-app
a372d97
address feedback from tyhicks (thanks!) regarding comments
0ea491d
fix testsuite for last commit
d9a1dce
remove redundant checks (thanks pedronis)
38b3feb
simplify pathBuf (thanks pedronis)
bb7cc54
add TODO for using interface/policy checkers (thanks pedronis)
b070992
remove 'send' with classic policy and clarify comments (thanks tyhicks)
3f5acbf
update parser error with peer=(name=... for KDE plug rule
ead951b
update the comment for attributes not matching to be more clear
758cda9
Merge remote-tracking branch 'upstream/master' into dbus-app
50e6cf1
dbusPermanentSlotAppArmor: allow GetConnectionCredentials (thanks jhe…
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the denials here? This will prevent people from using the interface altogether until explicitly allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I thought we agreed that people would need to have their well-known dbus name approved in the store. This will trigger a manual review and enforce the use of a snap declaration to claim the well-known name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, it doesn't prevent them from using the interface-- they can install a snap that slots this interface and get the bind rules needed to run; it is just that you can't connect other snaps to this interface until a snap declaration is present.
I think this is the right approach for this interface and others in general, but it sheds a light on a developer pain point as described in https://bugs.launchpad.net/snappy/+bug/1640874. I think this is perhaps solved by a locally signed snap declaration, but that needs your input (see my comments in the bug).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd indeed not auto-connect it, but is there a reason to disable the manual connection altogether? This would be the user manually establishing the allowance for dbus communication between two separate snaps, over a well defined API. I can't quite see why we'd reject this yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We reject it because the snap has not claimed the name via the store and doesn't yet have a snap declaration. On initial upload with using this interface, the reviewer will issue a snap declaration that allows the connection, and then all is well for connections and subsequent uploads. I think this is in line with everything we've discussed regarding the base declaration and snap declarations-- slot implementations (typically) need some sort of snap declaration since they (typically) allow privileged access to the system. In this case the privilege is to claim a well-known name.
On a related note, the review tools are now considering the base declaration and prompting for manual review due to this constraint. If we remove the constraint today, the tools would let this through without prompting for manual review. If we decide that the constraint should not be there, then the tools will have to be modified to prompt for manual review based on factors outside of the base declaration for an interface review (which is totally doable, but I thought one goal of the base declaration was to be the one place the review tools would consult for how interface reviews should be performed).