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

Integrating system logging into docker API calls #14446

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
9 participants
@alecbenson

alecbenson commented Jul 7, 2015

This patch provides much needed system logging for docker's API functions. With this patch, when an API request is made, an entry will be added to the syslog.

Important events will contain the action requested, the container's ID, the username and login UID of the user issuing the request, the process ID, and any initialized configuration settings. In an effort to reduce the size of the log message, uninitialized configuration parameters are not logged.

Here is an example of a start log:

{Action=start, Username=abenson, LoginUID=1000, PID=3333, ContainerID=c367e0b0dbbac2d2f5b88a28b89526a857266f7914032277e97f7b635fa7b222, Config=Hostname=c367e0b0dbba, AttachStdin=true, AttachStdout=true, AttachStderr=true, Tty=true, OpenStdin=true, StdinOnce=true, Cmd={parts:[/bin/bash]}, Image=docker.io/centos, NetworkDisabled=false, Labels=map[],  HostConfig=LxcConf={values:[]}, OomKillDisable=false, Privileged=false, PortBindings=map[], PublishAllPorts=false, Devices=[], NetworkMode=default, RestartPolicy={Name:no MaximumRetryCount:0}, ReadonlyRootfs=false, LogConfig={Type: Config:map[]}, }

Less "interesting" events (kill, stop, pause, etc) create a smaller entry:

Jun 18 09:47:33 redhat-abenson Docker[16612]: {Action=kill, ID=84cf8f58643c243be40753b136006cd3a4b2579d601bc3281c2d289d8d2a6158, Username=abenson, LoginUID=1000, PID=32401, }

This patch makes it much easier to determine what any container on a system is doing, and also provides valuable insight about how users on a system are interacting with the daemon. This patch was developed with the help and support of @rhatdan at Red Hat.

Signed-off-by: Alec Benson albenson@redhat.com

@cpuguy83

This comment has been minimized.

Show comment
Hide comment
@cpuguy83

cpuguy83 Jul 7, 2015

Contributor

I'm inclined to say this doesn't feel like the right place, especially since docker itself doesn't have any concept of users, or authorizing access, etc.

Contributor

cpuguy83 commented Jul 7, 2015

I'm inclined to say this doesn't feel like the right place, especially since docker itself doesn't have any concept of users, or authorizing access, etc.

@alecbenson

This comment has been minimized.

Show comment
Hide comment
@alecbenson

alecbenson Jul 7, 2015

@cpuguy83 There is a fantastic authentication framework that is being worked on at the moment by Nalin Dahyabhai.

The pull request for that framework can be found here:
#13994

While I think that this logging code provides plenty of utility on its own, it was explicitly designed with to be compatible with Nalin's work -- so if Docker does plan on implementing user authorization anytime soon, this will be very seamless to integrate.

alecbenson commented Jul 7, 2015

@cpuguy83 There is a fantastic authentication framework that is being worked on at the moment by Nalin Dahyabhai.

The pull request for that framework can be found here:
#13994

While I think that this logging code provides plenty of utility on its own, it was explicitly designed with to be compatible with Nalin's work -- so if Docker does plan on implementing user authorization anytime soon, this will be very seamless to integrate.

@phemmer

This comment has been minimized.

Show comment
Hide comment
@phemmer

phemmer Jul 7, 2015

Contributor

