[MIR] pay-service

Bug #1614202 reported by dobey
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pay-service (Ubuntu)
Fix Released
High
Unassigned

Bug Description

Availability: Already in universe.
Rationale: The package is seeded in the Ubuntu Touch image.
Security:
Quality Assurance: Unit tests running during package build.
Background Information: This source package provides binary packages which are installed on Ubuntu Touch by default, and which are necessary for use of certain system features:

  - libpay2
  - pay-service
  - pay-ui

Revision history for this message
Michael Terry (mterry) wrote :

- Needs a team bug subscriber. Maybe phablet-team or unity-api-team?

- For a package built with Go (as pay-service is), it needs to use dh-golang and "Built-Using: ${misc:Built-Using}" in its control file. The 3rd party packages are all bundled in too, which is bad. You should use golang-github-gosexy-gettext-dev instead of bundling github.com/gosexy/gettext for example.

- Not a blocker, but bad practice: "libpay2: package-name-doesnt-match-sonames libpay2.0.0" You probably want the library to include a symlink for just "libpay.so.2" pointing at "libpay.so.2.0.0" (compare contents of libpay2 with libparted2).

Changed in pay-service (Ubuntu):
status: New → Incomplete
Revision history for this message
dobey (dobey) wrote : Re: [Bug 1614202] Re: [MIR] pay-service

On Wed, 2016-08-17 at 19:21 +0000, Michael Terry wrote:
> - Needs a team bug subscriber.  Maybe phablet-team or unity-api-team?

Right. Subscribed.

> - For a package built with Go (as pay-service is), it needs to use
> dh-
> golang and "Built-Using: ${misc:Built-Using}" in its control
> file.  The
> 3rd party packages are all bundled in too, which is bad.  You should
> use
> golang-github-gosexy-gettext-dev instead of bundling
> github.com/gosexy/gettext for example.

The package isn't only golang. Isn't Built-Using added automatically? I
agree the bundling is not ideal, but none of these things were packaged
when we rewrote the service in golang, and it wasn't really clear how
such things should be packaged, as golang doesn't have dynamic libs, so
the only option at the time was to vendorize things in the source tree.
I'm happy to look at using packaged deps instead of bundled source, but
I think this probably shouldn't block the MIR. Can you file a bug about
the bundled source?

I don't think it's possible to switch to dh-golang (would require
significant changes to how the package as a whole is built, and make it
no longer developer-friendly), but I think I maybe misunderstood how
Built-Using works when I saw that e-mail for build deps in universe
mentioning it. I thought it was automatic. If it's not, please file a
bug about Built-Using missing and I'll add it to the control.

> - Not a blocker, but bad practice: "libpay2: package-name-doesnt-
> match-
> sonames libpay2.0.0"  You probably want the library to include a
> symlink
> for just "libpay.so.2" pointing at "libpay.so.2.0.0"  (compare
> contents
> of libpay2 with libparted2).

Hmm, indeed this looks like probably a bug in the packaging that's just
been there forever. Can you file a bug for this? Should be a trivial
fix at least.

dobey (dobey)
Changed in pay-service (Ubuntu):
status: Incomplete → New
Revision history for this message
Michael Terry (mterry) wrote :

Built-Using is not automatic (${misc:Built-Using} is filled automatically, but you have to put the line in there).

Built-Using, dh-golang, and unbundling us much as possible is a hard request by the security team for any Go code entering main.

If you think it's too difficult to modify this package, you can seek an exemption from the security team. But I'd rather see this package follow our Go packaging guidelines.

Bugs filed: bug 1614264 and bug 1614267.

Revision history for this message
dobey (dobey) wrote :

OK, I've asked security about this, and the answer is that dh-golang is not a hard requirement from the security team. The fact is that pay-service is not installable via "go get" and there is no good reason to make it so (golang's build tools lack a significant amount of system integration support, or support for building libraries and tools which are not written in go). This seems like the same issue of whether one should be required to use setup.py to install all python code or not, and there is no such requirement for that. Enforcing a requirement as such for binaries written with go does not seem good here.

As for the bundled source code, there is also no hard requirement that code may not be bundled, and there is precedent for having bundled code in packages (many GNOME packages have historically bundled code for certain in-development widgets and APIs for example).

So I don't think either of those two things should be a blocker for going into main here. Perhaps we can move away from bundled source in the future for many things, but I see no reason to force a project into a state where it is harder to maintain, in order to have the portion of code which is written in golang, to be installable with go get.

Revision history for this message
Michael Terry (mterry) wrote :

Sure, like I said elsewhere, if the security team is fine with it, I'm fine.

Certainly while some of the bundled code isn't packaged in the archive, other bundled code is (like gettext). That seems like low-hanging fruit. But not a blocker for main.

Oh and another Go-in-main requirement I forgot before is "the requesting team must state their commitment to testing no-change-rebuilds triggered by a dependent library/compiler and to fix any issues found for the lifetime of the release."

Can I have you on the record as committing to be on the hook for testing Go-triggered rebuilds?

Revision history for this message
dobey (dobey) wrote :

