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

update to new mp spdz version #15

Merged
merged 17 commits into from
May 6, 2022

Conversation

grafjo
Copy link
Contributor

@grafjo grafjo commented Mar 15, 2022

requires #14

Updates to MP-SPDZ v0.2.8

corresponding cli part carbynestack/cli#19

@strieflin
Copy link
Member

Needs to be rebased.

@Lila84 Lila84 force-pushed the feature/upstream-new-mp-spdz branch 3 times, most recently from 84b5220 to 5a0e51b Compare March 21, 2022 14:54
@kindlich kindlich force-pushed the feature/upstream-new-mp-spdz branch 3 times, most recently from 2b7e095 to 7551467 Compare April 6, 2022 15:46
pkg/ephemeral/player.go Outdated Show resolved Hide resolved
pkg/ephemeral/io/carrier.go Outdated Show resolved Hide resolved
pkg/ephemeral/spdz.go Outdated Show resolved Hide resolved
@sbckr
Copy link
Member

sbckr commented Apr 7, 2022

@grafjo Thank you for submitting this pull request 👍

I have added a few comments and questions. Please have a look at the Codacy findings too.

@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #15 (f29277c) into master (7c3f0e0) will decrease coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #15      +/-   ##
==========================================
- Coverage   80.12%   79.75%   -0.37%     
==========================================
  Files          29       29              
  Lines        1987     2010      +23     
==========================================
+ Hits         1592     1603      +11     
- Misses        335      342       +7     
- Partials       60       65       +5     
Impacted Files Coverage Δ
...carbynestack/ephemeral/pkg/ephemeral/io/carrier.go 76.47% <0.00%> (-9.74%) ⬇️
...b.com/carbynestack/ephemeral/cmd/discovery/main.go 58.33% <0.00%> (-3.13%) ⬇️
...b.com/carbynestack/ephemeral/pkg/ephemeral/spdz.go 89.74% <0.00%> (-0.78%) ⬇️
.../github.com/carbynestack/ephemeral/pkg/utils/os.go 87.09% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c3f0e0...f29277c. Read the comment docs.

@sbckr
Copy link
Member

sbckr commented Apr 7, 2022

Before this PR can be merged, we would at least have to provide a carbynestack/MP-SPDZ base image with the desired version v0.2.8, not? Are there any other modules being affected?

And if I remember correctly, reading inputs in MP-SPDZ has changed since version v0.1.3. With this it would also be great to provide insights on how upgrading MP-SPDZ affects the program code as used for the getting started guide or in-/output in general. Maybe even prepare a PR for this.

@sbckr sbckr added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 7, 2022
pkg/utils/os.go Outdated
err = command.Wait()
defer waitGroup.Done()
if err != nil {
println(fmt.Sprintf("Error occured!"))
Copy link
Member

Choose a reason for hiding this comment

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

We typically use a logger for emitting diagnosis information. What is the rationale of using println here instead? Is logging required here at all, as we anyway forward sysout/syserr to the caller?

Copy link
Contributor

Choose a reason for hiding this comment

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

In this method we don't have a logger available.
The reason why we print here is that if we ran into a timeout for the request, sometimes the process output was not printed.
That is also the reason for using the wait-group here.

Copy link
Member

Choose a reason for hiding this comment

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

You should be able to get a logger from the context using:

logger := log.FromContext(ctx)

Copy link
Contributor

Choose a reason for hiding this comment

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

No longer relevant, removed the printlns.

pkg/utils/os.go Outdated Show resolved Hide resolved
pkg/ephemeral/network/tls_connector.go Outdated Show resolved Hide resolved
pkg/ephemeral/network/tls_connector_test.go Outdated Show resolved Hide resolved
pkg/ephemeral/network/tls_connector.go Outdated Show resolved Hide resolved
pkg/ephemeral/network/tls_connector_test.go Outdated Show resolved Hide resolved
pkg/ephemeral/io/carrier.go Show resolved Hide resolved
pkg/ephemeral/io/carrier.go Outdated Show resolved Hide resolved
pkg/ephemeral/io/carrier.go Show resolved Hide resolved
pkg/ephemeral/io/carrier_test.go Outdated Show resolved Hide resolved
@strieflin
Copy link
Member

Seems on commit is not signed-off properly. Needs to be fixed before we can merge.

@grafjo
Copy link
Contributor Author

grafjo commented Apr 11, 2022

@grafjo Thank you for submitting this pull request +1

I have added a few comments and questions. Please have a look at the Codacy findings too.

@sbckr thank @kindlich - the main work was done by him :-)

