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

spread/google: add support for service accounts #187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,26 @@ Service accounts are best as they can be further constrained and not be
associated with your overall authenticated access. Do not ship your own
credentials to remote systems.

A service account can be attached to a created instances using the following
configuration:

```
(...)

backends:
google:
key: $(HOST:echo $GOOGLE_JSON_FILENAME)
...
systems:
- system-with-service-account:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this more realistic or more clarly a placeholder text.

attach-service-account: true
...
```

Service account can only be attached to an instance if the authentication key is
of `service_acccount` type, and the IAM role associated with the account has the
necessary permissions.

Images are located by first attempting to match the provided value exactly
against the image name, and then some processing is done to verify if an
image with the individual tokens in its description exists. Images are
Expand Down
65 changes: 58 additions & 7 deletions spread/google.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"golang.org/x/net/context"
"golang.org/x/oauth2"
"golang.org/x/oauth2/google"
"golang.org/x/oauth2/jwt"

"github.com/niemeyer/pretty"
"regexp"
Expand Down Expand Up @@ -44,6 +43,8 @@ type googleProvider struct {

client *http.Client

serviceAccount string

mu sync.Mutex

keyChecked bool
Expand Down Expand Up @@ -459,6 +460,20 @@ func (p *googleProvider) createMachine(ctx context.Context, system *System) (*go
},
}

if system.AttachServiceAccount {
if p.serviceAccount == "" {
return nil, &FatalError{fmt.Errorf("no service account to attach")}
}
// XXX the service account could be set from google key
// credentials, but the account used in the context of the
// request may not have the permissions to attach a service
// account to the instance
params["serviceAccounts"] = []googleParams{{
"email": p.serviceAccount,
"scopes": []string{"https://www.googleapis.com/auth/cloud-platform"},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the scopes also need to be parameterized (fewer/more/different scopes), or is that the only sensible value to be used when using Spread?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU it should match the same default scope we use when setting up the client https://github.com/snapcore/spread/blob/ded9133cdbceaf01f8a1c9decf6ff9ea56e194d6/spread/google.go#L776

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scope feels too powerful to give to an instance running, Do we know the minimum scopes we need for ubuntu pro?
if the scope should match the default scope for the key, would it make sense to avoid attaching the service account from the auth key to the machine and instead explicitly pass a less privileged (setup by IS) service account with minimum set of scopes needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exact same scope we use when creating GCP client in https://github.com/snapcore/spread/blob/master/spread/google.go#L776 it's also the scope of the token, but the actual permissions. What you can do is not driven by the token scope but by the IAM role assigned to your account. In fact, your application default account, or even the github account we use in default in snapd spred are unable to attach the instances because they lack the necessary permissions in their assigned roles roles.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the exact same scope we use when creating GCP client in https://github.com/snapcore/spread/blob/master/spread/google.go#L776

But the GCP client is spread, right? and the key owner is the one invoking spread (consciously) so it is expected and not a security issue here.

it's also the scope of the token, but the actual permissions.

Yes, but since the user can run spread that means their attached svc account can do CRUD operations on GCP instances.

My worry, is giving this power to an Ubuntu instance with lots of installed software (large attack surface), all of which would be able to do CRUD operations on the company's internal GCP account. The attack surface grows from just spread's code base, to Ubuntu + a bunch of installed software.

Again, I might be missing a puzzle piece here or some context that could make this concern a non issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot attach a service account unless the role assigned to your account is allowed to do so. The application default account all of use does not have that. The service account used by spread runners does not have that either. In fact you even won't be able to start an instance with the service account attached, all you get back is:

cannot allocate new Google server for ubuntu-pro-22.04-64: required
    'compute.instances.setServiceAccount' permission for 'projects/snapd-spread/zones/us-east1-b/instances/jun250926-154636'

You are not giving power to anyone, unless you generate a key for them in GCP admin console and give them the key they can use to pass to spread.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting my concerns from https://cloud.google.com/iam/docs/attach-service-accounts:

When the resource needs to access other Google Cloud services and resources, it uses its attached service account as its identity. For example, if you attach a service account to a Compute Engine instance, and the applications on the instance use a client library to call Google Cloud APIs, those applications automatically authenticate as the attached service account.

but since you tested it on launched machine, then I am confusing this with something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the whole point of attaching a service account to the machine. This way we as long as the account we use entitled to use Ubuntu Pro, the ubuntu-advantage-client will be able to automatically enable relevant support at boot time. There is no need to share or carry yet another token.

}}
}

if system.SecureBoot {
params["shieldedInstanceConfig"] = googleParams{
"enableSecureBoot": true,
Expand Down Expand Up @@ -755,6 +770,28 @@ func (p *googleProvider) waitOperation(ctx context.Context, s *googleServer, ver
panic("unreachable")
}

func serviceAccountFromKey(raw []byte) (string, error) {
const serviceAccountKey = "service_account"

// taken from golang.org/x/oauth/google
var credentials struct {
Type string `json:"type"`
ClientEmail string `json:"client_email"`
}

if err := json.Unmarshal(raw, &credentials); err != nil {
return "", err
}

if credentials.Type != serviceAccountKey {
// email is only relevant if dealing with service_account
// credentials type
return "", nil
}

return credentials.ClientEmail, nil
}

func (p *googleProvider) checkKey() error {
p.mu.Lock()
defer p.mu.Unlock()
Expand All @@ -771,15 +808,29 @@ func (p *googleProvider) checkKey() error {

if err == nil && p.client == nil {
ctx := context.Background()
if strings.HasPrefix(p.backend.Key, "{") {
var cfg *jwt.Config
cfg, err = google.JWTConfigFromJSON([]byte(p.backend.Key), googleScope)
var creds *google.Credentials
if p.backend.Key != "" {
var raw []byte
if strings.HasPrefix(p.backend.Key, "{") {
// already a raw JSON blob
raw = []byte(p.backend.Key)
} else {
raw, err = ioutil.ReadFile(p.backend.Key)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just os.ReadFile now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

spread is go 1.13 😞

}

if err == nil {
p.client = oauth2.NewClient(ctx, cfg.TokenSource(ctx))
creds, err = google.CredentialsFromJSON(ctx, raw, googleScope)
}

// identify service account if possible
p.serviceAccount, err = serviceAccountFromKey(raw)
} else {
os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", p.backend.Key)
p.client, err = google.DefaultClient(context.Background(), googleScope)
// none provided, let the google library find whatever
// is appropriate
creds, err = google.FindDefaultCredentials(ctx, googleScope)
}
if err == nil {
p.client = oauth2.NewClient(ctx, creds.TokenSource)
}
}
if err == nil {
Expand Down
3 changes: 3 additions & 0 deletions spread/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ type System struct {

Priority OptionalInt
Manual bool

// Only for Google
AttachServiceAccount bool `yaml:"attach-service-account"`
}

func (system *System) String() string { return system.Backend + ":" + system.Name }
Expand Down
Loading