On Fri, 2016-08-19 at 15:34 +0000, Michael Terry wrote:
> Oh and another Go-in-main requirement I forgot before is "the
> requesting
> team must state their commitment to testing no-change-rebuilds
> triggered
> by a dependent library/compiler and to fix any issues found for the
> lifetime of the release."
>
> Can I have you on the record as committing to be on the hook for
> testing
> Go-triggered rebuilds?

I had presumed this was a general requirement for any package, and not
limited to code in Go. But sure.

Revision history for this message
Michael Terry (mterry) wrote :

> I had presumed this was a general requirement for any package, and not
> limited to code in Go. But sure.

I can't tell if you're being serious or just taking an opportunity to be snarky, but I will answer as if you actually don't understand why Go is unique.

Normally, if the security team updates a compiled library, it's fairly easy to test. They install the updated library and run some of the apps that link against it. A relatively small QA surface, though certainly not trivial. And usually the maintainers of the apps that link against it aren't involved at all.

But Go is statically linked. Let's say the security team updates a Go library package. Now every app which uses that library needs to be rebuilt [1]. There's obviously a lot more that could go wrong when you have to rebuild (including things that are unrelated to the original fix). And if apps have their own bundled version of the code instead of using the packaged library, the security team has to go and patch the app especially with the security fix. Which is a whole new layer of complexity for them. [2]

So as part of the giant discussion about whether to allow Go code in main at all, the security team had a set of requirements, to reduce the burden on them: Built-Using, dh-golang, and unbundling. And getting the maintainer on the hook for help testing rebuilds and if anything goes wrong.

And while yes, usually maintainers are friendly and helpful, everyone has different time constraints and priorities, and the security team wants to make sure the maintainers plan accordingly, understanding that more time will have to be carved out to help test updates than a normal C package. It's easier to get agreement up front than chase someone in the middle of a heated security update.

But thanks for agreeing to help with updates. And now the only missing piece is Built-Using landing. And can you get the security team to comment on this bug mentioning your exemption from unbundling libraries that might be shared with other Go apps, so that there's a paper trail on that? Thanks.

[1] We know which apps need a rebuild from their Built-Using line. Hence that requirement.
[2] pay-service bundles some code that is unique to it, which is fine -- it only *needs* to be unbundled if other code uses it too. But it also bundles code like gettext that is not unique. Those libraries can and should be unbundled. But I'm not blocking on it per your exemption from the security team.

Revision history for this message
Michael Terry (mterry) wrote :

(In the medium term, Go will be getting shared library support, which will make this whole thing much easier.)

Revision history for this message
Michael Terry (mterry) wrote :

Reviewing the comments here, looks like the only things left are (1) to land the fix for bug 1614267 (Built-Using & dh-golang) and (2) a security team member saying they're fine with the Go code bundling that this package does.

As far as I can see, this package uses two github modules that are already packaged:

github.com/godbus (golang-dbus-dev)
github.com/gosexy (golang-github-gosexy-gettext-dev)

And then some launchpad modules that don't seem to be separately packaged yet.

launchpad.net/go-apparmor
launchpad.net/go-mir
launchpad.net/go-trust-store
launchpad.net/go-ual

As long as security is aware of these, so they know to update pay-service when these modules need to be patched, I'm fine with it. Otherwise, the normal work flow would be to upload those four modules as packages to yakkety and Build-Depend on them.

Revision history for this message
Emily Ratliff (emilyr) wrote :

Security Team ack for 16.10. As agreed via email, the security team will do the extra tracking required by the bundled packages and the dev team will assist with any CVEs that might arise. The package will be re-evaluated before 17.04.

Revision history for this message
Michael Terry (mterry) wrote :

Excellent, thanks Emily!

Changed in pay-service (Ubuntu):
status: New → Fix Committed
Revision history for this message
Steve Langasek (vorlon) wrote :

Override component to main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety: universe/gnome -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety amd64: universe/libs/optional/100% -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety arm64: universe/libs/optional/100% -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety armhf: universe/libs/optional/100% -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety i386: universe/libs/optional/100% -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety powerpc: universe/libs/optional/100% -> main
libpay2 15.10+16.10.20160825-0ubuntu1 in yakkety ppc64el: universe/libs/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety amd64: universe/libdevel/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety arm64: universe/libdevel/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety armhf: universe/libdevel/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety i386: universe/libdevel/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety powerpc: universe/libdevel/optional/100% -> main
libpay2-dev 15.10+16.10.20160825-0ubuntu1 in yakkety ppc64el: universe/libdevel/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety amd64: universe/gnome/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety arm64: universe/gnome/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety armhf: universe/gnome/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety i386: universe/gnome/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety powerpc: universe/gnome/optional/100% -> main
pay-service 15.10+16.10.20160825-0ubuntu1 in yakkety ppc64el: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety amd64: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety arm64: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety armhf: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety i386: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety powerpc: universe/gnome/optional/100% -> main
pay-test-app 15.10+16.10.20160825-0ubuntu1 in yakkety ppc64el: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety amd64: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety arm64: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety armhf: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety i386: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety powerpc: universe/gnome/optional/100% -> main
pay-ui 15.10+16.10.20160825-0ubuntu1 in yakkety ppc64el: universe/gnome/optional/100% -> main
31 publications overridden.

Changed in pay-service (Ubuntu):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.