@strieflin
Copy link
Member

Before this PR can be merged, we would at least have to provide a carbynestack/MP-SPDZ base image with the desired version v0.2.8, not? Are there any other modules being affected?

And if I remember correctly, reading inputs in MP-SPDZ has changed since version v0.1.3. With this it would also be great to provide insights on how upgrading MP-SPDZ affects the program code as used for the getting started guide or in-/output in general. Maybe even prepare a PR for this.

@sbckr The compliant base image is made available via carbynestack/base-images and not via carbynestack/MP-SPDZ. The latter was used to provide an upstream patch for supporting PRNG seeds. Anyway, the respective Dockerfile has to be updated to use the new MP-SPDZ version. It can be set easily using a Docker build argument. Created cabynestack/base-images#2 to track this. In addition, the base image used by ko must be updated accordingly:

github.com/carbynestack/ephemeral/cmd/ephemeral: ghcr.io/carbynestack/spdz:20210827

@strieflin
Copy link
Member

strieflin commented Apr 25, 2022

Couple of open change requests (see conversation history) here:

@kindlich
Copy link
Contributor

kindlich commented May 4, 2022

Regarding the Task Ensure tutorial code is still working:
It's more or less working, here's my notes from trying a local kind-deployment.

Info:

  • CarbyneStack checked out at Revision 99b9c56 (master currently)
  • Ephemeral checked out at Revision 8dde5de (hri-eu/feature/upstream-new-mp-spdz)
    -> Changed base-image to be ghcr.io/carbynestack/spdz:v0.2.8

Differences from Stack-Deployment:

  • I used EXTERNAL_IP_APOLLO and EXTERNAL_IP_STARBUCK instead of one EXTERNAL_IP env variable
    -> Since I set up both stacks at once in the same shell, this is a bit easier ^^
    -> Not related to these changes, but a difference nonetheless 😉

  • Docker-Network IP address range may differ (may be 172.19.0.0/16 or 172.20.0.0/16, ... instead of 172.18.0.0/16)

    • Use docker network inspect kind to be sure
  • Ephemeral Images were ko.local/ephemeral:locally-built (same for discovery and network-controller)

    • Ephemeral images built from Revision 8dde5de (hri-eu/feature/upstream-new-mp-spdz)
    • In the Stack Deployment, additional env vars used:
    • export DISCOVERY_IMAGE_REGISTRY=ko.local
    • export EPHEMERAL_IMAGE_REGISTRY=ko.local
    • export NETWORK_CONTROLLER_IMAGE_REGISTRY=ko.local
    • export DISCOVERY_IMAGE_REPOSITORY=discovery
    • export EPHEMERAL_IMAGE_REPOSITORY=ephemeral
    • export NETWORK_CONTROLLER_IMAGE_REPOSITORY=network-controller
    • export DISCOVERY_IMAGE_TAG=locally-built
    • export EPHEMERAL_IMAGE_TAG=locally-built
    • export NETWORK_CONTROLLER_IMAGE_TAG=locally-built
  • Used old Crypto-Material and CLI

  • Manually edited configmap discovery-config to add "playerCount": 2

    • Otherwise, discovery-service wouldn't even start (we added a check in the config validation)
    • Old Version of Ephemeral Helm chart used in helmfile.d?
  • Manually edited configmap cs-ephemeral-config1 to add "playerCount": 2

    • Otherwise, MP-SPDZ will error (since it would try to compute with 0 (default) players)
    • Should we add a validation in the startup so that instead the pod won't start in these cases (not part of this PR)?
    • Old Version of Ephemeral Helm chart used in helmfile.d?
  • Changed Billionaires-Problem Code to use regint(10000) instead of just 10000 In the header

    • Otherwise Code can't compile
     # Prologue to read in the inputs
     port=regint(10000)
     listen(port)
     socket_id = regint()
     acceptclientconnection(socket_id, port)
     v = sint.read_from_socket(socket_id, 2)
  • Manually exec'ed into the ephemeral-ephemeral pods to create the TLS certificates...

    • Seems we need them for Client<->MP-SPDZ connection
    • Generating them on each pod manually and not synching seems to work,
      so they are definitely only used for Ephemeral<->MP-SPDZ not between MP-SPDZ instances
    # Client Files
    kubectl --context=kind-apollo exec ephemeral-generic-00001-deployment-.... Scripts/setup-clients.sh 2
    kubectl --context=kind-starbuck exec ephemeral-generic-00001-deployment-.... Scripts/setup-clients.sh 2
    
    # Server Files
    kubectl --context=kind-apollo exec ephemeral-generic-00001-deployment-.... Scripts/setup-ssl.sh 2
    kubectl --context=kind-starbuck exec ephemeral-generic-00001-deployment-.... Scripts/setup-ssl.sh 2

@strieflin
Copy link
Member

This PR needs another rebase to master.

Johannes Graf and others added 17 commits May 5, 2022 15:23
Co-authored-by: Petra Scherer <[email protected]>
Co-authored-by: Timo Klenk <[email protected]>
Signed-off-by: Johannes Graf <[email protected]>
Signed-off-by: Petra Scherer <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Co-authored-by: Petra Scherer <[email protected]>
Co-authored-by: Timo Klenk <[email protected]>
Signed-off-by: Johannes Graf <[email protected]>
Signed-off-by: Petra Scherer <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Co-authored-by: Joo <[email protected]>
Co-authored-by: Lila <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Co-authored-by: Joo <[email protected]>
Signed-off-by: Timo Klenk <[email protected]>
Careful: Requires an MP-SPDZ image that was built with `NO_CLIENT_TLS`

Signed-off-by: Timo Klenk <[email protected]>
We don't need it if we properly handle the result on the caller-side

Signed-off-by: Timo Klenk <[email protected]>
@kindlich kindlich force-pushed the feature/upstream-new-mp-spdz branch from 8dbbf0d to f29277c Compare May 5, 2022 13:25
@sbckr
Copy link
Member

sbckr commented May 6, 2022

@grafjo @strieflin @kindlich
I have verified that the Millionaires Demo is working with the changes introduces with this PR#15 and the latest spdz base image. Only minor updates on carbynestack/carbynestack and the demo instructions in carbynestack/carbynestack.github.io are required

Test was performed as follows:

git fetch origin pull/15/head:update-spdz
git checkout update-spdz
  • Change Ephemeral base-image in .ko.yaml to ghcr.io/carbynestack/spdz:642d11f
#
# Copyright (c) 2021 - for information on the respective copyright owner
# see the NOTICE file and/or the repository https://github.com/carbynestack/ephemeral.
#
# SPDX-License-Identifier: Apache-2.0
#
# defaultBaseImage: ghcr.io/carbynestack/ubuntu:20.04-20210827
# baseImageOverrides:
#   github.com/carbynestack/ephemeral/cmd/ephemeral: ghcr.io/carbynestack/ephemeral-spdz-base-image:cleared-20210827
defaultBaseImage: ghcr.io/carbynestack/ubuntu:20.04-20210827-nonroot
baseImageOverrides:
    github.com/carbynestack/ephemeral/cmd/ephemeral: ghcr.io/carbynestack/spdz:642d11f
  • Build Ephemeral images using
