muenchnerkindl
commented
1 year ago Thank you for sharing your valuable contribution. I believe it would make a very nice addition to the collection of TLA+ specifications.
A few comments on how you could improve your spec, in roughly decreasing order of importance:
- Consider replacing certain CHOOSE expressions by existential quantification so that all possible matches will be considered when model checking. Here is a concrete example of how I'd define one of the actions in your spec, but there are several similar patterns:
FwProcAclTimeOut == \E r \in AclRuleSet : *aging and deleted randomly,remove from current rule table /\ r.sPort = MATCH_ANY /\ FwState = "Work" /\ AclRuleSet # {} * --- actually superfluous because we require r \in AclRuleSet /\ AclRuleSet' = AclRuleSet \ {r} /\ AgedRuleSet' = AgedRuleSet \cup {r} * record aged rule into log. /\ UNCHANGED user_vars /\ UNCHANGED sdpsvr_vars /\ UNCHANGED attacker_vars /\ UNCHANGED <<FwState,DropPackets>> /\ UNCHANGED server_vars /\ UNCHANGED Public_vars
As a rule of thumb, CHOOSE should be used only if there is exactly one value that matches the predicate.
-
I understand that your Encrypt / Decrypt operators are only approximations of the real functions, but do you actually need such a concrete representation? My understanding is that you work in the Dolev-Yao model, and there one typically abstracts these operations as uninterpreted terms that only ensure that Decrypt(Encrypt(d,k), k) = d while Decrypt(Encrypt(d,k), k') gives a meaningless result when k' # k.
-
Introduce LET definitions to eliminate common subexpressions in order to make definitions more readable, for example
UsrRcvSynAck ==
/\ uState = "Connecting"
/\ uTCPLinkSet # {}
/\ uChannel # <<>>
/\ Head(uChannel).Flg = "TCP_SYN_ACK"
/\ Head(uChannel).Type = "User"
/\ HasMatchLink(Head(uChannel),uTCPLinkSet) * Receive TCP_SYN_ACK from target server and match the connecting TCP socket
/\ LET l == GetMatchLink(Head(uChannel),uTCPLinkSet)
IN uTCPLinkSet' = (uTCPLinkSet \ {l})
\cup { [sIP |-> l.sIP,
sPort |-> l.sPort,
dIP |-> l.dIP,
dPort |-> l.dPort,
State |-> "ESTABLISHED" * Updata TCP link status to established
]
}
/\ uState' = "Connected" * The user successfully access the target server
/\ uChannel' = Tail(uChannel) *Send TCP ACK packet (the last step of hand shake) to target server
/\ FwDataChannel' = Append(FwDataChannel, EndPointBulidTcpAckPkt(Head(uChannel),"User"))
/\ UNCHANGED <<uIP, uID, Key, uTstamp, uSDPSvrInfo, uSvrInfo, uAuthSession>>
/\ UNCHANGED sdpsvr_vars
/\ UNCHANGED fw_vars
/\ UNCHANGED attacker_vars
/\ UNCHANGED server_vars
/\ UNCHANGED <<AuthChannel,FwCtlChannel,aChannel,sChannel>>
- Replace IF ... THEN ... ELSE expressions that return TRUE or FALSE by formulas. Two examples:
FindAntiReplay(msg,wnd) == \E r \in wnd : (msg.ClientID = r.ClientID /\ msg.Tstamp = r.Tstamp)
NewLink(p,LinkSet) == \/ LinkSet = {} * -- in fact, this disjunct is superfluous because \A x \in {} : ... is TRUE \/ \A x \in LinkSet: ( *without matching TCB (TCP Control Block) \/ x.sIP # p.sIP \/ x.dIP # p.dIP \/ x.sPort # p.sPort \/ x.dPort # p.dPort )
-
Parentheses around properly aligned conjunction / disjunction lists are superfluous.
-
Make use of operators defined in the Community Modules. In particular, you include a copy of module Bitwise, which is part of the community modules. (But perhaps you don't need those operators at all, see above remark, and I am not even sure if you currently use them in your spec, given that you define your own XOR?) Also, your operator Seq2Set could be replaced by either ToSet from module SequencesExt or Range from module Functions.
-
While you are at it, try to reduce the number of typos, for example replace UsrBulidTcpSynPkt by UsrBuildTcpSynPkt, and improve the clarity of comments, for example I don't really know how to interpret "If a data access link with an invalid authentication session ID, it means we don't know the data access is resulted by which Auth session".
When you are satisfied with your spec, please open a pull request so that we can merge it into the collection.
Thanks again!
Stephan
lemmy
My case to share is about the TLA+ Spec of Software Defined Perimeter (SDP) architecture and algorithm based on the open source project fwknop.
https://github.com/10227694/SDP_Verification
The subdirectory SDP_Attack_Spec contains the specification based on the following materials: ( https://cloudsecurityalliance.org/artifacts/software-defined-perimeter-zero-trust-specification-v2/ ) ( http://www.cipherdyne.org/fwknop/ )
The subdirectory SDP_Attack_New_Solution_Spec contains the specification for the improved SDP architecture and algorithm which fixed the discovered vulnerability.
The slide "Specifying and Verifying SDP Protocol Based Zero Trust Architecture Using TLA+.pptx" contains the key description of the reserach work.
For detail descriptions, please refer to:
https://dl.acm.org/doi/10.1145/3558819.3558826