distributed build behavior, java.io.* classes, and security

[Robert St. John]

I am working on a plugin, subclassing hudson.tasks.Builder.  The plugin uses a 3rd party library that will perform file system operations on workspace artifacts using java.io.File and java.io.InputStream.  In a distributed, master-slave configuration, does Jenkins guarantee that all build steps of a particular job-run perform on the same node where the workspace of the job-run is located, or could those java.io.* classes potentially throw exceptions because the workspace and artifacts are remote with respect to where my build step is running?

I am aware of using the FilePath API to perform actions on files located on different nodes.  I am thinking of avoiding this in the interest of security, however, because the the plugin is signing files using a private key, and I am concerned about sending the private key over the wire to another node where the target files are located.  I am using the Credentials plugin to load a keystore from a StandardCertificateCredential, which as I understand ultimately uses FilePath to load the keystore from the master node.  I am even thinking of making the build step send the target files to sign to the master node to be signed, then returned to the build node to prevent passing the private key over the network.  Are my concerns unfounded?  

Thank you.

Robert

[Jesse Glick]

On Tue, Dec 27, 2016 at 12:54 PM, Robert St. John <rest...@gmail.com> wrote:
> I am using the Credentials
> plugin to load a keystore from a StandardCertificateCredential, which as I
> understand ultimately uses FilePath to load the keystore from the master
> node.

Yes, existing Jenkins code does send secrets over the wire to agents,
if they are being used for a build run on that agent. In general this
is unavoidable; in many cases the secrets will in fact wind up on disk
on the agent for the duration of the build, or be accessible from
`/proc`.

If you wish to protect secrets from access by other builds, you have
no choice but to ensure either that each build runs on a “one-shot”
agent running in a container/VM (the Docker cloud model); or that each
build is somehow required to run all its build steps inside a fresh
container/VM even when the agent is reused (the Docker Pipeline model,
with restricted scripts).

And yes you must use `FilePath` for all file operations relating to
workspaces. You can use `act` to perform a chunk of work remotely in
the interests of efficiency.

[Robert St. John]

Thanks for your reply.  I discovered by trial and error today what you said about using FilePath.  I changed all my java.io usage to FilePath and used a MasterToSlaveFileCallable subclass to accomplish my goal.  What was not intuitive to me was that my Builder code is executed on master, while the process I invoked using the launcher ran on the slave.  I assumed that all the instructions of my Builder were executing on the slave.  I ran into a problem when code in my Builder needed to use a java.io.File object in the Builder to reference the output file of the process I launched with the launcher, and so the java.io.File did not exist as far as the Builder was concerned.  Am I interpreting what I'm seeing correctly?

Regarding what you said about secrets, I ended up passing a decrypted java.security.Key instance through a FileCallable to the slave.  Would that ever be written to disk?  I could alternatively look up the private key from within the FlleCallable, but I believe that would result in the Credentials plugin sending the key store password secret over the wire anyway.  What should I do to maximize the security and minimize assumptions about the way my Builder is run?

Thanks.

Robert

--
You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/xdyw9Jo4Fug/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr3gqj2OuSNdKeH5MNRrhACFpyLi-N5%2BLBHMeNdvj272rQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Stephen Connolly]

On 6 January 2017 at 03:18, Robert St. John <rest...@gmail.com> wrote:

Thanks for your reply.  I discovered by trial and error today what you said about using FilePath.  I changed all my java.io usage to FilePath and used a MasterToSlaveFileCallable subclass to accomplish my goal.  What was not intuitive to me was that my Builder code is executed on master, while the process I invoked using the launcher ran on the slave.  I assumed that all the instructions of my Builder were executing on the slave.  I ran into a problem when code in my Builder needed to use a java.io.File object in the Builder to reference the output file of the process I launched with the launcher, and so the java.io.File did not exist as far as the Builder was concerned.  Am I interpreting what I'm seeing correctly?

Regarding what you said about secrets, I ended up passing a decrypted java.security.Key instance through a FileCallable to the slave.  Would that ever be written to disk?  I could alternatively look up the private key from within the FlleCallable, but I believe that would result in the Credentials plugin sending the key store password secret over the wire anyway.  What should I do to maximize the security and minimize assumptions about the way my Builder is run?

BEWARE THE ANTIPATTERN

NEVER send a Credentials instance to an agent

The secret will be unencryptable when it lands on the agent as the agent does not have the master key

You need to extract the secrets on the master JVM and handle sending them over yourself.

This is why there are bugs in e.g. Subversion plugin and Git plugin (manifesting most obviously in the inability to use git with SSH key credentials that have a passphrase to checkout on a remote agent)

 

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAEoDabuM2X6JnJh42LJfrv%2B55RzqOE%2B2U-4EMEW32GtXPVciJw%40mail.gmail.com.

[Jesse Glick]

On Thu, Jan 5, 2017 at 10:18 PM, Robert St. John <rest...@gmail.com> wrote:
> my Builder code is executed on master

Yes, `perform` is run in the master JVM.

> while the
> process I invoked using the launcher ran on the slave

`Launcher` transparently starts processes remotely where required.