ko publish -B --tags=spdz-update \
	github.com/carbynestack/ephemeral/cmd/discovery \
	github.com/carbynestack/ephemeral/cmd/ephemeral \
	github.com/carbynestack/ephemeral/cmd/network-controller -L
  • Load Docker images into kind clusters
kind load docker-image ko.local/network-controller:spdz-update  --name apollo
kind load docker-image ko.local/network-controller:spdz-update  --name starbuck
kind load docker-image ko.local/discovery:spdz-update  --name apollo
kind load docker-image ko.local/discovery:spdz-update  --name starbuck
kind load docker-image ko.local/ephemeral:spdz-update  --name apollo
kind load docker-image ko.local/ephemeral:spdz-update  --name starbuck
  • Update ephemeralhelm chart version carbynestack/carbynestack helmfile.d/0300.ephemeral.yaml
[...]
releases:
- name: {{ requiredEnv "RELEASE_NAME" }}-ephemeral
  chart: carbynestack-oci/ephemeral
  version: "0.1-SNAPSHOT-2275528139-15-7c3f0e0"
[...]
  • Deploy Carbyne Stack
export APOLLO_FQDN="172.18.1.128.sslip.io"                                   
export STARBUCK_FQDN="172.18.2.128.sslip.io"                                   
export RELEASE_NAME=cs                                              
export DISCOVERY_MASTER_HOST=$APOLLO_FQDN                             
export NO_SSL_VALIDATION=true                                       
export DISCOVERY_IMAGE_REGISTRY=ko.local
export DISCOVERY_IMAGE_REPOSITORY=discovery
export DISCOVERY_IMAGE_TAG=spdz-update                                       
export EPHEMERAL_IMAGE_REGISTRY=ko.local
export EPHEMERAL_IMAGE_REPOSITORY=ephemeral
export EPHEMERAL_IMAGE_TAG=spdz-update                                       
export NETWORK_CONTROLLER_IMAGE_REGISTRY=ko.local
export NETWORK_CONTROLLER_IMAGE_REPOSITORY=network-controller
export NETWORK_CONTROLLER_IMAGE_TAG=spdz-update

export FRONTEND_URL=$STARBUCK_FQDN
export IS_MASTER=false
export AMPHORA_VC_PARTNER_URI=http://$APOLLO_FQDN/amphora
kubectl config use-context kind-starbuck
helmfile apply

export FRONTEND_URL=$APOLLO_FQDN
export IS_MASTER=true
export AMPHORA_VC_PARTNER_URI=http://$STARBUCK_FQDN/amphora
export CASTOR_SLAVE_URI=http://$STARBUCK_FQDN/castor
kubectl config use-context kind-apollo
helmfile apply
export CLI_VERSION=0.1-SNAPSHOT-1576571202-7-cf3db5b
curl -o cs.jar -L https://github.com/carbynestack/cli/releases/download/$CLI_VERSION/cli-$CLI_VERSION-jar-with-dependencies.jar

mkdir -p ~/.cs
cat <<EOF | envsubst > ~/.cs/config
{
	"prime" : 198766463529478683931867765928436695041,
	"r" : 141515903391459779531506841503331516415,
	"noSslValidation" : true,
	"trustedCertificates" : [ ],
	"providers" : [ {
	  "amphoraServiceUrl" : "http://$apollo_fqdn/amphora",
	  "castorServiceUrl" : "http://$apollo_fqdn/castor",
	  "ephemeralServiceUrl" : "http://$apollo_fqdn/",
	  "id" : 1,
	  "baseUrl" : "http://$apollo_fqdn/"
	}, {
	  "amphoraServiceUrl" : "http://$starbuck_fqdn/amphora",
	  "castorServiceUrl" : "http://$starbuck_fqdn/castor",
	  "ephemeralServiceUrl" : "http://$starbuck_fqdn/",
	  "id" : 2,
	  "baseUrl" : "http://$starbuck_fqdn/"
	} ],
	"rinv" : 133854242216446749056083838363708373830
}
EOF