This also will only work when using a unix domain socket. For clients going over TCP (boot2docker), or other operating systems, this can't be implemented (at least not as it is).
While I love all the nifty things you can do with unix domain sockets, I think the use here is too limited. (but I don't work for docker, so this is just a third party opinion)

Contributor

phemmer commented Jul 7, 2015

This also will only work when using a unix domain socket. For clients going over TCP (boot2docker), or other operating systems, this can't be implemented (at least not as it is).
While I love all the nifty things you can do with unix domain sockets, I think the use here is too limited. (but I don't work for docker, so this is just a third party opinion)

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 7, 2015

Contributor

We will extend it to work over the network as we move forward. Docker without proper logging of who is doing what, is going to be a tough sell into sites that care about security. Docker logging of administrative access is totally useless at this point. This gives us a framework for recording this information.

Contributor

rhatdan commented Jul 7, 2015

We will extend it to work over the network as we move forward. Docker without proper logging of who is doing what, is going to be a tough sell into sites that care about security. Docker logging of administrative access is totally useless at this point. This gives us a framework for recording this information.

System logging of docker API calls
Signed-off-by: alec <abenson@wpi.edu>

@GordonTheTurtle GordonTheTurtle removed the dco/no label Jul 7, 2015

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jul 7, 2015

Contributor

👎 on this as it is. I already expressed my concerns about the previous PR and this new one does nothing to address those issues:

  1. Supporting logging only when the daemon is listening on unix sockets is far from ideal.
  2. The fact that it ignores any logging configuration for the daemon and writes to syslog, whether you like it or not is not the way to go either.

I'd rather have a well defined authentication/authorization framework and build audit logging in top of it than having a half backed implementation like this one.

Contributor

calavera commented Jul 7, 2015

👎 on this as it is. I already expressed my concerns about the previous PR and this new one does nothing to address those issues:

  1. Supporting logging only when the daemon is listening on unix sockets is far from ideal.
  2. The fact that it ignores any logging configuration for the daemon and writes to syslog, whether you like it or not is not the way to go either.

I'd rather have a well defined authentication/authorization framework and build audit logging in top of it than having a half backed implementation like this one.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 8, 2015

Contributor

I would prefer not to wait for authorization, but having authentication is fine. Alec has the a more advanced patch based on @nalind framework for authentication, which should handle the Local sockets as well as network connections.

Contributor

rhatdan commented Jul 8, 2015

I would prefer not to wait for authorization, but having authentication is fine. Alec has the a more advanced patch based on @nalind framework for authentication, which should handle the Local sockets as well as network connections.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 8, 2015

Contributor

I also do not see this as the same as the logging setup for logging the stdout/stderror of a container. These have to be treated differently. Perhaps it needs to be documented. But administrative actions on the docker daemon are potentially more security sensitive then what a contained process does.

Contributor

rhatdan commented Jul 8, 2015

I also do not see this as the same as the logging setup for logging the stdout/stderror of a container. These have to be treated differently. Perhaps it needs to be documented. But administrative actions on the docker daemon are potentially more security sensitive then what a contained process does.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 8, 2015

Contributor

Current discussion on authentication is happening at
#13697

Contributor

rhatdan commented Jul 8, 2015

Current discussion on authentication is happening at
#13697

@alecbenson

This comment has been minimized.

Show comment
Hide comment
@alecbenson

alecbenson Jul 10, 2015

@calavera I agree with @rhatdan that we need to make a distinction between actions that are happening inside of a docker container and administrative actions that are occurring on the host machine. Lumping these two together is a violation of separation of concerns and certainly a step in the wrong direction with respect to security.

Docker cannot continue to put security on the backburner if it wants to continue to be used in enterprise deployments. At the very least, this patch gives customers valuable information about what the docker daemon is doing and can be (and has been) easily extended to integrate with a working authentication framework like Nalin's.

alecbenson commented Jul 10, 2015

@calavera I agree with @rhatdan that we need to make a distinction between actions that are happening inside of a docker container and administrative actions that are occurring on the host machine. Lumping these two together is a violation of separation of concerns and certainly a step in the wrong direction with respect to security.

Docker cannot continue to put security on the backburner if it wants to continue to be used in enterprise deployments. At the very least, this patch gives customers valuable information about what the docker daemon is doing and can be (and has been) easily extended to integrate with a working authentication framework like Nalin's.

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jul 13, 2015

Contributor

@alecbenson this discussion is about changes you are suggesting we make in the code base. Speculation about what docker cares and not cares aren't appropriate. If you have comments about the direction of the project, you'll find our ROADMAP in https://github.com/docker/docker/blob/master/ROADMAP.md.

Contributor

calavera commented Jul 13, 2015

@alecbenson this discussion is about changes you are suggesting we make in the code base. Speculation about what docker cares and not cares aren't appropriate. If you have comments about the direction of the project, you'll find our ROADMAP in https://github.com/docker/docker/blob/master/ROADMAP.md.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 13, 2015

Contributor

@alecbenson @calavera The comments were a little strident. I agree. But getting proper logging into a daemon that is as powerful as the docker daemon is the goal here.

Contributor

rhatdan commented Jul 13, 2015

@alecbenson @calavera The comments were a little strident. I agree. But getting proper logging into a daemon that is as powerful as the docker daemon is the goal here.

@nalind

This comment has been minimized.

Show comment
Hide comment
@nalind

nalind Jul 13, 2015

Contributor

@calavera Would it be preferable to route this through the existing logging interface, and force the logging drivers to make things differentiable to admins who read those logs directly?

The current syslog driver logs messages at either Info or Err log levels, depending on the output stream, and in the most recent proposal, we were starting to use syslog with the Alert level. If we switch to pushing messages things through the logger Log interface, marking these messages by using a different Source value in the Message structure that we pass to it (stdout and stderr are the values which are used now, we'd be adding another one), we could conceivably retrofit logic for handling a different Source value into each of the drivers. For example, the syslog driver would log at Alert level, the journald driver would send messages using the PriAlert level, that sort of thing. The logs implementation already filters out messages that aren't from stdout or stderr when it reads things back, so it'd be no less usable than what we have now.

Contributor

nalind commented Jul 13, 2015

@calavera Would it be preferable to route this through the existing logging interface, and force the logging drivers to make things differentiable to admins who read those logs directly?

The current syslog driver logs messages at either Info or Err log levels, depending on the output stream, and in the most recent proposal, we were starting to use syslog with the Alert level. If we switch to pushing messages things through the logger Log interface, marking these messages by using a different Source value in the Message structure that we pass to it (stdout and stderr are the values which are used now, we'd be adding another one), we could conceivably retrofit logic for handling a different Source value into each of the drivers. For example, the syslog driver would log at Alert level, the journald driver would send messages using the PriAlert level, that sort of thing. The logs implementation already filters out messages that aren't from stdout or stderr when it reads things back, so it'd be no less usable than what we have now.

@raychaser

This comment has been minimized.

Show comment
Hide comment
@raychaser

raychaser Jul 13, 2015

Audit logging is great, and the authentication stuff in progress is very exciting, especially if RBAC can be layered on top eventually. This stuff is obviously super important for enterprise users (see for example the amount of effort that went into AWS CloudTrail...)

From the perspective of someone who is working for a living on centralizing logs from many hosts into a single place, it would be much easier if existing Docker ways of dealing with logs could be used here so that existing ways of collecting logs from Docker could be reused!

Specifically, it would be great if it would be possible to get to these logs remotely as well (via the API). My first thought when I saw this PR was - why not add this to the /events API? It is already scoped to be on a "per-Daemon basis".

If that doesn't work, adding to the existing logging framework is still preferable over inventing a new thing IMHO. The logging interface assumes logs are per-container. So maybe adding a notion of daemon logs alongside container logs can help?

In any case, the ability to get monitoring information out of the API (logs, stats, events) is tremendous. It works locally and remotely, and it works even through Swarm, for example. Also, it is not OS specific, so it works on Windows as well. It would be great if a way could be found to allow the audit logging to be available this way as well.

raychaser commented Jul 13, 2015

Audit logging is great, and the authentication stuff in progress is very exciting, especially if RBAC can be layered on top eventually. This stuff is obviously super important for enterprise users (see for example the amount of effort that went into AWS CloudTrail...)

From the perspective of someone who is working for a living on centralizing logs from many hosts into a single place, it would be much easier if existing Docker ways of dealing with logs could be used here so that existing ways of collecting logs from Docker could be reused!

Specifically, it would be great if it would be possible to get to these logs remotely as well (via the API). My first thought when I saw this PR was - why not add this to the /events API? It is already scoped to be on a "per-Daemon basis".

If that doesn't work, adding to the existing logging framework is still preferable over inventing a new thing IMHO. The logging interface assumes logs are per-container. So maybe adding a notion of daemon logs alongside container logs can help?

In any case, the ability to get monitoring information out of the API (logs, stats, events) is tremendous. It works locally and remotely, and it works even through Swarm, for example. Also, it is not OS specific, so it works on Windows as well. It would be great if a way could be found to allow the audit logging to be available this way as well.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 14, 2015

Contributor

Well audit logging has specific rules and requires calls to specific APIs. Also the audit information is currently being caught by this patch and being sent to syslog.

Contributor

rhatdan commented Jul 14, 2015

Well audit logging has specific rules and requires calls to specific APIs. Also the audit information is currently being caught by this patch and being sent to syslog.

@nalind

This comment has been minimized.

Show comment
Hide comment
@nalind

nalind Jul 15, 2015

Contributor

@alecbenson pointed out that a lot of the logdriver logic is based around the assumption that it's logging about a specific container, so it's not a perfect fit for logging requests that aren't, such as the ones that backend the 'ps' command, and I agree. We need to keep looking at other options.

Contributor

nalind commented Jul 15, 2015

@alecbenson pointed out that a lot of the logdriver logic is based around the assumption that it's logging about a specific container, so it's not a perfect fit for logging requests that aren't, such as the ones that backend the 'ps' command, and I agree. We need to keep looking at other options.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 20, 2015

Contributor

Where do we stand on this. From our point of view and the point of view of enterprise customers, being able to log activity of the administrator is critical. Users of docker are people on the other end of the socket, for now we can only record via Unix Domain Socket, network connections can not be trusted, and we tell customers not to use them.

I take it this will not make docker-1.8.

@crosbymichael @jfrazelle @tianon @cpuguy83

Contributor

rhatdan commented Jul 20, 2015

Where do we stand on this. From our point of view and the point of view of enterprise customers, being able to log activity of the administrator is critical. Users of docker are people on the other end of the socket, for now we can only record via Unix Domain Socket, network connections can not be trusted, and we tell customers not to use them.

I take it this will not make docker-1.8.

@crosbymichael @jfrazelle @tianon @cpuguy83

@calavera

This comment has been minimized.

Show comment
Hide comment
@calavera

calavera Jul 20, 2015

Contributor

@rhatdan network connections are perfectly safe and can be trusted using TLS.

Getting the user by introspecting the file descriptor associated to the response writer is a hack that we don't want in our code. We DO want, on the other hand, a framework to provide common knowledge of user/credentials between client and server, regardless the transport.

I'm sure we can modify the current logging drivers enough to support auditing logs. Hardcoding syslog might be good for you, but it doesn't mean it's good for everyone else.

Contributor

calavera commented Jul 20, 2015

@rhatdan network connections are perfectly safe and can be trusted using TLS.

Getting the user by introspecting the file descriptor associated to the response writer is a hack that we don't want in our code. We DO want, on the other hand, a framework to provide common knowledge of user/credentials between client and server, regardless the transport.

I'm sure we can modify the current logging drivers enough to support auditing logs. Hardcoding syslog might be good for you, but it doesn't mean it's good for everyone else.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 20, 2015

Contributor

TLS Means that every process on one system that can access the credentials now has full root on another system, if the user setup TLS Correctly.

I doubt that docker will ever not trust the local root connection to the docker.sock. Integration with pam_rootok for example would allows this. Examining who is on the other end of the connection is fundamental to client server authentication.

I did not know that syslog was such a strange thing to add to a daemon. We are not interested in auditing until we get proper administrator logging.

Contributor

rhatdan commented Jul 20, 2015

TLS Means that every process on one system that can access the credentials now has full root on another system, if the user setup TLS Correctly.

I doubt that docker will ever not trust the local root connection to the docker.sock. Integration with pam_rootok for example would allows this. Examining who is on the other end of the connection is fundamental to client server authentication.

I did not know that syslog was such a strange thing to add to a daemon. We are not interested in auditing until we get proper administrator logging.

@rhatdan

This comment has been minimized.

Show comment
Hide comment
@rhatdan

rhatdan Jul 27, 2015

Contributor

Is there a way forward on this pull request?

Contributor

rhatdan commented Jul 27, 2015

Is there a way forward on this pull request?

@icecrime

This comment has been minimized.

Show comment
Hide comment
@icecrime

icecrime Aug 8, 2015

Contributor

Sorry, but multiple maintainers have expressed a negative opinion there already. Furthermore, the authentication framework it is based on (#13994) is still under design (discussions are happening on #13697). We're not against the idea, it just seems slightly premature.

Contributor

icecrime commented Aug 8, 2015

Sorry, but multiple maintainers have expressed a negative opinion there already. Furthermore, the authentication framework it is based on (#13994) is still under design (discussions are happening on #13697). We're not against the idea, it just seems slightly premature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment