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

opencap alias support #17

Merged
merged 2 commits into from
Sep 17, 2018
Merged

opencap alias support #17

merged 2 commits into from
Sep 17, 2018

Conversation

wagslane
Copy link
Contributor

@wagslane wagslane commented Sep 15, 2018

I added support for OpenCAP aliases. I tested to make sure the addresses are pulled in correctly. Obviously I raised an issue where my wallet isn't creating a receive block but that shouldn't change this PR.

I'm not sure if I messed up how you wanted to organize your code or not, let me know.

I think those generated files should NOT be in the repo. I was talking about the files below that should be in the repo:

  • libsciter-gtk-64.so
  • sciter.dll
  • sciter-osx-64.dylib

In order to test, make an alias at https://ogdolo.com

@inkeliz
Copy link
Member

inkeliz commented Sep 16, 2018

I not sure if it should be inside Util, or have they own folder, like Nanollet/OpenCAP. Seems that the http.Get doesn't set a timeout, maybe should create a http.Client{Timeout: ... } instead of use the DefaultClient?

I think this ifs:

addr, err := Util.LookupAlias(addrOrAlias)
if err != nil && err.Error() != "Invalid alias provided" {
	DOM.UpdateNotification(w, err.Error())
	return
} else if err != nil {
	addr = addrOrAlias
}

https://github.com/lane-c-wagner/Nanollet/blob/80633f81fc26efa62ec1fcf6299e8a28765d796d/GUI/App/nanollet.go#L67-L73

Something like a switch:

var address Wallet.Address
switch {
case Wallet.Address(addrOrAlias).IsValid():
	address = Wallet.Address(addrOrAlias)
case opencap.ValidateAlias(addrOrAlias)
	addr, err := Util.LookupAlias(addrOrAlias)
	append(errs, err)
	address = Wallet.Adress(addr)
default:
        err = ...
}

If LookupAlias returns a Wallet.Address, which is impossible inside Util, we can use directly address, error = AnotherLib.LookupAlias(addrOrAlias).

At least for me it's more easy to understand what is doing on, if it's already a valid address it is used as a address. If a new option becomes available is just a new case, in most of cases.

I'm testing it right now.

@inkeliz
Copy link
Member

inkeliz commented Sep 16, 2018

Should we add KeyPinning for already-know OpenCAP severs?

In the past, when uses a server (instead of connect to the nodes). So, the wallet have a hardcoded public-key, it prevents a MITM when the client trust in some insecure certificate authorities. In this case it was set in:

publickey: []byte{0x26, 0x97, 0xab, 0x86, 0x23, 0xd3, 0xab, 0x86, 0xee, 0x4c, 0xe3, 0x05, 0xe3, 0x25, 0x9f, 0xfc, 0xef, 0x04, 0x4a, 0x41, 0xc3, 0x59, 0x27, 0x1c, 0x36, 0x59, 0x0f, 0x85, 0xaf, 0x95, 0xee, 0xf4, 0x9f, 0x08, 0x10, 0x80, 0x8f, 0xb0, 0x60, 0x5f, 0xad, 0x55, 0xa0, 0x56, 0x74, 0x12, 0x56, 0x63, 0xee, 0x3a, 0x5c, 0xda, 0x3e, 0xa5, 0xed, 0xee, 0x01, 0x7e, 0x30, 0xec, 0x6c, 0x20, 0xf9, 0x4f},

Then it's used in the TlsConfig of the Dialer:

config.TlsConfig = &tls.Config{
InsecureSkipVerify: Config.IsDebugEnabled(),
MinVersion: tls.VersionTLS12,
CurvePreferences: []tls.CurveID{tls.X25519},
VerifyPeerCertificate: internal.VerifyPeerCertificate(c.publickey, c.endpoint),

I think should possible to do a Key Pinning using the Config, but I'm not sure how long should this key live. If the TLS key rotates too frequently, it's useless. If the key is intended to be replaced together with the certificate (every "certificate renew" uses a new key) we can set a lifetime based on the certificate.

@wagslane
Copy link
Contributor Author

I don't think we can do anything for "known opencap servers" because the point of OpenCAP is to be an opencap protocol and allow anyone to run a server. Ogdolo just happens to be the only one running now because its a new protocol. In the future there will hopefully be more servers than we can count!

@wagslane
Copy link
Contributor Author

wagslane commented Sep 16, 2018

I not sure if it should be inside Util, or have they own folder, like Nanollet/OpenCAP. Seems that the http.Get doesn't set a timeout, maybe should create a http.Client{Timeout: ... } instead of use the DefaultClient?

I think this ifs:

addr, err := Util.LookupAlias(addrOrAlias)
if err != nil && err.Error() != "Invalid alias provided" {
	DOM.UpdateNotification(w, err.Error())
	return
} else if err != nil {
	addr = addrOrAlias
}

https://github.com/lane-c-wagner/Nanollet/blob/80633f81fc26efa62ec1fcf6299e8a28765d796d/GUI/App/nanollet.go#L67-L73

Something like a switch:

var address Wallet.Address
switch {
case Wallet.Address(addrOrAlias).IsValid():
	address = Wallet.Address(addrOrAlias)
case opencap.ValidateAlias(addrOrAlias)
	addr, err := Util.LookupAlias(addrOrAlias)
	append(errs, err)
	address = Wallet.Adress(addr)
default:
        err = ...
}

If LookupAlias returns a Wallet.Address, which is impossible inside Util, we can use directly address, error = AnotherLib.LookupAlias(addrOrAlias).

At least for me it's more easy to understand what is doing on, if it's already a valid address it is used as a address. If a new option becomes available is just a new case, in most of cases.

I'm testing it right now.

LookupAlias returns the string representation of a Nano address and an error, I can change that though

@inkeliz
Copy link
Member

inkeliz commented Sep 16, 2018

I working on it right now. 👍

@wagslane
Copy link
Contributor Author

Cool, I'll just let you refactor. If need any input let me know

@wagslane
Copy link
Contributor Author

Also, if you have any generic OpenCAP questions, we are available in our discord server: https://discord.gg/FTMCBE8

@inkeliz inkeliz merged commit 80633f8 into brokenbydefault:master Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants