aesmd: allow to customize quote provider library, init sgx-azure-dcap-client at 1.11.2 #203449

Trundle · 2022-11-28T13:07:34Z

Description of changes

aesmd: allow to customize quote provider library
sgx-azure-dcap-client: init at 1.11.2

Tested in an Azure VM (with Enarx)

Things done

rvolosatovs

Mostly some questions

rvolosatovs · 2022-11-30T08:31:44Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_report.h";
+      hash = "sha256-NCDH3uhSlRRx0DDA/MKhWlUnA1rJ94O4DLuzqmnfr0I=";
+    })
+    (fetchurl {
+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_key.h";
+      hash = "sha256-3ApIE2QevE8MeU0y5UGvwaKD9OOJ3H9c5ibxsBSr49g=";
+    })
+    (fetchurl {
+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_attributes.h";


since these 3 come from the same repo/HEAD, how about introducing a variable for the commit hash to simplify maintenance?

We also thought about that but when we realized that those were touched four years ago for the last time, we decided it's probably not worth it.

We could do a sparse checkout to not rely on 3 FODs.

See latest push. I’m not sure if that’s really much of an improvement overall.

rvolosatovs · 2022-11-30T08:32:44Z

nixos/modules/services/security/aesmd.nix

@@ -83,7 +89,6 @@ in
        storeAesmFolder = "${sgx-psw}/aesm";
        # Hardcoded path AESM_DATA_FOLDER in psw/ae/aesm_service/source/oal/linux/aesm_util.cpp
        aesmDataFolder = "/var/opt/aesmd/data";
-        aesmStateDirSystemd = "%S/aesmd";


this was unused?

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

veehaitch · 2022-11-30T08:45:24Z

@ofborg test aesmd

nixos-discourse · 2022-12-02T21:42:11Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/699

SuperSandro2000 · 2022-12-03T04:39:09Z

pkgs/os-specific/linux/sgx/psw/default.nix

@@ -121,7 +121,7 @@ stdenv.mkDerivation rec {

    mkdir $out/bin
    makeWrapper $out/aesm/aesm_service $out/bin/aesm_service \
-      --prefix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ protobuf ]}:$out/aesm \
+      --suffix LD_LIBRARY_PATH : ${lib.makeLibraryPath [ protobuf ]}:$out/aesm \


There shouldn't be any files lik aesm at $out in a normal package

SuperSandro2000 · 2022-12-03T04:40:11Z

pkgs/os-specific/linux/sgx/azure-dcap-client/test-suite.nix

+  # No need to build `sgx-azure-dcap-client` again
+  postPatch = ''
+    substituteInPlace ./src/Linux/Makefile.in \
+      --replace '$(TEST_SUITE): $(PROVIDER_LIB) $(TEST_SUITE_OBJ)' \
+                '$(TEST_SUITE): $(TEST_SUITE_OBJ)'
+  '';


I think we should do all patching in the default.nix if possible

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

SuperSandro2000 · 2022-12-03T04:41:03Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+  configurePhase = ''
+    runHook preConfigure
+
+    substitute src/Linux/Makefile{.in,} \
+      --replace '##CURLINC##' '${curl.dev}/include/curl/' \
+      --replace '-Wall' '-Wall -Wno-deprecated-declarations' \
+      --replace 'prefix = /usr/local' 'prefix ='
+
+    runHook postConfigure
+  '';


We are not running any configure steps here so it can all go to postPatch

SuperSandro2000 · 2022-12-03T04:41:17Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+  patchPhase = ''
+    runHook prePatch
+
+    mkdir src/Linux/ext
+    ln -s ${headers} src/Linux/ext/intel
+
+    runHook postPatch
+  '';


Suggested change

patchPhase = ''

runHook prePatch

mkdir src/Linux/ext

ln -s ${headers} src/Linux/ext/intel

runHook postPatch

'';

postPatch = ''

mkdir src/Linux/ext

ln -s ${headers} src/Linux/ext/intel

'';

SuperSandro2000 · 2022-12-03T04:41:30Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+  ];
+
+  buildInputs = [
+    curl.dev


Suggested change

curl.dev

curl

that should be chosen automatically

SuperSandro2000 · 2022-12-03T04:42:15Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_report.h";
+      hash = "sha256-NCDH3uhSlRRx0DDA/MKhWlUnA1rJ94O4DLuzqmnfr0I=";
+    })
+    (fetchurl {
+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_key.h";
+      hash = "sha256-3ApIE2QevE8MeU0y5UGvwaKD9OOJ3H9c5ibxsBSr49g=";
+    })
+    (fetchurl {
+      url = "https://raw.githubusercontent.com/intel/linux-sgx/1ccf25b64abd1c2eff05ead9d14b410b3c9ae7be/common/inc/sgx_attributes.h";


We could do a sparse checkout to not rely on 3 FODs.

SuperSandro2000 · 2022-12-03T04:43:02Z

nixos/modules/services/security/aesmd.nix

+    environment = mkOption {
+      type = with types; attrsOf str;
+      default = { };
+      description = mdDoc "Additional environment variables to pass to the AESM service.";
+      # Example environment variable for `sgx-azure-dcap-client` provider library
+      example = {
+        AZDCAP_COLLATERAL_VERSION = "v2";
+        AZDCAP_DEBUG_LOG_LEVEL = "INFO";
+      };
+    };


I don't think we need that and should advice people to use the option directly instead.

You think they should use config.systemd.services.aesmd.environment? I don't think that's very ergonomic. How should we advice people?

Mic92 · 2022-12-04T11:01:17Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+
+    substitute src/Linux/Makefile{.in,} \
+      --replace '##CURLINC##' '${curl.dev}/include/curl/' \
+      --replace '-Wall' '-Wall -Wno-deprecated-declarations' \


This is usually more robust since it does not require patching:

NIX_CFLAGS_COMPILE = "-Wno-deprecated-declarations";

Mic92 · 2022-12-04T11:03:49Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+  installFlags = [
+    "DESTDIR=$(out)"
+  ];


Suggested change

installFlags = [

"DESTDIR=$(out)"

];

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

Mic92 · 2022-12-04T11:04:10Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+    substitute src/Linux/Makefile{.in,} \
+      --replace '##CURLINC##' '${curl.dev}/include/curl/' \
+      --replace '-Wall' '-Wall -Wno-deprecated-declarations' \
+      --replace 'prefix = /usr/local' 'prefix =' \


Suggested change

--replace 'prefix = /usr/local' 'prefix =' \

Mic92 · 2022-12-04T11:07:11Z

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix

+      --replace '##CURLINC##' '${curl.dev}/include/curl/' \
+      --replace '-Wall' '-Wall -Wno-deprecated-declarations' \
+      --replace 'prefix = /usr/local' 'prefix =' \
+      --replace '$(TEST_SUITE): $(PROVIDER_LIB) $(TEST_SUITE_OBJ)' '$(TEST_SUITE): $(TEST_SUITE_OBJ)'


Is provider lib not build by the project anymore?

This just makes sure that the provider lib is not built again when building the test suite. Also see #203449 (comment)

Changes sgx-psw to append `aesm` to `LD_LIBRARY_PATH`: - Append instead of prepend to allow for overriding in service config - As we already add a wrapper to add `aesm` to `LD_LIBRARY_PATH` it is not necessary to also set in `LD_LIBRARY_PATH` of the systemd service. Co-authored-by: Vincent Haupert <mail@vincent-haupert.de>

veehaitch · 2022-12-13T08:32:18Z

Anything else we should do here, @SuperSandro2000 @Mic92?

nixos-discourse · 2022-12-24T14:19:34Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1594

github-actions bot added 6.topic: nixos 8.has: module (update) labels Nov 28, 2022

Trundle requested a review from rvolosatovs November 28, 2022 13:08

Trundle force-pushed the azure-quote-provider branch from 20a19c4 to 8591a44 Compare November 28, 2022 13:12

ofborg bot added the 8.has: package (new) label Nov 28, 2022

ofborg bot requested review from RealityAnomaly and veehaitch November 28, 2022 13:24

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Nov 28, 2022

veehaitch force-pushed the azure-quote-provider branch from 8591a44 to 0b94244 Compare November 29, 2022 15:41

rvolosatovs approved these changes Nov 30, 2022

View reviewed changes

veehaitch added the 12.approvals: 1 label Nov 30, 2022

veehaitch requested a review from Mic92 November 30, 2022 08:58

SuperSandro2000 reviewed Dec 3, 2022

View reviewed changes

veehaitch force-pushed the azure-quote-provider branch from 0b94244 to b621aa1 Compare December 3, 2022 09:17

mweinelt removed the 12.approvals: 1 label Dec 3, 2022

Mic92 reviewed Dec 4, 2022

View reviewed changes

pkgs/os-specific/linux/sgx/azure-dcap-client/default.nix Show resolved Hide resolved

Mic92 reviewed Dec 4, 2022

View reviewed changes

Trundle and others added 4 commits December 4, 2022 20:12

sgx-azure-dcap-client: init at 1.11.2

7de32b0

sgx-azure-quote-provider: add test-suite derivation

4e937f0

nixos/aesmd: add option environment

dbff3c2

veehaitch force-pushed the azure-quote-provider branch from b621aa1 to dbff3c2 Compare December 4, 2022 19:13

veehaitch requested a review from Mic92 December 4, 2022 23:50

veehaitch mentioned this pull request Dec 12, 2022

sgx-sdk: fix linking issues with tests #205789

Merged

13 tasks

SuperSandro2000 merged commit c8c8ac5 into NixOS:master Dec 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aesmd: allow to customize quote provider library, init sgx-azure-dcap-client at 1.11.2 #203449

aesmd: allow to customize quote provider library, init sgx-azure-dcap-client at 1.11.2 #203449

Trundle commented Nov 28, 2022

rvolosatovs left a comment

rvolosatovs Nov 30, 2022 •

edited

Loading

veehaitch Nov 30, 2022

SuperSandro2000 Dec 3, 2022

veehaitch Dec 3, 2022

rvolosatovs Nov 30, 2022

veehaitch Nov 30, 2022

veehaitch commented Nov 30, 2022

nixos-discourse commented Dec 2, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

SuperSandro2000 Dec 3, 2022

veehaitch Dec 3, 2022

Mic92 Dec 4, 2022

Mic92 Dec 4, 2022

Mic92 Dec 4, 2022

Mic92 Dec 4, 2022

veehaitch Dec 4, 2022

veehaitch commented Dec 13, 2022

nixos-discourse commented Dec 24, 2022

aesmd: allow to customize quote provider library, init sgx-azure-dcap-client at 1.11.2 #203449

aesmd: allow to customize quote provider library, init sgx-azure-dcap-client at 1.11.2 #203449

Conversation

Trundle commented Nov 28, 2022

Description of changes

Things done

rvolosatovs left a comment

Choose a reason for hiding this comment

rvolosatovs Nov 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veehaitch commented Nov 30, 2022

nixos-discourse commented Dec 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veehaitch commented Dec 13, 2022

nixos-discourse commented Dec 24, 2022

rvolosatovs Nov 30, 2022 •

edited

Loading