> I assumed that all
> the instructions of my Builder were executing on the slave.

No. BTW Daniel Beck is working on improved developer documentation
which ought to make the division clearer. See the docs mailing list.

> I ran into a
> problem when code in my Builder needed to use a java.io.File object in the
> Builder to reference the output file of the process I launched with the
> launcher, and so the java.io.File did not exist as far as the Builder was
> concerned.

Again, from code invoked via from `perform`, you may only use
`FilePath`, not `java.io.*`. (Unless you are inside a remote callable;
e.g., `FilePath.act`.)

[Jesse Glick]

On Fri, Jan 6, 2017 at 4:47 AM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> NEVER send a Credentials instance to an agent
>
> The secret will be unencryptable when it lands on the agent as the agent
> does not have the master key
>
> You need to extract the secrets on the master JVM and handle sending them
> over yourself.

Can you explain please? `Secret.value` is plaintext and is in the
serialized form, so when a `UsernamePasswordCredentialsImpl` (for
example) is passed over the Remoting channel its `password` should
arrive intact.

> This is why there are bugs in e.g. Subversion plugin and Git plugin
> (manifesting most obviously in the inability to use git with SSH key
> credentials that have a passphrase to checkout on a remote agent)

Is this not precisely what
`BasicSSHUserPrivateKey.CredentialsSnapshotTakerImpl` is for? Again
the `passphrase` is a `Secret` which ought to survive the trip to the
agent. And `writeReplace` ought to be ensuring that the `snapshot` is
taken automatically when the credentials are included in any
serialized request.

[Jesse Glick]

On Thu, Jan 5, 2017 at 10:18 PM, Robert St. John <rest...@gmail.com> wrote:
> I ended up passing a decrypted
> java.security.Key instance through a FileCallable to the slave. Would that
> ever be written to disk?

Not unless you write it.

> I could alternatively look up the private key from
> within the FlleCallable, but I believe that would result in the Credentials
> plugin sending the key store password secret over the wire anyway.

I suppose you are talking about two kinds of secrets here: a keystore
passphrase, which might be used to protect many keys, some more
important than others; and a particular private key needed by this
build to sign something. In that case it is appropriate to obtain the
particular private key on the master side and send only it over the
wire, so that a malicious build would not obtain access to anything
else in the keystore or its passphrase. (Make sure the global security
setting for “slave-to-master security” is enabled!)

[Robert St. John]

Thank you both for your input.  The plugin currently does as you say, Stephen, looking up the StandardCertificateCredentials instance on the master node.  The part that makes me wary is using the FileCallable to send the decrypted java.security.Key to the slave.  Does master-slave communication use TLS as it pertains to FileCallable?

The only other solution obvious to me is to send the file to the master to be signed.  While not desirable, that would not be too horrible because the files should be tolerably small - probably no bigger than 10 MB in most cases.

I'd like to release this plugin as a Jenkins-hosted plugin eventually, but I of course would not want to release something that undermines users' security, especially in enterprise and cloud environments.  I'd be grateful if you both took a brief look at the code to make sure it's on the up and up.  It's a small project, and everything relevant is in the SignApksBuilder class.  Are there any other security-related items I should document, such as your note, Jesse, to enable slave-to-master security?

By the way, Stephen, is there any reason that StandardCertificateCredentials could not be modified to allow a passphrase per private key, separate from the keystore's passphrase?

Thanks.

Robert

[Jesse Glick]

On Fri, Jan 6, 2017 at 1:45 PM, Robert St. John <rest...@gmail.com> wrote:
> Does master-slave
> communication use TLS

Depends entirely on the agent’s launcher. SSH is common, for example.
For purposes of your plugin you may assume that the connection itself
is secure.

Your current approach to extract the `PrivateKey` from the keystore on
the master makes sense to me. While you *could* transmit the entire
`CertificateCredentialsImpl`, this seems like a needless risk.

[Stephen Connolly]

So if the Secret is populated when on the master and has been snapshotted, then *over remoting* you *should* be ok sending it over, but you are sending it in plain text not encrypted.

If you are sending a non-snapshotted credential over remoting, that may be a credential linked to an external credentials store, so the secret value may not be present until you make the call to extract the value... which means that the secret population by https://github.com/jenkinsci/jenkins/blob/5483ee13833bb44ff611e8da3a8c74379ba26e11/core/src/main/java/hudson/util/Secret.java#L202 will blow up

Basically as a wrapper, Secret is rather poor IMHO - at least when used from an agent. Perhaps with some additional methods it could be made safer to use and we can roll back from my statement of *Do not use* to be *Do not use without snapshotting*

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-dev+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr2P5hKz9KmQQgnMdredMa_xMTYkz0Bjy%2B2AtqyJOVWBOQ%40mail.gmail.com.

[Jesse Glick]

On Sat, Jan 7, 2017 at 10:18 AM, Stephen Connolly
<stephen.al...@gmail.com> wrote:
> If you are sending a non-snapshotted credential over remoting

How is that even possible? `Credentials` implementations which have a
corresponding snapshotter override `writeReplace` to make sure
`snapshot` is called before being sent over the wire. Right?