nfsv4.0: fix handling of SETCLIENTID operation

fix corner cases of SETCLIENTID operation as specified in rfc7530

Acked-by: Karen Hoyos
Target: master

Author: kofemann

core/src/main/java/org/dcache/nfs/v4/NFS4Client.java

@@ -89,9 +89,14 @@ public class NFS4Client {
     private final byte[] _ownerId;
 
     /**
-     * Verifier that is used to detect client reboots.
+     * Client side generated verifier that is used to detect client reboots.
      */
-    private final verifier4 _verifier;
+    private final verifier4 _clientVerifier;
+
+    /**
+     * Server side generated verifier that is used to detect retry.
+     */
+    private verifier4 _serverVerifier;
 
     /**
      * The RPCSEC_GSS principal sent via the RPC headers.
@@ -194,7 +199,8 @@ public NFS4Client(NFSv4StateHandler stateHandler, clientid4 clientId, int minorV
         _stateHandler = stateHandler;
         _clock = _stateHandler.getClock();
         _ownerId = Arrays.copyOf(ownerID, ownerID.length);
-        _verifier = verifier;
+        _clientVerifier = verifier;
+        _serverVerifier = verifier4.valueOf(System.currentTimeMillis());
         _principal = principal;
         _clientId = clientId;
 
@@ -236,8 +242,8 @@ public byte[] getOwnerId() {
      *
      * @return client generated verifier
      */
-    public verifier4 verifier() {
-        return _verifier;
+    public verifier4 serverGeneratedVerifier() {
+        return _serverVerifier;
     }
 
     /**
@@ -248,10 +254,15 @@ public clientid4 getId() {
         return _clientId;
     }
 
-    public boolean verifierEquals(verifier4 verifier) {
-        return _verifier.equals(verifier);
+    public boolean clientGeneratedVerifierEquals(verifier4 verifier) {
+        return _clientVerifier.equals(verifier);
     }
 
+    public boolean serverGeneratedVerifierEquals(verifier4 verifier) {
+        return _serverVerifier.equals(verifier);
+    }
+
+
     public synchronized boolean isConfirmed() {
         return _isConfirmed;
     }
@@ -294,6 +305,7 @@ public void refreshLeaseTime() {
     public synchronized void reset() {
         refreshLeaseTime();
         _isConfirmed = false;
+        _serverVerifier = verifier4.valueOf(System.currentTimeMillis());
     }
 
     /**

core/src/main/java/org/dcache/nfs/v4/OperationEXCHANGE_ID.java

@@ -156,7 +156,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
                 throw new NoEntException("no such client");
             }
 
-            if (!client.verifierEquals(verifier)) {
+            if (!client.clientGeneratedVerifierEquals(verifier)) {
                 _log.debug("case 8: Update but Wrong Verifier");
                 throw new NotSameException("Update but Wrong Verifier");
             }
@@ -181,7 +181,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
             } else {
 
                 if (client.isConfirmed()) {
-                    if (client.verifierEquals(verifier) && principal.equals(client.principal())) {
+                    if (client.clientGeneratedVerifierEquals(verifier) && principal.equals(client.principal())) {
                         _log.debug("Case 2: Non-Update on Existing Client ID");
                         client.refreshLeaseTime();
                     } else if (principal.equals(client.principal())) {

core/src/main/java/org/dcache/nfs/v4/OperationSETCLIENTID.java

@@ -55,28 +55,53 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
         final byte[] id = _args.opsetclientid.client.id;
         NFS4Client client = context.getStateHandler().clientByOwner(id);
 
-        if (client != null && client.isConfirmed() && client.isLeaseValid()) {
 
-            if (!client.principal().equals(context.getPrincipal())) {
-                netaddr4 addr = new netaddr4(client.getRemoteAddress());
-                res.status = nfsstat.NFSERR_CLID_INUSE;
-                res.client_using = new clientaddr4(addr);
-                throw new ClidInUseException();
-            }
-            client.reset();
+        if (client == null) {
+            // new client
+            client = context.getStateHandler().createClient(
+                    context.getRemoteSocketAddress(),
+                    context.getLocalSocketAddress(),
+                    context.getMinorversion(),
+                    _args.opsetclientid.client.id, _args.opsetclientid.client.verifier,
+                    context.getPrincipal(), false);
+        } else if (!client.isConfirmed()) {
 
-        } else {
+            // existing client, but not confirmed. either retry or client restarted before confirmation
+            context.getStateHandler().removeClient(client);
             client = context.getStateHandler().createClient(
                     context.getRemoteSocketAddress(),
                     context.getLocalSocketAddress(),
                     context.getMinorversion(),
                     _args.opsetclientid.client.id, _args.opsetclientid.client.verifier,
                     context.getPrincipal(), false);
+
+        } else if (!client.clientGeneratedVerifierEquals(verifier)) {
+
+            // existing client, different verifier. Client rebooted.
+            // create new record, keep the old one as required by the RFC 7530
+            client = context.getStateHandler().createClient(
+                    context.getRemoteSocketAddress(),
+                    context.getLocalSocketAddress(),
+                    context.getMinorversion(),
+                    _args.opsetclientid.client.id, _args.opsetclientid.client.verifier,
+                    context.getPrincipal(), false);
+
+        } else if (client.isLeaseValid()) {
+
+            // can't be reused, if principal have changes and client has state
+            if (!client.principal().equals(context.getPrincipal()) && client.hasState()) {
+                netaddr4 addr = new netaddr4(client.getRemoteAddress());
+                res.status = nfsstat.NFSERR_CLID_INUSE;
+                res.client_using = new clientaddr4(addr);
+                throw new ClidInUseException();
+            }
+
+            client.reset();
         }
 
         res.resok4 = new SETCLIENTID4resok();
         res.resok4.clientid = client.getId();
-        res.resok4.setclientid_confirm = client.verifier();
+        res.resok4.setclientid_confirm = client.serverGeneratedVerifier();
         res.status = nfsstat.NFS_OK;
     }
 }

core/src/main/java/org/dcache/nfs/v4/OperationSETCLIENTID_CONFIRM.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009 - 2017 Deutsches Elektronen-Synchroton,
+ * Copyright (c) 2009 - 2025 Deutsches Elektronen-Synchroton,
  * Member of the Helmholtz Association, (DESY), HAMBURG, GERMANY
  *
  * This library is free software; you can redistribute it and/or modify
@@ -49,7 +49,7 @@ public void process(CompoundContext context, nfs_resop4 result) throws ChimeraNF
         NFS4Client client = context.getStateHandler().getClient(_args.opsetclientid_confirm.clientid);
 
         res.status = nfsstat.NFSERR_INVAL;
-        if (client.verifierEquals(_args.opsetclientid_confirm.setclientid_confirm)) {
+        if (client.serverGeneratedVerifierEquals(_args.opsetclientid_confirm.setclientid_confirm)) {
             res.status = nfsstat.NFS_OK;
             client.setConfirmed();
         }

full-test.yml

@@ -24,7 +24,7 @@ services:
         sleep 10
         /run-nfs4.0.sh --xml=/report/xunit-report-v40.xml --maketree nfs4j:/data all nochar nosocket noblock nofifo \
           noACC2a noACC2b noACC2c noACC2d noACC2f noACC2r noACC2s \
-          noCID1 noCID2 noCID4a noCID4b noCID4c noCID4d noCID4e \
+          noCID1 noCID4d \
           noCLOSE10 noCLOSE12 noCLOSE5 noCLOSE6 noCLOSE8 noCLOSE9 \
           noCMT1aa noCMT1b noCMT1c noCMT1d noCMT1e noCMT1f noCMT2a noCMT2b noCMT2c noCMT2d noCMT2f \
           noCMT2s noCMT3 noCMT4 noCR12 noLKT1 noLKT2a noLKT2b noLKT2c noLKT2d noLKT2f noLKT2s noLKT3 \