curl -O -L https://github.com/carbynestack/base-images/raw/3595c5427915b2f9e1f22804e3f742cda9e72312/fake-crypto-material.zip
unzip -d crypto-material fake-crypto-material.zip
rm fake-crypto-material.zip

cat << 'EOF' > upload-tuples.sh
#!/bin/bash
SCRIPT_PATH="$( cd "$(dirname "$0")" ; pwd -P )"
TUPLE_FOLDER=${SCRIPT_PATH}/crypto-material/2-128-40
CLI_PATH=${SCRIPT_PATH}
NUMBER_OF_CHUNKS=1

function uploadTuples {
	 echo ${NUMBER_OF_CHUNKS}
	 for type in INPUT_MASK_GFP MULTIPLICATION_TRIPLE_GFP; do
	    for (( i=0; i<${NUMBER_OF_CHUNKS}; i++ )); do
	       local chunkId=$(uuidgen)
	       echo "Uploading ${type} to [http://${APOLLO_FQDN}/castor](http://$%7bAPOLLO_FQDN%7d/castor) (Apollo)"
	       java -jar ${CLI_PATH}/cs.jar castor upload-tuple -f ${TUPLE_FOLDER}/Triples-p-P0 -t ${type} -i ${chunkId} 1
	       local statusMaster=$?
	       echo "Uploading ${type} to [http://${STARBUCK_FQDN}/castor](http://$%7bSTARBUCK_FQDN%7d/castor) (Starbuck)"
	       java -jar ${CLI_PATH}/cs.jar castor upload-tuple -f ${TUPLE_FOLDER}/Triples-p-P1 -t ${type} -i ${chunkId} 2
	       local statusSlave=$?
	       if [[ "${statusMaster}" -eq 0 && "${statusSlave}" -eq 0 ]]; then
	          java -jar ${CLI_PATH}/cs.jar castor activate-chunk -i ${chunkId} 1
	          java -jar ${CLI_PATH}/cs.jar castor activate-chunk -i ${chunkId} 2
	       else
	          echo "ERROR: Failed to upload one tuple chunk - not activated"
	       fi
	    done
	 done
}

uploadTuples
EOF
chmod 755 upload-tuples.sh
./upload-tuples.sh
  • Perform Millionaires

⚠️ ensure Java 1.8 is used

export JEFFS_NET_WORTH_ID=$(java -jar cs.jar amphora create-secret 177 -t billionaire=Jeff)
export ELONS_NET_WORTH_ID=$(java -jar cs.jar amphora create-secret 151 -t billionaire=Elon)

cat << 'EOF' > billionaires.mpc
# Prologue to read in the inputs
port=regint(10000)
listen(port)
socket_id = regint()
acceptclientconnection(socket_id, port)
v = sint.read_from_socket(socket_id, 2)

# The logic
first_billionaires_net_worth = v[0]
second_billionaires_net_worth= v[1]
result = first_billionaires_net_worth < second_billionaires_net_worth

# Epilogue to return the outputs 
resp = Array(1, sint)
resp[0] = result
sint.write_to_socket(socket_id, resp)
EOF

export RESULT_ID=$(cat billionaires.mpc | java -jar cs.jar ephemeral execute \
	-i $JEFFS_NET_WORTH_ID \
	-i $ELONS_NET_WORTH_ID \
	ephemeral-generic.default \
	| tail -n +2 \
	| sed 's/[][]//g')

java -jar cs.jar amphora get-secret $RESULT_ID

I will provide pull requests to cover the required changes mentioned. However, this requires #15 to be merged

Copy link
Member

@strieflin strieflin left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/utils/os.go Outdated
err = command.Wait()
defer waitGroup.Done()
if err != nil {
println(fmt.Sprintf("Error occured!"))
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to get a logger from the context using:

logger := log.FromContext(ctx)

@strieflin
Copy link
Member

Thanks a lot @sbckr for testing this. That has been the last open point on our to-do list (see #15 (comment)). Ready to merge.

@strieflin strieflin merged commit c43d17c into carbynestack:master May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants