Add NNS Challenge support #1
Labels
No labels
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/lego#1
Loading…
Reference in a new issue
No description provided.
Delete branch "mbiryukova/lego:feature/add_nns_challenge"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
2ee8e19a37
to604b9b6e91
604b9b6e91
tode42ce9ece
de42ce9ece
tof765b0a087
@ -0,0 +37,4 @@
invoker interface {
Call(contract util.Uint160, operation string, params ...any) (*result.Invoke, error)
}
As I proposed in TrueCloudLab/certificates#1 (comment), let's remove
invoker
interface and usemultiSchemeClient
interface to simplify the code@ -0,0 +95,4 @@
}
// Present creates a TXT record using the specified parameters to fulfill the nns-01 challenge.
func (n *NNSProvider) Present(domain, token, keyAuth string) error {
Present
andCleanup
functions implementProvider
interface so we can useNNSProvider
asProvider
type. It worth to mention it in the comment.@ -0,0 +101,4 @@
return fmt.Errorf("retrieve wallet from file: %w", err)
}
acc := w.Accounts[n.accNumber]
acc.Decrypt(n.accPassword, w.Scrypt)
I think you can decrypt account once in
NewNNSProvider
function, store account in theNNSProvider
structure and remove fields likewalletFile
,accPassword
, etc. What do you think?@ -0,0 +103,4 @@
acc := w.Accounts[n.accNumber]
acc.Decrypt(n.accPassword, w.Scrypt)
cli, err := rpcclient.New(context.Background(), n.nnsServer, rpcclient.Options{})
You should either close the client in the end of the function. Or you can client in the
NewNNSProvider
and reuse it in all functions. The same forCleanUp
function.@ -127,0 +134,4 @@
switch {
case !ctx.IsSet("wallet"):
log.Fatal("No wallet file provided for nns challenge.")
case !ctx.IsSet("wallet.account-number"):
Let's do a bit different here. While account number is okay for prototyping, in production we do not operate with account positions. Instead we use account address, which is a short representation of account public key.
Pretty much all wallet configuration in our project looks like this:
To get required account you can do something like this
f765b0a087
toe07e209879
@ -0,0 +40,4 @@
core: core,
validate: validate,
provider: provider,
nnsTimeout: 10 * time.Second,
Can't find a usage of this timeout.
@ -0,0 +43,4 @@
acc := w.GetAccount(accAddress)
acc.Decrypt(accountPassword, w.Scrypt)
cli, err := rpcclient.New(context.Background(), nnsServer, rpcclient.Options{})
Let's redo this a little bit.
We create
cli
there to use it foractor.NewSimple()
andactor.NewPollingWaiter()
. First one requiresactor.RPCActor
interface. Second one requiresactor.RPCPollingWaiter
interface.Both interfaces are implemented by client and websocket client which are created in
multiSchemeClient
. So I think we can expandmultiSchemeClient
with two new interfaces and reuse it insteadrpcclient
. One less client and a little bit less code.@ -0,0 +134,4 @@
// CleanUp removes the TXT record matching the specified parameters.
// It implements Provider interface in order to use NNSProvider as Provider.
func (n *NNSProvider) CleanUp(domain, token, keyAuth string) error {
defer n.close()
This
defer
looks a bit strange. Defers are usually placed where resource is created to not forget to remove it.Here is the different case. I think we can simply call
n.close()
at the end of this function.Also, is it correct that after
CleanUp
there will be noPresent()
invocations?In case of using CLI - yes. If we call the challenge several times from the code - this solution with closing in
CleanUp
isn't suitable@ -127,0 +136,4 @@
case !ctx.IsSet("wallet"):
log.Fatal("No wallet file provided for nns challenge.")
case !ctx.IsSet("wallet.account-address"):
log.Fatal("No address of account from wallet file provided.")
It seems we can use default address if this flag
wallet.account-address
was not provided@ -0,0 +64,4 @@
}
// Generate the Key Authorization for the challenge
keyAuth, err := c.core.GetKeyAuthorization(chlng.Token)
It seems this code is the same for
PreSolve
,Solve``,
CleanUp` methods. Can we extract it to separate private method and to reuse?e07e209879
to0a1ad0970c
0a1ad0970c
toa49270bb2e
@ -0,0 +17,4 @@
type ChallengeOption func(*Challenge) error
// CondOption Conditional challenge option.
func CondOption(condition bool, opt ChallengeOption) ChallengeOption {
Currently, we have no option so let's drop this
CondOption
@ -0,0 +50,4 @@
}
}
acc := w.GetAccount(address)
acc.Decrypt(accountPassword, w.Scrypt)
Why don't we check error here?
@ -0,0 +100,4 @@
// Present creates a TXT record using the specified parameters to fulfill the nns-01 challenge.
// It implements Provider interface in order to use NNSProvider as Provider.
func (n *NNSProvider) Present(domain, token, keyAuth string) error {
token
isn't used. So let's writePresent(domain, _, keyAuth string)
The same for
CleanUp
@ -0,0 +101,4 @@
// Present creates a TXT record using the specified parameters to fulfill the nns-01 challenge.
// It implements Provider interface in order to use NNSProvider as Provider.
func (n *NNSProvider) Present(domain, token, keyAuth string) error {
n.dial()
Why don't we check error?
@ -0,0 +95,4 @@
// Close closes connections of multiSchemeClient.
func (n *NNSProvider) close() {
n.client.Close()
n.client = nil
We do we set
n.client
tonil
?a49270bb2e
tof1c224c819
@ -0,0 +105,4 @@
func (n *NNSProvider) Present(domain, _, keyAuth string) error {
err := n.dial()
if err != nil {
n.client.Close()
Actually, here
panic
can happen (ifn.client
isnil
), when this is the first invocation and we couldn't create client indial
.By the way, why don't we init client once (in
NewNNSProvider
) and makeNNSProvider.close
to be exported (so that client code that will useNNSProvided
close connection explicitly). I don't know how often client code will invokePresent
method, so I don't insist on this changesRemoved this line because
CleanUp
with close will be called.If we will use lego as a library,
Present
can be called several times as well asCleanUp
, and we can init client inNewNNSProvider
and close it explicitly. But if we use lego as CLI there is no place where to close client exceptCleanUp
. So I think the most logical way is to init client inPresent
and close inCleanUp
f1c224c819
to79c5b83559